uros-b commented on code in PR #17842:
URL: https://github.com/apache/iceberg/pull/17842#discussion_r3880229627
##########
spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -86,6 +87,35 @@ public void after() {
sql("DROP TABLE IF EXISTS %s", tableName);
}
+ @TestTemplate
+ public void writeBranchOption() {
+ Table table = validationCatalog.loadTable(tableIdent);
+
+ SparkWriteConf branchFromOption =
+ new SparkWriteConf(
+ spark, table, null, ImmutableMap.of(SparkWriteOptions.BRANCH,
"branchA"));
+ assertThat(branchFromOption.branch()).isEqualTo("branchA");
+
+ SparkWriteConf matchingIdentifierAndOption =
+ new SparkWriteConf(
+ spark, table, "branchA", ImmutableMap.of(SparkWriteOptions.BRANCH,
"branchA"));
+ assertThat(matchingIdentifierAndOption.branch()).isEqualTo("branchA");
+ }
+
+ @TestTemplate
+ public void writeBranchOptionConflictsWithIdentifier() {
+ Table table = validationCatalog.loadTable(tableIdent);
+
+ SparkWriteConf conflicting =
+ new SparkWriteConf(
+ spark, table, "branchB", ImmutableMap.of(SparkWriteOptions.BRANCH,
"branchA"));
+
+ assertThatThrownBy(conflicting::branch)
+ .isInstanceOf(ValidationException.class)
+ .hasMessageContaining(
+ "Must not specify different branches in both table identifier and
write option");
+ }
+
Review Comment:
No test exercises the PR's headline behavioral guarantee: option branch
takes precedence over a session WAP branch (i.e., optionBranch != null while
wapEnabled() == true && wapBranch != null -> returns option branch, bypassing
the WAP guards). This is the one new code path introduced by the PR; the
companion docs PR #17827 explicitly documents this precedence, but it is
unverified by any test in either module. Please add a case asserting the option
wins when WAP is also configured.
--
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]