Spark: Backport the branch write options to Spark 3.5 and Spark 4.0 - #17842
Spark: Backport the branch write options to Spark 3.5 and Spark 4.0#17842nimesh1601 wants to merge 2 commits into
Conversation
Backport the branch write option added to Spark 4.1 in apache#15288 to the Spark 3.5 and 4.0 modules. SparkWriteOptions.BRANCH lets a DataFrame write target a table branch via .option("branch", "..."), mirroring the existing SparkReadOptions.BRANCH read option. SparkWriteConf.branch() resolves the write option first: it must not conflict with the identifier branch when both are set, and it takes precedence over the session WAP branch. This matches the read-side resolution already present in SparkReadConf and the v4.1 behavior, without pulling in the larger v4.1 SparkTableUtil refactor from apache#15288 (consistent with the minimal backport approach used in apache#16245). Generated-by: GitHub Copilot CLI (Claude Opus 4.8) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
nimesh1601
changed the title
nimesh1601
changed the title
|
@bryanck Can you help in reviewing this PR ? |
| .hasMessageContaining( | ||
| "Must not specify different branches in both table identifier and write option"); | ||
| } | ||
|
|
There was a problem hiding this 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.
There was a problem hiding this 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.
2 participants
Summary
Backport the branch write option added to Spark 4.1 in #15288 to the Spark 3.5 and 4.0 modules.
SparkWriteOptions.BRANCH("branch") lets a DataFrame write target a table branch via.option("branch", "..."), mirroring the existingSparkReadOptions.BRANCHread option that hasbeen available in these modules since #5150.
Motivation
SparkReadOptionsexposes abranchread option in v3.5/v4.0/v4.1, but the corresponding writeoption only exists in v4.1 (added by #15288). This leaves v3.5 and v4.0 users unable to target a
branch through the DataFrame writer option — they can only use the
branch_<name>identifier suffixor a session-level WAP branch. This PR closes that read/write asymmetry for the older modules.
Change
SparkWriteConf.branch()now resolves the write option first:branchis set in both the table identifier and the write option, they must match(otherwise a
ValidationExceptionis raised, symmetric to the existing read-side check inSparkReadConf.branch()).This is intentionally the minimal backport: it does not pull in the larger v4.1
SparkTableUtilread/write resolution refactor from #15288 (which is v4.1-only and touched ~19files). This mirrors the minimal-backport approach used in #16245.
Tests
Added
writeBranchOptionandwriteBranchOptionConflictsWithIdentifiertoTestSparkWriteConfin both v3.5 and v4.0. Existing WAP-branch tests are unaffected because the new code is a no-op
when the
branchoption is absent../gradlew -DsparkVersions=3.5,4.0 :iceberg-spark:iceberg-spark-3.5_2.12:test :iceberg-spark:iceberg-spark-4.0_2.13:test --tests "org.apache.iceberg.spark.TestSparkWriteConf"— greenspotlessApply/spotlessCheck— cleanRelated