peter-toth commented on code in PR #58338:
URL: https://github.com/apache/spark/pull/58338#discussion_r3872837969
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/physical/partitioning.scala:
##########
@@ -579,7 +581,17 @@ case class KeyedPartitioning(
def groupedSatisfies(required: Distribution): Boolean = {
required match {
case c @ ClusteredDistribution(requiredClustering,
requireAllClusterKeys, _, _) =>
- if (requireAllClusterKeys) {
+ if (isNarrowed && !isGrouped &&
+ !SQLConf.get.v2BucketingAllowKeysSubsetOfPartitionKeys) {
Review Comment:
Agreed, and thanks for the `allClusterKeysCovered` pointer - its comment
("Key order and duplicated cluster keys don't matter") is exactly the tolerance
I want this guard to have. The follow-up is already written: the flag will mean
actual key collapse - a projection mapping keys that were distinct in the input
onto the same projected key - instead of "positions were dropped", so a source
that reports several splits per partition key no longer trips it. Measured on
your shape, with the opt-in off: before, no `GroupPartitionsExec` and 2
shuffles; after, 1 `GroupPartitionsExec` and no shuffle. I will check
`allClusterKeysCovered` against the tightened notion explicitly and report what
I find on that PR, and link it here.
--
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]
