-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Optimize CASE expressions by removing WHEN false branches #17628
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Done! Can you also put a note in the PR description about what issue this fixes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest looking at this recent PR for a good reference on how to implement this: #17602
expr: None, | ||
when_then_expr, | ||
else_expr, | ||
}) if !when_then_expr.is_empty() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this if guard make the below case arm unreachable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think so
In general I think we could combine this branch with the one above (that checks for a single true
clause) 🤔
So the logic would be
- if the
WHEN
exprs have any true or false literals - then rewrite the when_then expr, removing all
false
literals and any arms after the firsttrue
Thank you for this PR @SidChaudhary7 It seems there are a few failing tests -- can you look into them? |
Once we merge this pr from @jackkleeman
If @SidChaudhary7 doesn't have time by tomorrow, I'll try and push it along |
I don't mind taking it if you'd rather save your time for other things. Should be pretty straightforward for me since I did the previous WHEN TRUE PR. I'll leave that up to you @alamb |
Which issue does this PR close?
CASE WHEN false THEN ...
#17590Expr::Case
expressions #1693Rationale for this change
This PR implements an optimization for CASE expressions that removes unreachable
WHEN false
branches and handlesWHEN true
cases that make subsequent branches unreachable.For example:
Are there any user-facing changes?
No
This is my first contribution to DataFusion. Could a committer please trigger the CI tasks?
@alamb @andygrove @tustvold @ursabot