dramaticlly commented on code in PR #7652:
URL: https://github.com/apache/iceberg/pull/7652#discussion_r1199158777
##########
spark/v3.4/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestBranchDDL.java:
##########
@@ -89,6 +91,13 @@ public void testCreateBranch() throws NoSuchTableException {
() -> sql("ALTER TABLE %s CREATE BRANCH %s", tableName, branchName));
}
+ @Test
+ public void testCreateBranchOnEmptyTable() {
+ Assertions.assertThatThrownBy(() -> sql("ALTER TABLE %s CREATE BRANCH %s",
tableName, "b1"))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageContaining("Cannot set b1 to unknown snapshot");
Review Comment:
discussed offline, looks like this is relate to implicit scala Long to
primitive java.lang.long conversion, seems fragile and confusion if we rely on
that to guard against bad input. So we ended check snapshotId not null before
pass to java code and also make unit test more self-contained.
@amogh-jahagirdar ready for another look, if new exception and error message
looks good to you, I'll backport fix to other spark version and try to get it
merged for 1.3.0 release
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
