[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #5618: Support performing merge appends and delete files on branches

[GitBox <gi...@apache.org>]

amogh-jahagirdar commented on code in PR #5618:
URL: https://github.com/apache/iceberg/pull/5618#discussion_r956546724


##########
core/src/test/java/org/apache/iceberg/TestDeleteFiles.java:
##########
@@ -300,6 +300,70 @@ public void testDeleteCaseSensitivity() {
         statuses(Status.DELETED));
   }
 
+  @Test
+  public void testMultipleFileDeletesOnBranch() {
+    String testBranch = "testBranch";
+    table.newAppend().appendFile(FILE_A).appendFile(FILE_B).appendFile(FILE_C).commit();
+    Snapshot main = readMetadata().currentSnapshot();
+
+    // Validate main history is as expected
+    Assert.assertEquals("Should have 1 manifest", 1, main.allManifests(FILE_IO).size());
+    validateSnapshot(null, main, FILE_A, FILE_B, FILE_C);
+
+    // Perform first delete on branch
+    table.newDelete().deleteFile(FILE_A).toBranch(testBranch).commit();
+    Snapshot afterDeletingA = table.snapshot(table.refs().get(testBranch).snapshotId());
+    Assert.assertEquals("Should have 1 manifest", 1, afterDeletingA.allManifests(FILE_IO).size());
+    validateManifestEntries(
+        afterDeletingA.allManifests(table.io()).get(0),
+        ids(afterDeletingA.snapshotId(), main.snapshotId(), main.snapshotId()),
+        files(FILE_A, FILE_B, FILE_C),
+        statuses(Status.DELETED, Status.EXISTING, Status.EXISTING));
+    table.newDelete().deleteFile(FILE_B).toBranch(testBranch).commit();
+
+    // Perform second delete
+    Snapshot afterDeletingB = table.snapshot(table.refs().get(testBranch).snapshotId());
+    Assert.assertEquals("Should have 1 manifest", 1, afterDeletingB.allManifests(FILE_IO).size());
+    validateManifestEntries(
+        afterDeletingB.allManifests(FILE_IO).get(0),
+        ids(afterDeletingB.snapshotId(), main.snapshotId()),
+        files(FILE_B, FILE_C),
+        statuses(Status.DELETED, Status.EXISTING));
+  }
+
+  @Test
+  public void testDeleteWithRowFilterWithCombinedPredicatesOnBranch() {

Review Comment:
   I've parameterized the tests, it does seem like for meaningful tests to be written here we also need to have an implementation for MergeAppend. So I added 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.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org