[GitHub] [iceberg] namrathamyske commented on a diff in pull request #5234: Core, API: BaseRowDelta to branch Impl

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

namrathamyske commented on code in PR #5234:
URL: https://github.com/apache/iceberg/pull/5234#discussion_r1067542176


##########
core/src/test/java/org/apache/iceberg/TestRowDelta.java:
##########
@@ -1449,18 +1519,48 @@ public void testRowDeltaCaseSensitivity() {
   }
 
   @Test
-  public void testRowDeltaToBranchUnsupported() {
+  public void testBranchValidationsNotValidAncestor() {
+    table.newAppend().appendFile(FILE_A).commit();
+    table.manageSnapshots().createBranch("branch", table.currentSnapshot().snapshotId()).commit();
+    table.newAppend().appendFile(FILE_B).commit();
+
+    // This commit will result in validation exception as we start validation from a snapshot which
+    // is not an ancestor of the branch
+    RowDelta rowDelta =
+        table
+            .newRowDelta()
+            .toBranch("branch")
+            .addDeletes(FILE_A_DELETES)
+            .validateFromSnapshot(table.currentSnapshot().snapshotId())
+            .conflictDetectionFilter(Expressions.alwaysTrue())
+            .validateNoConflictingDeleteFiles();
+
     AssertHelpers.assertThrows(
-        "Should reject committing row delta to branch",
-        UnsupportedOperationException.class,
-        "Cannot commit to branch someBranch: org.apache.iceberg.BaseRowDelta does not support branch commits",
-        () ->
-            table
-                .newRowDelta()
-                .caseSensitive(false)
-                .addRows(FILE_B)
-                .addDeletes(FILE_A2_DELETES)
-                .toBranch("someBranch")
-                .commit());
+        "Snapshot 2 is not an ancestor of 1",
+        IllegalArgumentException.class,
+        () -> rowDelta.commit());
+  }
+
+  @Test
+  public void testBranchValidationsValidAncestor() {
+    table.newAppend().appendFile(FILE_A).commit();
+    Long ancestorSnapshot = table.currentSnapshot().snapshotId();
+    table.manageSnapshots().createBranch("branch", ancestorSnapshot).commit();
+
+    // This commit not result in validation exception as we start validation from a snapshot which
+    // is an actual ancestor of the branch
+    table
+        .newRowDelta()
+        .toBranch("branch")
+        .addDeletes(FILE_A_DELETES)
+        .validateFromSnapshot(ancestorSnapshot)
+        .conflictDetectionFilter(Expressions.alwaysTrue())
+        .validateNoConflictingDeleteFiles()
+        .commit();
+
+    int branchSnapshot = 2;
+
+    Assert.assertEquals(table.currentSnapshot().snapshotId(), 1);
+    Assert.assertEquals(table.ops().current().ref("branch").snapshotId(), branchSnapshot);

Review Comment:
   Cool, will be removing these.



-- 
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