[SPARK-54850][SQL] Improve extractShuffleIds to find AdaptiveSparkPlanExec anywhere in plan tree
#53620
+4
−3
Conversation
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
…eSparkPlanExec` anywhere in plan tree
Member
|
@baibaichen SPARK-53413 is shipped with spark 4.1.0, you need re-file a JIRA ticket for the PR. |
baibaichen
changed the title
extractShuffleIds to find AdaptiveSparkPlanExec anywhere in plan treeextractShuffleIds to find AdaptiveSparkPlanExec anywhere in plan tree
Contributor
Author
SPARK-54850 is created for this PR |
cloud-fan
approved these changes
Dec 26, 2025
yaooqinn
changed the title
extractShuffleIds to find AdaptiveSparkPlanExec anywhere in plan treeextractShuffleIds to find AdaptiveSparkPlanExec anywhere in plan tree
Member
|
@baibaichen Can you retrigger the GHA? |
yaooqinn
pushed a commit
that referenced
this pull request
Dec 29, 2025
…PlanExec` anywhere in plan tree ### What changes were proposed in this pull request? This PR uses `collectFirst` to find the first `AdaptiveSparkPlanExec` node anywhere in the plan tree, instead of assuming the root plan is an `AdaptiveSparkPlanExec`. ### Why are the changes needed? #52157 introduced the `extractShuffleIds` method in `SQLExecution` to find shuffle IDs of `SparkPlan`. Previously, the method implicitly assumed that if AQE is enabled, the `AdaptiveSparkPlanExec` would be at the root of the input. Since Spark only inserts `AdaptiveSparkPlanExec` under Command, this assumption was fine. However, the `AdaptiveSparkPlanExec` may not be the root node in Gluten. Gluten needs to insert a special physical plan to do column to row transition. By using `collectFirst`, we can correctly locate the `AdaptiveSparkPlanExec` regardless of its position in the plan tree, which improves compatibility. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Pass GHA. ### Was this patch authored or co-authored using generative AI tooling? No Closes #53620 from baibaichen/feature/extractShuffleIds. Authored-by: Chang chen <baibaichen@gmail.com> Signed-off-by: Kent Yao <kentyao@microsoft.com> (cherry picked from commit 082de7f) Signed-off-by: Kent Yao <kentyao@microsoft.com>
Member
|
Merged to master and 4.1.1. Thank you @baibaichen @cloud-fan |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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?
This PR uses
collectFirstto find the firstAdaptiveSparkPlanExecnode anywhere in the plan tree, instead of assuming the root plan is anAdaptiveSparkPlanExec.Why are the changes needed?
#52157 introduced the
extractShuffleIdsmethod inSQLExecutionto find shuffle IDs ofSparkPlan. Previously, the method implicitly assumed that if AQE is enabled, theAdaptiveSparkPlanExecwould be at the root of the input. Since Spark only insertsAdaptiveSparkPlanExecunder Command, this assumption was fine. However, theAdaptiveSparkPlanExecmay not be the root node in Gluten. Gluten needs to insert a special physical plan to do column to row transition.By using
collectFirst, we can correctly locate theAdaptiveSparkPlanExecregardless of its position in the plan tree, which improves compatibility.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Pass GHA.
Was this patch authored or co-authored using generative AI tooling?
No