ulysses-you commented on PR #58942: URL: https://github.com/apache/spark/pull/58942#issuecomment-5776592901
Thanks @peter-toth, @dongjoon-hyun and @cloud-fan! Addressed in ce59a03fc24, which answers both comments above and the inline ones. The validator judges as-is layouts now. A keyed member contributes the layout it reports -- `KeyedShuffleSpec(k, distribution)`, with no projection onto the operation keys, no grouping and no re-sorting -- and is admitted on `satisfies`. The explicitly safe ungrouped case is the one exception: it is confined to the partially clustered producer configuration through a single predicate, `PartitioningCollection.mayUngroupedMember`, which `specsForPairing` and the per-side exemption both read, so the two cannot drift. The co-partitioning reading is `ShuffledJoin`-only, so `CoGroupExec` and `FlatMapCoGroupsInBatchExec` keep the per-side check. That closes the measured shapes: a projected key count no longer stands in for the child's own, so three partitions against two is refused; a projection onto distinct sorted keys no longer normalizes a key order; and an ungrouped side is admitted only where something builds one. Coverage for the two configurations asked for is in `ValidateRequirementsSuite`: subset-enabled/partial-disabled, where the projection's own layout passes and the source's keys, which no node projected, are refused, and a non-join operator, a cogroup over the pair a join reads, refused. Two consequences, stated rather than left implicit: - A join's distributions never pin `requiredNumPartitions` (`ShuffledJoin.scala:57`), so with the co-partitioning reading limited to the join, `specsForPairing`'s count clause is no longer reachable from the validator. The pinned-count test moved to `ShuffleSpecSuite`, as a direct test of the helper. - Where the operation's keys are a subset of the source's partitioning keys and the projection would merge no partition, `satisfies` admits the member but the pairing refuses it, the expression the operation does not cluster on mapping to no cluster key. The base accepted that shape through a projection it did not hold; here the plan has to hold the grouping node. This is the one place the validator is stricter than the base. The comment and doc sites raised inline are answered in their threads. -- 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]
