Skip to content

Conversation

@heyihong
Copy link
Contributor

@heyihong heyihong commented Jan 5, 2026

What changes were proposed in this pull request?

This PR simplifies the foreachWithSubqueries method in QueryPlan.scala by:

  1. Directly traversing both subqueries and children using the same traverse function
  2. Replacing the indirect foreach(actualFunc) call with a direct traverse(this) call

Why are the changes needed?

The original implementation was unnecessarily complex, using foreach to traverse children implicitly while recursively calling foreachWithSubqueries for subqueries. The new implementation is simpler and more straightforward.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests for query plan traversal should cover this change.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Cursor 2.2.44

@github-actions github-actions bot added the SQL label Jan 5, 2026
@github-actions
Copy link

github-actions bot commented Jan 5, 2026

JIRA Issue Information

=== Sub-task SPARK-54905 ===
Summary: Simplify foreachWithSubqueries implementation in QueryPlan
Assignee: None
Status: Open
Affected: ["4.2.0"]


This comment was automatically generated by GitHub Actions

@heyihong
Copy link
Contributor Author

heyihong commented Jan 5, 2026

@cloud-fan

@heyihong heyihong force-pushed the SPARK-54905 branch 2 times, most recently from 8c18ce2 to 3e962d1 Compare January 5, 2026 16:04
@heyihong heyihong changed the title [SPARK-54905][SQL] Simplify the traversal implementation of foreachWithSubqueries [SPARK-54905][SQL] Add comments to foreachWithSubqueries Jan 5, 2026
@heyihong heyihong requested a review from cloud-fan January 5, 2026 22:06
@heyihong heyihong force-pushed the SPARK-54905 branch 2 times, most recently from 5b45be0 to d17748c Compare January 5, 2026 22:34
@heyihong heyihong changed the title [SPARK-54905][SQL] Add comments to foreachWithSubqueries [SPARK-54905][SQL] Simplify foreachWithSubqueries implementation in QueryPlan Jan 7, 2026
@heyihong heyihong force-pushed the SPARK-54905 branch 3 times, most recently from 10f10da to b94fa1a Compare January 7, 2026 14:25
@heyihong heyihong closed this Jan 7, 2026
@heyihong heyihong reopened this Jan 7, 2026
@heyihong
Copy link
Contributor Author

heyihong commented Jan 7, 2026

Discussed with @cloud-fan offline, while the current convention is to let TreeNode methods to handle plan traversal logic (including withPruning), for foreachWithSubqueries, a clearer implementation should avoid doing so.

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 442f908 Jan 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants