[SPARK-57881][SQL] UnionExec outputPartitioning supports KeyedPartitioning#56961
[SPARK-57881][SQL] UnionExec outputPartitioning supports KeyedPartitioning#56961pan3793 wants to merge 2 commits into
Conversation
|
cc @peter-toth, could you please take a look? |
| case a: Attribute if attributeMap.contains(a) => attributeMap(a) | ||
| }) | ||
| val isGrouped = mergedKeys.distinct.size == mergedKeys.size | ||
| KeyedPartitioning(mergedExpressions, mergedKeys, isGrouped) |
There was a problem hiding this comment.
The 3-arg constructor leaves isNarrowed = false, so the merge drops the flag whenever a child KeyedPartitioning is narrowed (e.g. a union leg that is a ProjectExec/aggregate narrowing a composite partition key -- ProjectExec is a PartitioningPreservingUnaryExecNode, basicPhysicalOperators.scala:49). comparePartitioning only matches on expressions, so a narrowed child and a non-narrowed child with the same expressions are still considered compatible and merged here.
AliasAwareOutputExpression.scala:135-139 calls this flag out as needing to be "sticky ... a subsequent [node] that passes all positions through would otherwise recompute isNarrowed=false, silently dropping the protection" -- this merge is exactly that dropping. The consequence is in KeyedPartitioning.groupedSatisfies (partitioning.scala:588-593): a narrowed, non-grouped KP is meant to satisfy ClusteredDistribution only under v2BucketingAllowKeysSubsetOfPartitionKeys, precisely because GroupPartitionsExec will then merge partitions that held distinct keys in the finer partitioning (skew risk). Reporting isNarrowed=false lets that SPJ proceed without the opt-in.
Propagate the flag from the children:
| KeyedPartitioning(mergedExpressions, mergedKeys, isGrouped) | |
| val isNarrowed = partitionings.exists { | |
| case k: KeyedPartitioning => k.isNarrowed | |
| case _ => false | |
| } | |
| KeyedPartitioning(mergedExpressions, mergedKeys, isGrouped, isNarrowed) |
There was a problem hiding this comment.
@peter-toth, thank you for pointing out this correctness issue, applied this fix.
| } | ||
| } | ||
|
|
||
| test("SPARK-57881: storage-partitioned join leverages union output KeyedPartitioning to " + |
There was a problem hiding this comment.
All four tests partition by identity("id"). A variant over a bucket transform (Array(bucket(4, "id"))) would cover the PR's motivating bucketed-source case end-to-end.
There was a problem hiding this comment.
added a new test SPARK-57881: storage-partitioned join over union: bucket transform partitioning
LGTM, one test-coverage suggestion (non-blocking)Traced the merge end to end and the core is sound - the partition-index-to-key alignment holds: One gap worth closing: none of the added tests exercise a union child whose partitions are all pruned (an empty / What I verified:
|
|
@yadavay-amzn, thank you for the review, I added a new test case |
yadavay-amzn
left a comment
There was a problem hiding this comment.
LGTM, thanks @pan3793 !
…oning ### What changes were proposed in this pull request? Extend `spark.sql.unionOutputPartitioning` to `KeyedPartitioning`, so a `UNION ALL` of V2 storage-partitioned tables can do a shuffle-free SPJ. Changes in `UnionExec`: - `comparePartitioning`: add a `KeyedPartitioning` case matching on partition *expressions* (keys not compared — children carry different key sets that are merged below). - `outputPartitioning`: when all children report compatible `KeyedPartitioning`s, emit a merged one whose `partitionKeys` is the concatenation of the children's keys (one per physical partition), recomputing `isGrouped` and propagating `isNarrowed` (sticky: set if any child is narrowed). - `doExecute`: a `KeyedPartitioning` union uses the plain concatenating `UnionRDD`, not the index-based `SQLPartitioningAwareUnionRDD` (which is only correct for `HashPartitioning`/`SinglePartition` and would mix keys). The merged descriptor is consumed by the existing SPJ logic in `EnsureRequirements`: duplicate keys (overlapping child sets, `isGrouped=false`) are coalesced by `GroupPartitionsExec`; join-leg key-set mismatches are reconciled via the push-part-values path. ### Why are the changes needed? A `UNION ALL` of V2 bucketed tables reports `UnknownPartitioning` today, so any join on the partition key shuffles — defeating SPJ for sharded/bucketed sources. ```sql SELECT /*+ MERGE(u, t3) */ u.id, u.data, t3.data FROM (SELECT id, data FROM t1 UNION ALL SELECT id, data FROM t2) u JOIN t3 ON u.id = t3.id; ``` Before: both sides shuffle. After: the union reports a merged `KeyedPartitioning` over `id`; SPJ runs shuffle-free (with `GroupPartitionsExec` coalescing overlapping keys). ### Does this PR introduce _any_ user-facing change? Results are unchanged. The executed plan may drop a previously inserted shuffle. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: GLM 5.2 Closes #56961 from pan3793/SPARK-57881. Authored-by: Cheng Pan <pan3793@gmail.com> Signed-off-by: Cheng Pan <chengpan@apache.org> (cherry picked from commit f8ee9ef) Signed-off-by: Cheng Pan <chengpan@apache.org>
|
thank you @peter-toth @yadavay-amzn, merged |
What changes were proposed in this pull request?
Extend
spark.sql.unionOutputPartitioningtoKeyedPartitioning, so aUNION ALLof V2 storage-partitioned tables can do a shuffle-free SPJ.Changes in
UnionExec:comparePartitioning: add aKeyedPartitioningcase matching on partition expressions (keys not compared — children carry different key sets that are merged below).outputPartitioning: when all children report compatibleKeyedPartitionings, emit a merged one whosepartitionKeysis the concatenation of the children's keys (one per physical partition), recomputingisGroupedand propagatingisNarrowed(sticky: set if any child is narrowed).doExecute: aKeyedPartitioningunion uses the plain concatenatingUnionRDD, not the index-basedSQLPartitioningAwareUnionRDD(which is only correct forHashPartitioning/SinglePartitionand would mix keys).The merged descriptor is consumed by the existing SPJ logic in
EnsureRequirements: duplicate keys (overlapping child sets,isGrouped=false) are coalesced byGroupPartitionsExec; join-leg key-set mismatches are reconciled via the push-part-values path.Why are the changes needed?
A
UNION ALLof V2 bucketed tables reportsUnknownPartitioningtoday, so any join on the partition key shuffles — defeating SPJ for sharded/bucketed sources.Before: both sides shuffle. After: the union reports a merged
KeyedPartitioningoverid; SPJ runs shuffle-free (withGroupPartitionsExeccoalescing overlapping keys).Does this PR introduce any user-facing change?
Results are unchanged. The executed plan may drop a previously inserted shuffle.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: GLM 5.2