Skip to content

[SPARK-39249][SQL] Improve subexpression elimination for conditional expressions - #36626

Closed
WangGuangxin wants to merge 2 commits into
apache:masterfrom
WangGuangxin:SPARK-39249
Closed

[SPARK-39249][SQL] Improve subexpression elimination for conditional expressions#36626
WangGuangxin wants to merge 2 commits into
apache:masterfrom
WangGuangxin:SPARK-39249

Conversation

@WangGuangxin

@WangGuangxinWangGuangxin commented May 21, 2022

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Currently we can do subexpression elimination for conditional expressions when the subexpression is common across all branchGroups. In fact, we can farther improve this when there are common expressions between alwaysEvaluatedInputs and branchGroups.

Why are the changes needed?

Take the following case as an example

IF(IsNull(a), b, KnowNotNull(a))

amay miss subexpression elimination chances since it is not the common expression between all branchGroups, but it's safe to evaluate a as common subexpression and eagerly execute it since it's part of the prediction, which will always be executed. If a is a time-expensive expression, we may waste time on running it.

This kind of expressions are common when we do sum on decimal type because of #29026

Many queries on TPC-DS has positive improvement. Following is some obvious improvements on TPC-DS 10T

QueryWith this PRWithout this PRSpeed up
4310.862635.299104.37%
4447.85163.71733.16%
50188.106217.02313.32%
8036.72346.00625.28%
93193.26224.13513.78%
95102.811126.12518.48%

Does this PR introduce any user-facing change?

No

How was this patch tested?

add more UT.

@WangGuangxin

Copy link
Copy Markdown
ContributorAuthor

@viirya@cloud-fan Could you please help review this?

@AmplabJenkins

Copy link
Copy Markdown

Can one of the admins verify this patch?

@Kimahriman

Kimahriman commented May 21, 2022

Copy link
Copy Markdown
Contributor

FYI I created #32987 a while ago to address this in a more general way. I've tried to keep it up to date, but there seemed to be concerns about creating a subexpression for something that might only execute once I guess? Even though that's already happening in certain cases

@WangGuangxin

Copy link
Copy Markdown
ContributorAuthor

@viirya@cloud-fan Could you please help review this?

@github-actions

Copy link
Copy Markdown

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@WangGuangxin
WangGuangxin deleted the SPARK-39249 branch August 26, 2026 23:27
Sign up for freeto 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.

3 participants

@WangGuangxin@AmplabJenkins@Kimahriman