ulysses-you commented on code in PR #58338:
URL: https://github.com/apache/spark/pull/58338#discussion_r3872136536
##########
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:
[Non-blocking] `isNarrowed && !isGrouped` is a proxy for "the narrowing
collapsed distinct keys", but `!isGrouped` also holds when a source natively
reports multiple splits per partition key (e.g. uncommitted Iceberg files under
an `(a, b, c)` partitioning). In that case a projection keeping exactly the
operator's clustering `(a, b)` need not collapse any projected key, and
grouping would merge only same-key partitions - precisely what
`GroupPartitionsExec` does for any un-narrowed ungrouped KP - yet this guard
now refuses it even for `requireAllClusterKeysForDistribution=true`, inserting
an avoidable shuffle (results unchanged).
Peer code showing the intended tolerance:
`createKeyedShuffleSpec.allClusterKeysCovered` in `EnsureRequirements.scala`
(~lines 794-809) says "Key order and duplicated cluster keys don't matter" for
the same skew concern; after this change `groupedSatisfies` tolerates
duplicated keys strictly less than that adjacent co-partitioning gate.
OK to defer tightening to the follow-up, but please make sure it reconciles
with `allClusterKeysCovered` so both SPJ skew gates share one notion of
"collapsed keys".
--
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]
