[SPARK-56805][SQL] Enable spark.sql.codegen.wholeStage.union.enabled by default#55765
Closed
LuciferYang wants to merge 3 commits into
Closed
[SPARK-56805][SQL] Enable spark.sql.codegen.wholeStage.union.enabled by default#55765LuciferYang wants to merge 3 commits into
spark.sql.codegen.wholeStage.union.enabled by default#55765LuciferYang wants to merge 3 commits into
Conversation
…suites - PlanStabilitySuite.testQuery: force spark.sql.codegen.wholeStage.union.enabled=false so approved TPCDS/TPCH plans stay stable under SPARK_SQL_WSCG_UNION_ENABLED=true. - SQLQueryTestSuite.sparkConf: same conf via builder, inherited by ThriftServerQueryTestSuite, to keep explain.sql golden output unchanged. - SQLMetricsSuite SPARK-25278: wrap the repeated-plan test with withSQLConf(... -> false); the expected map assumes the non-fused Union node, pairing with the existing fused-path test at line 742.
LuciferYang
commented
May 8, 2026
LuciferYang
commented
May 8, 2026
LuciferYang
changed the title
spark.sql.codegen.wholeStage.union.enabled=truespark.sql.codegen.wholeStage.union.enabled=true enabled by default in the change pipeline
LuciferYang
changed the title
spark.sql.codegen.wholeStage.union.enabled=true enabled by default in the change pipeline
LuciferYang
changed the title
…by default Flip WHOLESTAGE_UNION_CODEGEN_ENABLED default from false to true and regenerate the golden files affected by Union codegen fusion. Revert the earlier test-only pins in PlanStabilitySuite / SQLQueryTestSuite and the workflow env var injection. Keep the SQLMetricsSuite SPARK-25278 pin since its expected metric map asserts on the non-fused Union plan shape.
Contributor
Author
|
cc @cloud-fan |
HyukjinKwon
approved these changes
May 10, 2026
Member
|
master only yeah? |
Member
|
actually I wonder if we should just enable it later though to reduce the diff between master and branch-4.x. but I am fine with enabling now too - no strong opinon |
Contributor
Author
I suggest backporting this to also cc @cloud-fan WDYT? |
dongjoon-hyun
approved these changes
May 11, 2026
Member
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
+1 for this PR to enable this at Apache Spark 4.3.0 by default because we introduced this at 4.2.0 (disabled).
dongjoon-hyun
changed the title
spark.sql.codegen.wholeStage.union.enabled by default
Member
|
cc @peter-toth |
peter-toth
approved these changes
May 11, 2026
Contributor
Enabling it on both 4.3 and 5.0 makes sense to me. |
Member
|
ok I am fine 👍 |
LuciferYang
added a commit
that referenced
this pull request
May 12, 2026
…` by default ### What changes were proposed in this pull request? Flip the default value of `spark.sql.codegen.wholeStage.union.enabled` (introduced by SPARK-56482) from `false` to `true`, and update the golden files / metric assertions that were pinned on the "Union is not fused" assumption. Concrete changes: - `SQLConf.WHOLESTAGE_UNION_CODEGEN_ENABLED` default `false` → `true`. - Regenerate the affected approved plans under `PlanStabilitySuite` (TPCDS V1.4 / V2.7 / Modified, TPCH) where Union fusion changes the `WholeStageCodegen` boundary. - Regenerate the `explain.sql.out` golden file under `SQLQueryTestSuite` (the `explain-aqe.sql.out` formatted plan does not carry codegen ids and is unchanged). - Wrap the `SQLMetricsSuite` test "SPARK-25278: output metrics are wrong for plans repeated in the query" with `withSQLConf(WHOLESTAGE_UNION_CODEGEN_ENABLED -> "false")`. Its expected metric map assumes the non-fused 4-node physical layout (`Union -> Project -> 2 x LocalTableScan`); this is an assertion about plan shape rather than a golden file, so the explicit pin is retained. ### Why are the changes needed? The UnionExec whole-stage codegen path introduced by SPARK-56482 is currently disabled by default, so only a handful of cases that explicitly enable the conf exercise it on master CI, and follow-up changes can easily regress without being noticed. Flipping the default to `true` makes every Scala unit test, every golden-file / metric-driven suite (`PlanStabilitySuite`, `SQLQueryTestSuite`, `SQLMetricsSuite`, ...), the `tpcds-1g` end-to-end TPC-DS steps, and local development runs all exercise this path by default, providing steady, continuous regression coverage for UnionExec whole-stage codegen. ### Does this PR introduce _any_ user-facing change? Yes. The default value of `spark.sql.codegen.wholeStage.union.enabled` flips from `false` to `true`. Query semantics via the public API are unchanged (results, row ordering, and error behavior are all the same), but: - `UnionExec`'s non-partitioning-aware path now merges with its parent/child operators into the same `WholeStageCodegenExec` stage, so `EXPLAIN` output's stage boundaries and codegen ids change. - User code that asserts on the exact number of physical plan nodes or on the `WholeStageCodegen` boundary split (e.g., some tests or performance monitoring scripts) may need updates. - Users who want the previous behavior can explicitly `SET spark.sql.codegen.wholeStage.union.enabled = false`. ### How was this patch tested? CI, plus locally regenerating the `PlanStabilitySuite` and `SQLQueryTestSuite` golden files via `SPARK_GENERATE_GOLDEN_FILES=1` and manually reviewing the diffs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #55765 from LuciferYang/test-with-union-cg-true. Authored-by: YangJie <yangjie01@baidu.com> Signed-off-by: yangjie01 <yangjie01@baidu.com> (cherry picked from commit cd97891) Signed-off-by: yangjie01 <yangjie01@baidu.com>
Contributor
Author
|
Merged into master/branch-4.x. Thanks @HyukjinKwon @dongjoon-hyun and @peter-toth |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Flip the default value of
spark.sql.codegen.wholeStage.union.enabled(introduced by SPARK-56482) fromfalsetotrue, and update the golden files / metric assertions that were pinned on the "Union is not fused" assumption.Concrete changes:
SQLConf.WHOLESTAGE_UNION_CODEGEN_ENABLEDdefaultfalse→true.PlanStabilitySuite(TPCDS V1.4 / V2.7 / Modified, TPCH) where Union fusion changes theWholeStageCodegenboundary.explain.sql.outgolden file underSQLQueryTestSuite(theexplain-aqe.sql.outformatted plan does not carry codegen ids and is unchanged).SQLMetricsSuitetest "SPARK-25278: output metrics are wrong for plans repeated in the query" withwithSQLConf(WHOLESTAGE_UNION_CODEGEN_ENABLED -> "false"). Its expected metric map assumes the non-fused 4-node physical layout (Union -> Project -> 2 x LocalTableScan); this is an assertion about plan shape rather than a golden file, so the explicit pin is retained.Why are the changes needed?
The UnionExec whole-stage codegen path introduced by SPARK-56482 is currently disabled by default, so only a handful of cases that explicitly enable the conf exercise it on master CI, and follow-up changes can easily regress without being noticed. Flipping the default to
truemakes every Scala unit test, every golden-file / metric-driven suite (PlanStabilitySuite,SQLQueryTestSuite,SQLMetricsSuite, ...), thetpcds-1gend-to-end TPC-DS steps, and local development runs all exercise this path by default, providing steady, continuous regression coverage for UnionExec whole-stage codegen.Does this PR introduce any user-facing change?
Yes. The default value of
spark.sql.codegen.wholeStage.union.enabledflips fromfalsetotrue. Query semantics via the public API are unchanged (results, row ordering, and error behavior are all the same), but:UnionExec's non-partitioning-aware path now merges with its parent/child operators into the sameWholeStageCodegenExecstage, soEXPLAINoutput's stage boundaries and codegen ids change.WholeStageCodegenboundary split (e.g., some tests or performance monitoring scripts) may need updates.SET spark.sql.codegen.wholeStage.union.enabled = false.How was this patch tested?
CI, plus locally regenerating the
PlanStabilitySuiteandSQLQueryTestSuitegolden files viaSPARK_GENERATE_GOLDEN_FILES=1and manually reviewing the diffs.Was this patch authored or co-authored using generative AI tooling?
No.