This is an automated email from the ASF dual-hosted git repository.
blue pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/main by this push:
new afe4aec4db Spark: Don't allow branch_ usage with VERSION AS OF (#9219)
afe4aec4db is described below
commit afe4aec4db795f4829757d738a5bbcf1b7db8fd2
Author: Eduard Tudenhoefner <[email protected]>
AuthorDate: Tue Dec 5 17:14:09 2023 +0100
Spark: Don't allow branch_ usage with VERSION AS OF (#9219)
---
.../main/java/org/apache/iceberg/spark/SparkCatalog.java | 2 +-
.../java/org/apache/iceberg/spark/source/SparkTable.java | 4 ++++
.../java/org/apache/iceberg/spark/sql/TestSelect.java | 15 +++++++++++++++
.../main/java/org/apache/iceberg/spark/SparkCatalog.java | 2 +-
.../java/org/apache/iceberg/spark/source/SparkTable.java | 4 ++++
.../java/org/apache/iceberg/spark/sql/TestSelect.java | 15 +++++++++++++++
.../main/java/org/apache/iceberg/spark/SparkCatalog.java | 2 +-
.../java/org/apache/iceberg/spark/source/SparkTable.java | 4 ++++
.../java/org/apache/iceberg/spark/sql/TestSelect.java | 15 +++++++++++++++
9 files changed, 60 insertions(+), 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
index 02bbec6824..48decf995f 100644
--- 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 class SparkCatalog extends BaseCatalog {
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
index 574d014e83..eddcdb1819 100644
---
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 class SparkTable
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
index e08bc4574d..1368c26792 100644
---
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
@@ -426,6 +426,21 @@ public class TestSelect extends SparkCatalogTestBase {
});
}
+ @Test
+ public void testInvalidTimeTravelAgainstBranchIdentifierWithAsOf() {
+ long snapshotId =
validationCatalog.loadTable(tableIdent).currentSnapshot().snapshotId();
+
validationCatalog.loadTable(tableIdent).manageSnapshots().createBranch("b1").commit();
+
+ // create a second snapshot
+ sql("INSERT INTO %s VALUES (4, 'd', 4.0), (5, 'e', 5.0)", tableName);
+
+ // 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
public void testSpecifySnapshotAndTimestamp() {
// get the snapshot ID of the last write
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
index 6958ebc103..6b7becc77c 100644
--- 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 class SparkCatalog extends BaseCatalog {
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
index e200bee03e..bbc7434138 100644
---
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 class SparkTable
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
index dacaee7d80..161c2e0ba6 100644
---
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
@@ -421,6 +421,21 @@ public class TestSelect extends SparkCatalogTestBase {
.hasMessage("Cannot do time-travel based on both table identifier and
AS OF");
}
+ @Test
+ public void testInvalidTimeTravelAgainstBranchIdentifierWithAsOf() {
+ long snapshotId =
validationCatalog.loadTable(tableIdent).currentSnapshot().snapshotId();
+
validationCatalog.loadTable(tableIdent).manageSnapshots().createBranch("b1").commit();
+
+ // create a second snapshot
+ sql("INSERT INTO %s VALUES (4, 'd', 4.0), (5, 'e', 5.0)", tableName);
+
+ // 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
public void testSpecifySnapshotAndTimestamp() {
// get the snapshot ID of the last write
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
index 467b428994..eef0f0703b 100644
--- 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 class SparkCatalog extends BaseCatalog {
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
index e200bee03e..bbc7434138 100644
---
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 class SparkTable
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
index dacaee7d80..161c2e0ba6 100644
---
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
@@ -421,6 +421,21 @@ public class TestSelect extends SparkCatalogTestBase {
.hasMessage("Cannot do time-travel based on both table identifier and
AS OF");
}
+ @Test
+ public void testInvalidTimeTravelAgainstBranchIdentifierWithAsOf() {
+ long snapshotId =
validationCatalog.loadTable(tableIdent).currentSnapshot().snapshotId();
+
validationCatalog.loadTable(tableIdent).manageSnapshots().createBranch("b1").commit();
+
+ // create a second snapshot
+ sql("INSERT INTO %s VALUES (4, 'd', 4.0), (5, 'e', 5.0)", tableName);
+
+ // 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
public void testSpecifySnapshotAndTimestamp() {
// get the snapshot ID of the last write
