ulysses-you commented on code in PR #58339:
URL: https://github.com/apache/spark/pull/58339#discussion_r3910705128
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/physical/partitioning.scala:
##########
@@ -1678,7 +1751,18 @@ case class KeyedShuffleSpec(
// The shuffled side is laid out on this side's partition keys, so it
inherits the flag. That
// is conservative rather than strictly true, and it can only ever add a
shuffle: a later
// grouping of the shared key set carries the collapsed side's risk.
- partitioning.copy(expressions = newExpressions)
+ //
+ // The child re-shuffled onto this layout may hold keys outside the
declared set, so every
+ // partitioning produced here carries the unknown-keys marker too (see
+ // `KeyedPartitioning.mayContainUnknownPartitionKeys`).
`createPartitioning` is only reached
+ // from `EnsureRequirements`' shuffle loop to re-shuffle a join child onto
the best spec's
+ // layout, and the re-shuffled child's keys are never provably a subset of
the declared keys:
+ // a non-keyed (v1) child's keys are unknown to the planner, and a keyed
child with an
+ // incompatible partitioning is re-evaluated in this spec's key space,
which the planner
+ // cannot bound. Marking every such partitioning is therefore
+ // sound (conservative only when the two sides happen to share a transform
and the re-shuffled
+ // keys are a known subset).
+ partitioning.copy(expressions = newExpressions,
mayContainUnknownPartitionKeys = true)
Review Comment:
Fixed in 7464c25: `KeyedPartitioning.keysSatisfy` gives an unknown-keyed
partitioning no subset relaxation -- it keeps the full-key rules
(`requireAllClusterKeys` exact match, full coverage otherwise), so a window or
aggregate keyed on a strict subset of the partition columns falls back to a
shuffle. Co-location holds for whole keys only (equal undeclared keys hash to
the same partition), so full-key clustering stays sound unshuffled. Your repro
is the test `SPARK-59050: SPJ: a window keyed on a subset of an unknown-keyed
layout must shuffle` (measured on the pre-gate tree: the id=1 count split into
two groups of 1; run for AQE off and on), with `...a window keyed on the full
key of an unknown-keyed layout does not shuffle` as the positive control
pinning 1 shuffle for the complete key.
--
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]
