singhpk234 commented on code in PR #16158:
URL: https://github.com/apache/iceberg/pull/16158#discussion_r3254108123
##########
spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/sql/TestTableEncryption.java:
##########
@@ -212,16 +212,27 @@ public void testInsertAndDelete() {
expected,
sql("SELECT * FROM %s ORDER BY id", tableName));
- sql("DELETE FROM %s WHERE id < 4", tableName);
+ sql("DELETE FROM %s WHERE id < 3", tableName);
- expected = ImmutableList.of(row(4L, "d", 4.0F), row(5L, "e", 5.0F),
row(6L, "f", Float.NaN));
+ expected =
+ ImmutableList.of(
+ row(3L, "c", Float.NaN),
+ row(4L, "d", 4.0F),
+ row(5L, "e", 5.0F),
+ row(6L, "f", Float.NaN));
assertEquals(
"Should return all expected rows",
expected,
sql("SELECT * FROM %s ORDER BY id", tableName));
}
+ @TestTemplate
+ public void testInsertAndDeleteMOR() {
+ sql("ALTER TABLE %s SET TBLPROPERTIES
('write.delete.mode'='merge-on-read')", tableName);
+ testInsertAndDelete();
Review Comment:
i would also additional assertions that dvs were there ? by loading the
table using the catalog obj and checking its recent snapshot summary
I understand test is gonna trigger this implicitly, though i would suggest
being explicit would be ideal as we are strictly checking dvs here.
Also what happens in v2 tables pos deletes and Eq delete, do you know ?
##########
core/src/main/java/org/apache/iceberg/DVUtil.java:
##########
@@ -188,8 +190,16 @@ private static List<DeleteFile> writeDVs(
FileIO fileIO,
String dvOutputLocation,
Map<String, Pair<PartitionSpec, StructLike>> partitions) {
- OutputFile dvOutputFile = fileIO.newOutputFile(dvOutputLocation);
- try (DVFileWriter dvFileWriter = new BaseDVFileWriter(() -> dvOutputFile,
path -> null)) {
+ EncryptedOutputFile dvEncryptedOutputFile;
+ if (fileIO instanceof EncryptingFileIO) {
+ dvEncryptedOutputFile = ((EncryptingFileIO)
fileIO).newEncryptingOutputFile(dvOutputLocation);
+ } else {
+ dvEncryptedOutputFile =
+
EncryptionUtil.plainAsEncryptedOutput(fileIO.newOutputFile(dvOutputLocation));
+ }
Review Comment:
```suggestion
if (fileIO instanceof EncryptingFileIO encryptingFileIO) {
dvEncryptedOutputFile =
encryptingFileIO.newEncryptingOutputFile(dvOutputLocation);
} else {
dvEncryptedOutputFile =
EncryptionUtil.plainAsEncryptedOutput(fileIO.newOutputFile(dvOutputLocation));
```
##########
spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/sql/TestTableEncryption.java:
##########
@@ -212,16 +212,27 @@ public void testInsertAndDelete() {
expected,
sql("SELECT * FROM %s ORDER BY id", tableName));
- sql("DELETE FROM %s WHERE id < 4", tableName);
+ sql("DELETE FROM %s WHERE id < 3", tableName);
- expected = ImmutableList.of(row(4L, "d", 4.0F), row(5L, "e", 5.0F),
row(6L, "f", Float.NaN));
+ expected =
+ ImmutableList.of(
+ row(3L, "c", Float.NaN),
+ row(4L, "d", 4.0F),
+ row(5L, "e", 5.0F),
+ row(6L, "f", Float.NaN));
assertEquals(
"Should return all expected rows",
expected,
sql("SELECT * FROM %s ORDER BY id", tableName));
}
+ @TestTemplate
+ public void testInsertAndDeleteMOR() {
Review Comment:
lets name the other test COW then ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
