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: [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]
