dongjoon-hyun commented on PR #58318: URL: https://github.com/apache/spark/pull/58318#issuecomment-5429408987
Thank you for the careful review, @peter-toth! **On 1:** You are right about the inner-join case, and about the mechanism: `ShuffledJoin` builds a `PartitioningCollection` from both sides only for inner-like joins, and on the subset-keys path the two `partitionKeys` are separate objects built by `projectKeys`, so the `eq` fast path does not apply and SPARK-56877's `require` runs. I reproduced your experiment and got the same planning-time failure on the `SPARK-58988` test. That loudness is a side effect of inner joins exposing both sides' partitionings, though, not a guard on the physical layout. For join types where `ShuffledJoin.outputPartitioning` exposes only one side (`LeftOuter`, `RightOuter`, `LeftSemi`, `LeftAnti`), nothing compares the two orders. With the same reversed ordering, a LEFT JOIN variant of the `SPARK-58988` scenario plans fine (same shape: one shuffle, one `GroupPartitionsExec`) and silently returns wrong results: ``` !== Correct Answer - 3 == == Spark Answer - 3 == ![1,aa,2021,1,aa] [1,aa,2021,null,null] ``` I updated the scaladoc, the test comment, and the PR description to state both failure modes precisely instead of claiming the divergence is always silent, and added that LEFT JOIN variant to `KeyGroupedPartitioningSuite` as a regression test -- for the one-sided join types the answer check is the only guard, and it also covers the shuffle partitioner's physical layout, which the unit test in `GroupPartitionsExecSuite` does not reach. **On 2:** Fair point -- SPARK-59022 was `getPartitioner` deriving a third order that ignored the declared one, not two sorts drifting apart. I reworded the description to call it a related but distinct hazard, and to note this coupling was found during the review of #58311. -- 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]
