Repository navigation
fix: copy rows retained by identity broadcasts - #7260
Conversation
sunchao
left a comment
There was a problem hiding this comment.
LGTM. Reviewed 52c6067 against 01fc37a with five independent reviewers, including Spark semantics, row ownership, planner paths, tests, and an adversarial pass. No actionable issue found.
The copy happens before either converter reuses the UnsafeRow, preserving the complete row storage for identity broadcasts. Hashed broadcasts remain unchanged. Release branch-1.2 has the same original bug, so this is an intended correctness fix there too.
The new regression passed with both converters in the Spark 4.1 execution job, alongside 1,780 passing tests. That job tested merge ff9f7c3, whose tree matches the reviewed head. A separate Spark 4.1.3 ownership probe reproduced repeated rows without copying and distinct rows with copying, including strings, nested structs, arrays, and nulls. Broader Spark SQL jobs were skipped; no full local Comet runtime suite was run.
Which issue does this PR close?
Part of #6741. First of four smaller changes replacing #7117. The expression safeguards are tracked separately in #7261; both PRs target main independently.
Rationale for this change
An identity broadcast retains every input row, but the JVM columnar-to-row converters reuse their row object. Materializing that iterator can therefore repeat the last row of each batch instead of preserving the build rows.
What changes are included in this PR?
Copy each row when adapting a Comet broadcast to
IdentityBroadcastMode. Hashed broadcast modes already copy row data into their own storage. This fix is independent of the make_date evaluation changes.How are these changes tested?
A focused regression directly converts an identity broadcast containing four distinct rows across two partitions. It verifies retained values and cardinality with the direct JVM converter enabled and disabled.
Full-reactor JVM and test-source compilation passes on Spark 4.1 / JDK 17, including formatting and style checks. Runtime execution is blocked because the configured native dependency registry does not contain the locked
aws-config 1.13.0package.