ulysses-you commented on code in PR #59863:
URL: https://github.com/apache/spark/pull/59863#discussion_r4441770467
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/KeyGroupedPartitioningSuite.scala:
##########
@@ -5416,6 +5553,79 @@ class KeyGroupedPartitioningSuite
}
}
+ test("SPARK-60618: partially clustered join keeps its replicate-side choice
when the " +
+ "alignment is the identity") {
+ // The smaller side by statistics is replicated: `pc_small` holds five
splits in 200 bytes,
+ // `pc_large` three splits in 216. Both are bucketed, whose transform
derives no ordering, so
+ // the join's sort has to be added. The larger side ends up with one split
per bucket, so the
+ // alignment on it is the identity and the pass-through node has to stay:
a re-run peels a
+ // local sort only through a `GroupPartitionsExec`, and reading the
statistics from the scans
+ // again is what keeps the choice. Without the node the counts (five
against three) pick the
+ // other side.
Review Comment:
Updated in e1763809aca: the test now says that its scan leaf keeps the
identity node. I kept the replicate-side guard for plans with existing keyed
shuffle stages and noted that the planner does not create keyed shuffles when
partial clustering is enabled.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/GroupPartitionsExec.scala:
##########
@@ -114,9 +114,10 @@ case class GroupPartitionsExec(
* `plannedPartitioning` rebuilds its keyed members on top of the grouping.
A rule that hands this
* node a child reporting something else invalidates both, so the keyed
claim is dropped and only
* the physical output count is reported. `ValidateRequirements` then finds
the parent's
- * distribution unsatisfied and `AdaptiveSparkPlanExec` reverts the rewrite,
which is what an
- * `AQEShuffleReadExec` landing over a keyed shuffle stage relies on. Only
an `AQEShuffleReadRule`
- * is validated that way, so `checkChildStillMatches` covers the rest at
execution.
+ * distribution unsatisfied and `AdaptiveSparkPlanExec` reverts the rewrite.
A coalesced or
+ * skewed `AQEShuffleReadExec` over a keyed stage also reports
`UnknownPartitioning` itself, so
+ * validation can reject it even without this node. Only an
`AQEShuffleReadRule` is validated
+ * that way, so `checkChildStillMatches` covers the rest at execution.
Review Comment:
Fixed in e1763809aca: the documentation now describes coalesced reads only
and makes clear that a side whose identity node was skipped has no
execution-time child-partitioning check.
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/KeyGroupedPartitioningSuite.scala:
##########
@@ -1152,12 +1152,149 @@ class KeyGroupedPartitioningSuite
testWithCustomersAndOrders(customers_partitions, orders_partitions, 2, 0)
}
+ test("SPARK-60618: scan rebuilt with different partition keys after
alignment") {
Review Comment:
Fixed in e1763809aca: moved the scan-rebuild test next to SPARK-59252 and
the keyed-shuffle test next to the SPARK-41471 shuffle-one-side tests.
--
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]
