peter-toth commented on PR #58527:
URL: https://github.com/apache/spark/pull/58527#issuecomment-5551727395

   Thanks @sunchao, and you are right — I checked and it is worse than a 
wording problem.
   
   My "the cogroup key is synthesized" claim came from a measurement I did for 
a different follow-up, and that measurement only covered the Scala 
`CoGroupExec`, whose key comes from an `AppendColumns` no `KeyedPartitioning` 
satisfies. A Pandas or Arrow cogroup groups on real columns, so two keyed 
children do reach the per-child branch, exactly as you say, and this suite 
already had a `FlatMapCoGroupsInPandasExec` test with two of them.
   
   So the `joinKeyPositions` half is not latent. I rebuilt the plan test on 
`FlatMapCoGroupsInPandasExec` instead of the synthetic parent I had used: both 
sides declare the same two key columns in the opposite order, so the cogroup 
key sits at position 1 on the left and 0 on the right, and both project onto 
the same key set. Without the production change the right side is handed the 
left side's positions and ends up grouped on its other partition column — 
`List(Some(List(1)), Some(List(1)))` where `List(Some(List(1)), Some(List(0)))` 
is right. I have not built an end-to-end query for it, so I am not claiming a 
wrong-results reproduction, only the plan.
   
   Pushed as f6a1442, and the description no longer calls that half latent. The 
only part that stays latent is the three-or-more clustered children case.
   


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