You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "nimesh1601 (via GitHub)" <gi...@apache.org> on 2026/08/27 04:21:51 UTC
[PR] Spark 3.5, 4.0: Add branch write option to SparkWriteOptions [iceberg]
nimesh1601 opened a new pull request, #17842:
URL: https://github.com/apache/iceberg/pull/17842
## 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 existing `SparkReadOptions.BRANCH` read option that has
been available in these modules since #5150.
## Motivation
`SparkReadOptions` exposes a `branch` read option in v3.5/v4.0/v4.1, but the corresponding write
option 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 suffix
or 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:
- If `branch` is set in both the table identifier and the write option, they must match
(otherwise a `ValidationException` is raised, symmetric to the existing read-side check in
`SparkReadConf.branch()`).
- The explicit option/identifier branch takes precedence over the session WAP branch.
- When no option/identifier branch is set, the existing WAP-branch resolution is unchanged.
This is intentionally the **minimal** backport: it does not pull in the larger v4.1
`SparkTableUtil` read/write resolution refactor from #15288 (which is v4.1-only and touched ~19
files). This mirrors the minimal-backport approach used in #16245.
## Tests
Added `writeBranchOption` and `writeBranchOptionConflictsWithIdentifier` to `TestSparkWriteConf`
in both v3.5 and v4.0. Existing WAP-branch tests are unaffected because the new code is a no-op
when the `branch` option 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"` — green
- `spotlessApply` / `spotlessCheck` — clean
## Related
- #15288 — Spark 4.1: Align handling of branches in reads and writes (introduced the v4.1 write option)
- #16245 — precedent for a minimal branch backport to v3.4/v3.5/v4.0
- #5150 — original branch read option
---
**AI Disclosure**
- Model: Claude Opus 4.8
- Platform/Tool: GitHub Copilot CLI
- Human Oversight: partially reviewed
- Prompt Summary: Investigate why the Spark branch write option (SparkWriteOptions.BRANCH) exists only in Spark 4.1, then backport it to the Spark 3.5 and 4.0 modules following the v4.1 (#15288) semantics and the minimal-backport pattern from #16245, with tests.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Backport the branch write options to Spark 3.5 and Spark 4.0 [iceberg]
Posted by "github-actions[bot] (via GitHub) via issues" <is...@iceberg.apache.org>.
github-actions[bot] commented on PR #17842:
URL: https://github.com/apache/iceberg/pull/17842#issuecomment-6566043331
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Backport the branch write options to Spark 3.5 and Spark 4.0 [iceberg]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Backport the branch write options to Spark 3.5 and Spark 4.0 [iceberg]
Posted by "nimesh1601 (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Backport the branch write options to Spark 3.5 and Spark 4.0 [iceberg]
Posted by "cbb330 (via GitHub)" <gi...@apache.org>.
cbb330 commented on PR #17842:
URL: https://github.com/apache/iceberg/pull/17842#issuecomment-5808191577
Thanks for the backport!
Metadata-only deletes still go through `SparkTable.deleteWhere` -> `SparkTableUtil.determineWriteBranch(spark, branch)`, which keeps the `"Cannot write to both branch and WAP branch"` guard. So `DELETE FROM t.branch_audit WHERE <whole-partition predicate>` with `spark.wap.branch` set should still throw, while INSERT/row-level ops now succeed.
#15288 updated this path. Intentional to skip, or should it be ported, with a TestDelete case?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Backport the branch write options to Spark 3.5 and Spark 4.0 [iceberg]
Posted by "nimesh1601 (via GitHub)" <gi...@apache.org>.
nimesh1601 commented on PR #17842:
URL: https://github.com/apache/iceberg/pull/17842#issuecomment-5599572376
> One remaining gap vs SparkReadConf.branch() (and v4.1 determineWriteBranch): only the option early-returns. Identifier + session WAP still hits "Cannot write to both branch and WAP branch". So, after this PR:
>
> * .option("branch", "audit") + spark.wap.branch -> writes to audit
> * writeTo("t.branch_audit") + spark.wap.branch -> still ValidationException
>
> Could we use the same inputBranch = identifier != null ? identifier : option early-return as SparkReadConf, and update TestPartitionedWritesToWapBranch the way #15288 did on v4.1? If keeping identifier+WAP as an error is intentional, please call that out in the PR description, right now it says identifier also takes precedence.
>
> A small writeTo(...).option("branch", ...) round-trip would also help; the current tests only construct SparkWriteConf.
Thanks @uros-b. I've dropped the identifier+WAP error rather than keeping it. `SparkWriteConf.branch()` (3.5 + 4.0) now mirrors `SparkReadConf.branch()` / the v4.1 determineWriteBranch : it computes `inputBranch = branch != null ? branch : optionBranch` and early-returns when non-null, so an explicit identifier branch also takes precedence over the session WAP branch. The now-unreachable Cannot write to both branch and WAP branch guard is removed. Both of your cases now behave the same:
- option("branch", "audit") + spark.wap.branch → writes to audit
- writeTo("t.branch_audit") + spark.wap.branch → writes to audit
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Backport the branch write options to Spark 3.5 and Spark 4.0 [iceberg]
Posted by "nimesh1601 (via GitHub)" <gi...@apache.org>.
nimesh1601 commented on PR #17842:
URL: https://github.com/apache/iceberg/pull/17842#issuecomment-5674694847
@uros-b can you take a look?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Backport the branch write options to Spark 3.5 and Spark 4.0 [iceberg]
Posted by "nimesh1601 (via GitHub)" <gi...@apache.org>.
nimesh1601 commented on PR #17842:
URL: https://github.com/apache/iceberg/pull/17842#issuecomment-5442998059
@bryanck Can you help in reviewing this PR ?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Backport the branch write options to Spark 3.5 and Spark 4.0 [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17842:
URL: https://github.com/apache/iceberg/pull/17842#issuecomment-6391412247
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Backport the branch write options to Spark 3.5 and Spark 4.0 [iceberg]
Posted by "github-actions[bot] (via GitHub) via issues" <is...@iceberg.apache.org>.
github-actions[bot] closed pull request #17842: Spark: Backport the branch write options to Spark 3.5 and Spark 4.0
URL: https://github.com/apache/iceberg/pull/17842
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Backport the branch write options to Spark 3.5 and Spark 4.0 [iceberg]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #17842:
URL: https://github.com/apache/iceberg/pull/17842#issuecomment-5598165102
One remaining gap vs SparkReadConf.branch() (and v4.1 determineWriteBranch): only the option early-returns. Identifier + session WAP still hits "Cannot write to both branch and WAP branch". So, after this PR:
- .option("branch", "audit") + spark.wap.branch -> writes to audit
- writeTo("t.branch_audit") + spark.wap.branch -> still ValidationException
Could we use the same inputBranch = identifier != null ? identifier : option early-return as SparkReadConf, and update TestPartitionedWritesToWapBranch the way #15288 did on v4.1? If keeping identifier+WAP as an error is intentional, please call that out in the PR description, right now it says identifier also takes precedence.
A small writeTo(...).option("branch", ...) round-trip would also help; the current tests only construct SparkWriteConf.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Backport the branch write options to Spark 3.5 and Spark 4.0 [iceberg]
Posted by "nimesh1601 (via GitHub)" <gi...@apache.org>.
nimesh1601 commented on PR #17842:
URL: https://github.com/apache/iceberg/pull/17842#issuecomment-5589051845
@uros-b does this looks good ?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org