LuciferYang commented on code in PR #58419:
URL: https://github.com/apache/spark/pull/58419#discussion_r4027816125
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/AdaptiveSparkPlanExec.scala:
##########
@@ -134,7 +134,15 @@ case class AdaptiveSparkPlanExec(
Seq(
CoalesceBucketsInJoin,
RemoveRedundantProjects,
+ // Must run before `ensureRequirements`, which asks a `UnionExec` what
it reports: it
+ // records the conf that answer depends on, so the following
`StampUnionDecisions` freezes the
+ // decision under the same value the exchanges were planned against.
+ SnapshotUnionOutputPartitioningConf,
Review Comment:
Added in `AdaptiveQueryExecSuite`: `queryStagePreparationRules` is now
`private[adaptive]`, and the case asserts the snapshot sits immediately before
`EnsureRequirements`, the stamp immediately after it, and the trailing barrier
last in the list.
My first version took only the first stamp index, which the trailing barrier
satisfies on its own, so deleting the load-bearing one left the test green.
Collecting every index closes that.
A behavioural discriminator is not reachable single-threaded. With the two
rules adjacent, the only way the answers diverge is another thread writing the
conf inside that one-rule window, so what the case pins is the order, as the
non-AQE half does.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -2938,7 +2938,9 @@ object SQLConf {
.doc("When both this conf and `spark.sql.codegen.wholeStage` are true, "
+
"UnionExec participates in whole-stage codegen on its " +
"non-partitioning-aware path: the parent and all children fuse into " +
- "a single WholeStageCodegenExec stage.")
+ "a single WholeStageCodegenExec stage. The value is read when a
UnionExec's " +
+ "decision is fixed during physical preparation, so a change does not
reach a " +
Review Comment:
Fixed, and four more cases in the same suite had the same shape: BHJ child
union, the BHJ case feeding a downstream aggregate, SMJ child union, and column
pruning. Each compared a plan's output against its own, since the expected
value came from the plan the flag-on preparation had already stamped.
All five now build a fresh DataFrame per flag value through
`assertFlagParity`, which takes a builder for exactly this reason, and the
helper itself now asserts the flag-off half took no union into codegen, so
every parity case gets that.
That assertion needed a discriminator the suite did not have. Asking whether
a codegen stage holds a `UnionExec` somewhere is true even when
`CollapseCodegenStages` left it under an `InputAdapter`, taking no part in
codegen, and `fusedUnions` asks for the union to root its own stage, which
misses one fused into a parent operator's stage: the aggregate case is that
shape. The new `codegenUnions` walks each stage and stops at `InputAdapter`,
and it replaced the former `unionInsideWSCG` at every call site, several of
which were asserting the weaker property while their names claimed fusion.
--
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]
