ulysses-you commented on PR #58279:
URL: https://github.com/apache/spark/pull/58279#issuecomment-5521837261

   Thanks @peter-toth @dongjoon-hyun -- everything addressed in 5af8667631b 
(rebased onto current master):
   
   **Correctness findings** (all three confirmed against the code, all 
regressions of this PR):
   
   * `unwrapGroupPartitions` now shares the rewrite descent, so the 
statistics-based replicate-side choice and the original partition keys read 
from the pre-alignment plan on every pass.
   * `applyGroupPartitions` keeps the `joinKeyPositions` a reused node already 
holds; the incoming ones are in the node's already projected report.
   * The descent and drop are confined to the join path 
(`applyGroupPartitions`); `withJoinKeyPositions` is back to reusing only a 
topmost node.
   
   Per @peter-toth's suggestion the descent is unified in one helper, 
`innermostGroupPartition`, shared by the statistics read, the rewrite and the 
`ShuffleExchangeExec` site (which now strips every grouping the rule inserted).
   
   Shape 1 was considered gated on join parents, but without the outer wrap 
pass 2 fails `createKeyedShuffleSpec`'s `satisfies` gate and silently degrades 
to a shuffle join; the no-placeholder shape belongs to the decide-the-node-once 
redesign, which goes into the follow-up with the five-shape consolidation. The 
description now carries the re-run reframing.
   
   Tests: 3 end-to-end (row count, replicate-side choice, subset-key positions) 
and 5 unit tests. Each mechanism is pinned by a shape verified to fail with its 
fix reverted in isolation. Local suites green: `KeyGroupedPartitioningSuite`, 
`EnsureRequirementsSuite`, `GroupPartitionsExecSuite`.
   


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

Reply via email to