don't allow branch_ usage with VERSION AS OF

apache · Nov 23, 2023 · 0127c1d · 0127c1d
1 parent 986f333
commit 0127c1d
Show file tree

Hide file tree

Showing 9 changed files with 36 additions and 3 deletions.
diff --git a/spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java b/spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java
@@ -171,7 +171,7 @@ public Table loadTable(Identifier ident, String version) throws NoSuchTableExcep
       SparkTable sparkTable = (SparkTable) table;
 
       Preconditions.checkArgument(
-          sparkTable.snapshotId() == null,
+          sparkTable.snapshotId() == null && sparkTable.branch() == null,
           "Cannot do time-travel based on both table identifier and AS OF");
 
       try {

diff --git a/spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkTable.java b/spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkTable.java
@@ -173,6 +173,10 @@ public Long snapshotId() {
     return snapshotId;
   }
 
+  public String branch() {
+    return branch;
+  }
+
   public SparkTable copyWithSnapshotId(long newSnapshotId) {
     return new SparkTable(icebergTable, newSnapshotId, refreshEagerly);
   }

diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSelect.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSelect.java
@@ -371,6 +371,7 @@ public void testTimestampAsOf() {
   public void testInvalidTimeTravelBasedOnBothAsOfAndTableIdentifier() {
     // get the snapshot ID of the last write
     long snapshotId = validationCatalog.loadTable(tableIdent).currentSnapshot().snapshotId();
+    validationCatalog.loadTable(tableIdent).manageSnapshots().createBranch("b1").commit();
     // get a timestamp just after the last write
     long timestamp =
         validationCatalog.loadTable(tableIdent).currentSnapshot().timestampMillis() + 2;
@@ -424,6 +425,12 @@ public void testInvalidTimeTravelBasedOnBothAsOfAndTableIdentifier() {
               "SELECT * FROM %s.%s TIMESTAMP AS OF %s",
               tableName, timestampPrefix + timestamp, timestamp);
         });
+
+    // using branch_b1 in the table identifier and VERSION AS OF
+    Assertions.assertThatThrownBy(
+            () -> sql("SELECT * FROM %s.branch_b1 VERSION AS OF %s", tableName, snapshotId))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Cannot do time-travel based on both table identifier and AS OF");
   }
 
   @Test

diff --git a/spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java b/spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java
@@ -170,7 +170,7 @@ public Table loadTable(Identifier ident, String version) throws NoSuchTableExcep
       SparkTable sparkTable = (SparkTable) table;
 
       Preconditions.checkArgument(
-          sparkTable.snapshotId() == null,
+          sparkTable.snapshotId() == null && sparkTable.branch() == null,
           "Cannot do time-travel based on both table identifier and AS OF");
 
       try {

diff --git a/spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/source/SparkTable.java b/spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/source/SparkTable.java
@@ -173,6 +173,10 @@ public Long snapshotId() {
     return snapshotId;
   }
 
+  public String branch() {
+    return branch;
+  }
+
   public SparkTable copyWithSnapshotId(long newSnapshotId) {
     return new SparkTable(icebergTable, newSnapshotId, refreshEagerly);
   }

diff --git a/spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/sql/TestSelect.java b/spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/sql/TestSelect.java
@@ -370,6 +370,7 @@ public void testTimestampAsOf() {
   public void testInvalidTimeTravelBasedOnBothAsOfAndTableIdentifier() {
     // get the snapshot ID of the last write
     long snapshotId = validationCatalog.loadTable(tableIdent).currentSnapshot().snapshotId();
+    validationCatalog.loadTable(tableIdent).manageSnapshots().createBranch("b1").commit();
     // get a timestamp just after the last write
     long timestamp =
         validationCatalog.loadTable(tableIdent).currentSnapshot().timestampMillis() + 2;
@@ -419,6 +420,12 @@ public void testInvalidTimeTravelBasedOnBothAsOfAndTableIdentifier() {
             })
         .isInstanceOf(IllegalArgumentException.class)
         .hasMessage("Cannot do time-travel based on both table identifier and AS OF");
+
+    // using branch_b1 in the table identifier and VERSION AS OF
+    Assertions.assertThatThrownBy(
+            () -> sql("SELECT * FROM %s.branch_b1 VERSION AS OF %s", tableName, snapshotId))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Cannot do time-travel based on both table identifier and AS OF");
   }
 
   @Test

diff --git a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java
@@ -170,7 +170,7 @@ public Table loadTable(Identifier ident, String version) throws NoSuchTableExcep
       SparkTable sparkTable = (SparkTable) table;
 
       Preconditions.checkArgument(
-          sparkTable.snapshotId() == null,
+          sparkTable.snapshotId() == null && sparkTable.branch() == null,
           "Cannot do time-travel based on both table identifier and AS OF");
 
       try {

diff --git a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/source/SparkTable.java b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/source/SparkTable.java
@@ -173,6 +173,10 @@ public Long snapshotId() {
     return snapshotId;
   }
 
+  public String branch() {
+    return branch;
+  }
+
   public SparkTable copyWithSnapshotId(long newSnapshotId) {
     return new SparkTable(icebergTable, newSnapshotId, refreshEagerly);
   }

diff --git a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/sql/TestSelect.java b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/sql/TestSelect.java
@@ -370,6 +370,7 @@ public void testTimestampAsOf() {
   public void testInvalidTimeTravelBasedOnBothAsOfAndTableIdentifier() {
     // get the snapshot ID of the last write
     long snapshotId = validationCatalog.loadTable(tableIdent).currentSnapshot().snapshotId();
+    validationCatalog.loadTable(tableIdent).manageSnapshots().createBranch("b1").commit();
     // get a timestamp just after the last write
     long timestamp =
         validationCatalog.loadTable(tableIdent).currentSnapshot().timestampMillis() + 2;
@@ -419,6 +420,12 @@ public void testInvalidTimeTravelBasedOnBothAsOfAndTableIdentifier() {
             })
         .isInstanceOf(IllegalArgumentException.class)
         .hasMessage("Cannot do time-travel based on both table identifier and AS OF");
+
+    // using branch_b1 in the table identifier and VERSION AS OF
+    Assertions.assertThatThrownBy(
+            () -> sql("SELECT * FROM %s.branch_b1 VERSION AS OF %s", tableName, snapshotId))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Cannot do time-travel based on both table identifier and AS OF");
   }
 
   @Test