sunchao opened a new pull request, #58684: URL: https://github.com/apache/spark/pull/58684
JIRA: [SPARK-59054](https://issues.apache.org/jira/browse/SPARK-59054). This is the branch-4.0 adaptation of the existing upstream issue. Draft: standalone and full-build validation remain pending. ### What changes were proposed in this pull request? Backport [SPARK-59054 / #58345](https://github.com/apache/spark/pull/58345) to branch-4.0, adapting the implementation to `KeyGroupedPartitioning`. Both the driver's partition-key map and each executor's lookup keys now use `InternalRowComparableWrapper` with the shuffle side's expression types. The wrapper is serializable, rebuilding its transient schema and ordering from the existing caches after deserialization. `KeyGroupedPartitioner` uses an immutable lookup map, so the executor can safely reuse its temporary key row. The adaptation preserves the existing order of `uniquePartitionValues`. It includes only the small precomputed wrapper-factory and constructor support needed from [SPARK-54383 / #53097](https://github.com/apache/spark/pull/53097); existing two-argument constructor callers remain supported. It does not require the later SPJ partitioning refactor. Original SPARK-59054 implementation and regression coverage: Dongjoon Hyun, commit `74b5db8651d5cc13b6e75a96044c0186f16570f7`. Wrapper-factory prerequisite: Chirag Singh, commit `13fea4fa02c6a6fa77cf72ec15649dd262addef6`. ### Why are the changes needed? Spark 4.0 groups partition keys using SQL row equality but previously routed shuffled rows through `Seq[Any]` equality. Binary arrays with equal contents can fail the lookup because their array instances differ, causing matching rows to reach different partitions and be omitted from the join result. Using the same SQL-aware representation on both sides also handles signed zero consistently and keeps struct lookup keys aligned with the shuffled side's declared field names. ### Does this PR introduce _any_ user-facing change? Yes. Storage-partitioned joins with `spark.sql.sources.v2.bucketing.shuffle.enabled=true` correctly route binary and other SQL-comparable partition keys that previously could lose matches. ### How was this patch tested? Carried over the three SPARK-59054 regressions in `KeyGroupedPartitioningSuite`: - `SPARK-59054: shuffle one side: partition keys with binary type` - `SPARK-59054: shuffle one side: struct partition keys with different field names` - `SPARK-59054: shuffle one side: partition transform collapsing -0.0 and 0.0` Each compares results with one-sided bucketing shuffle enabled and disabled and asserts the expected one-versus-two-shuffle plan. The signed-zero test uses the original public test transform and the 4.0 catalog's existing cleanup mechanism. Static validation passed: `git diff --check`, added-source ASCII, and source line-length checks. The baseline binary-key case returned only two of eight expected rows after its one-shuffle assertion passed. The struct-field-name and signed-zero cases already passed on the baseline; they provide compatibility coverage, not claimed before-failures. Fresh selected-source local validation of a combined branch-4.0 tree containing the separate window, key-routing, scan-ordering and NULL-fixture proposals passed all 11 focused tests. The broader run completed all 244 exact test identities: 237 passed and seven function-based ordering cases in WriteDistributionAndOrderingSuite failed. All seven also failed on the Apache production baseline with matching exception types, messages and first eight stack frames; they remain a limitation of this local setup, not a passing full-suite result. No tests were skipped and no suite aborted; fresh-class origin checks passed. The separate baseline control reproduced six targeted production regressions, while five expected controls passed. Removing only the two NULL-fixture guards separately reproduced the insertion exception and stale-value result. Candidate tests cover their complete loops; baseline failures stop at the first failing iteration and do not establish later iterations. All nine changed files in the combined 4.0 tree passed Scalastyle with zero errors or warnings. JDK 17 / Scala 2.13.16 freshly compiled 40 selected Scala production sources, five Java sources and nine test/fixture sources; remaining dependencies were cached and fingerprinted. This is not a complete build, standalone validation of this PR, a whole-source-equivalent Apache runtime, or CI success. Earlier failed setup and audit attempts are retained separately and are not counted as passes. Standalone validation still required: ```text build/sbt "sql/testOnly *KeyGroupedPartitioningSuite -- -z SPARK-59054" ``` ### Was this patch authored or co-authored using generative AI tooling? The original SPARK-59054 commit records `Generated-by: Claude Fable 5`. Generated-by: OpenAI Codex (version not exposed in this session), for the branch-4.0 adaptation and PR preparation. -- 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]
