amogh-jahagirdar commented on code in PR #18408:
URL: https://github.com/apache/iceberg/pull/18408#discussion_r4254138968
##########
core/src/test/java/org/apache/iceberg/TestRemoveSnapshots.java:
##########
@@ -2144,6 +2144,105 @@ public void testCleanupLevelNullValidation() {
.hasMessageContaining("Invalid cleanup level: null");
}
+ @TestTemplate
+ public void testExpireDoesNotDeletePuffinFileSharedByLiveDeletionVectors()
throws Exception {
+ assumeThat(formatVersion).as("Deletion vectors require V3 or
later").isGreaterThanOrEqualTo(3);
+
+ table.newAppend().appendFile(FILE_A).appendFile(FILE_B).commit();
+
+ // one Puffin file with deletion vectors for FILE_A and FILE_B
+ String sharedPuffinPath = "/path/to/shared-dvs-" + UUID.randomUUID() +
".puffin";
+ DeleteFile dvA = deletionVector(sharedPuffinPath, FILE_A, 4L);
+ DeleteFile dvB = deletionVector(sharedPuffinPath, FILE_B, 44L);
+ table.newRowDelta().addDeletes(dvA).addDeletes(dvB).commit();
+
+ assertThat(liveDeleteFilePaths())
+ .as("Table should have two deletion vectors in the shared Puffin file")
+ .containsExactly(sharedPuffinPath, sharedPuffinPath);
+
+ // replace FILE_A's deletion vector; FILE_B's stays in the shared Puffin
file
+ DeleteFile newDvA =
+ deletionVector("/path/to/new-dv-" + UUID.randomUUID() + ".puffin",
FILE_A, 4L);
+ table
+ .newRowDelta()
+ .validateFromSnapshot(table.currentSnapshot().snapshotId())
+ .removeDeletes(dvA)
+ .addDeletes(newDvA)
+ .commit();
+
+ table.newAppend().appendFile(FILE_C).commit();
+
+ long tAfterCommits =
waitUntilAfter(table.currentSnapshot().timestampMillis());
+
+ Set<String> deletedFiles = Sets.newHashSet();
+
removeSnapshots(table).expireOlderThan(tAfterCommits).deleteWith(deletedFiles::add).commit();
+
+ assertThat(table.snapshots()).hasSize(1);
+ assertThat(deletedFiles)
+ .as("Puffin file with a live deletion vector must not be deleted")
+ .doesNotContain(sharedPuffinPath);
+ assertThat(liveDeleteFilePaths())
Review Comment:
Same as above, though I think I feel a bit more strongly about removing this
instance especially. This is basically testing scan planning behavior of live
files which is independent of this.
--
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]
