peter-toth commented on PR #58351: URL: https://github.com/apache/spark/pull/58351#issuecomment-5464623787
@ulysses-you the report I promised on #58338, on how the tightened notion reconciles with `allClusterKeysCovered`. **Your shape is the one this PR fixes.** A source reporting several splits per partition key, projected onto exactly the operator's clustering, no longer trips the gate, because the flag means an actual collapse instead of "positions were dropped". Measured on it with the opt-in off: before, no `GroupPartitionsExec` and 2 shuffles; after, 1 `GroupPartitionsExec` and 0 shuffles. It is a test in this PR, `several splits per partition key are grouped without allowKeysSubsetOfPartitionKeys`, plus a unit test on the partitioning itself. **So the mismatch you named is gone.** Duplicated keys on their own no longer refuse anything, which is the tolerance `allClusterKeysCovered`'s comment states. The gate now refuses only when two keys that were distinct in the source ended up on one key, and it is insensitive to key order and to repeated cluster keys for the same reason that gate is: it compares distinct key *values*, which no permutation or repetition of the key expressions changes. **On the two gates sharing one notion, I ended up thinking they should not.** They ask different questions and neither subsumes the other. `allClusterKeysCovered` is a set-coverage test on expressions, "is every cluster key among the partition keys", and it guards against joining on keys coarser than the join keys. The collapse gate is a test on the key values, and it guards a coarsening that already happened upstream in a projection or a reduction. Coverage says nothing about whether those keys collapsed, so with `requireAllClusterKeysForCoPartition` on the collapse gate still has work to do. What they now share is the tolerance, not the notion. -- 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]
