nimesh1601 commented on code in PR #17842:
URL: https://github.com/apache/iceberg/pull/17842#discussion_r3885883343
##########
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:
Thanks for the review @uros-b ! you're right that the option-wins-over-WAP
path wasn't covered. I've added a test for it to TestSparkWriteConf in both
the v3.5 and v4.0 modules. While adding this, I noticed the original v4.1 PR
(#15288) doesn't unit-test the option ->branch resolution either - it only
changed the SparkWriteConf constructors mechanically and covered branch
precedence via the SQL-identifier integration tests. So this actually adds
coverage the source PR lacked. Happy to open a small follow-up PR adding the
sameTestSparkWriteConf case to v4.1 for parity if that's useful.
--
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]
