RussellSpitzer commented on a change in pull request #1211:
URL: https://github.com/apache/iceberg/pull/1211#discussion_r456022020
##########
File path: core/src/test/java/org/apache/iceberg/TestRemoveSnapshots.java
##########
@@ -404,6 +404,55 @@ public void dataFilesCleanup() throws IOException {
Assert.assertTrue("FILE_B should be deleted",
deletedFiles.contains(FILE_B.path().toString()));
}
+ @Test
+ public void noDataFileCleanup() throws IOException {
+ table.newFastAppend()
+ .appendFile(FILE_A)
+ .commit();
+
+ table.newFastAppend()
+ .appendFile(FILE_B)
+ .commit();
+
+ table.newRewrite()
+ .rewriteFiles(ImmutableSet.of(FILE_B), ImmutableSet.of(FILE_D))
+ .commit();
+ long thirdSnapshotId = table.currentSnapshot().snapshotId();
+
+ table.newRewrite()
+ .rewriteFiles(ImmutableSet.of(FILE_A), ImmutableSet.of(FILE_C))
+ .commit();
+ long fourthSnapshotId = table.currentSnapshot().snapshotId();
+
+ long t4 = System.currentTimeMillis();
+ while (t4 <= table.currentSnapshot().timestampMillis()) {
+ t4 = System.currentTimeMillis();
+ }
+
+ List<ManifestFile> manifests = table.currentSnapshot().dataManifests();
+
+ ManifestFile newManifest = writeManifest(
+ "manifest-file-1.avro",
+ manifestEntry(Status.EXISTING, thirdSnapshotId, FILE_C),
+ manifestEntry(Status.EXISTING, fourthSnapshotId, FILE_D));
+
+ RewriteManifests rewriteManifests = table.rewriteManifests();
+ manifests.forEach(rewriteManifests::deleteManifest);
+ rewriteManifests.addManifest(newManifest);
+ rewriteManifests.commit();
+
+ Set<String> deletedFiles = Sets.newHashSet();
+
+ table.expireSnapshots()
+ .cleanUpFiles(false)
+ .expireOlderThan(t4)
+ .deleteWith(deletedFiles::add)
+ .commit();
+
+ Assert.assertFalse("FILE_A should not be deleted",
deletedFiles.contains(FILE_A.path().toString()));
+ Assert.assertFalse("FILE_B should not be deleted",
deletedFiles.contains(FILE_B.path().toString()));
Review comment:
All of the tests here just check whether or not the set contains or
doesn't contain a path. The deleteFunction here just adds paths to a set
instead of deleting things so if they don't appear in the set the file was not
"deleted". So in all the tests in this file the function will never actually
delete.
##########
File path: core/src/test/java/org/apache/iceberg/TestRemoveSnapshots.java
##########
@@ -404,6 +404,55 @@ public void dataFilesCleanup() throws IOException {
Assert.assertTrue("FILE_B should be deleted",
deletedFiles.contains(FILE_B.path().toString()));
}
+ @Test
+ public void noDataFileCleanup() throws IOException {
+ table.newFastAppend()
+ .appendFile(FILE_A)
+ .commit();
+
+ table.newFastAppend()
+ .appendFile(FILE_B)
+ .commit();
+
+ table.newRewrite()
+ .rewriteFiles(ImmutableSet.of(FILE_B), ImmutableSet.of(FILE_D))
+ .commit();
+ long thirdSnapshotId = table.currentSnapshot().snapshotId();
+
+ table.newRewrite()
+ .rewriteFiles(ImmutableSet.of(FILE_A), ImmutableSet.of(FILE_C))
+ .commit();
+ long fourthSnapshotId = table.currentSnapshot().snapshotId();
+
+ long t4 = System.currentTimeMillis();
+ while (t4 <= table.currentSnapshot().timestampMillis()) {
+ t4 = System.currentTimeMillis();
+ }
+
+ List<ManifestFile> manifests = table.currentSnapshot().dataManifests();
+
+ ManifestFile newManifest = writeManifest(
+ "manifest-file-1.avro",
+ manifestEntry(Status.EXISTING, thirdSnapshotId, FILE_C),
+ manifestEntry(Status.EXISTING, fourthSnapshotId, FILE_D));
+
+ RewriteManifests rewriteManifests = table.rewriteManifests();
+ manifests.forEach(rewriteManifests::deleteManifest);
+ rewriteManifests.addManifest(newManifest);
+ rewriteManifests.commit();
+
+ Set<String> deletedFiles = Sets.newHashSet();
+
+ table.expireSnapshots()
+ .cleanUpFiles(false)
+ .expireOlderThan(t4)
+ .deleteWith(deletedFiles::add)
+ .commit();
+
+ Assert.assertFalse("FILE_A should not be deleted",
deletedFiles.contains(FILE_A.path().toString()));
Review comment:
Sure we can do that
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
