-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix optimizer regression with simplifying expressions in subquery filters #3764
Conversation
@@ -144,6 +144,10 @@ impl Optimizer { | |||
Arc::new(DecorrelateWhereIn::new()), | |||
Arc::new(ScalarSubqueryToJoin::new()), | |||
Arc::new(SubqueryFilterToJoin::new()), | |||
// simplify expressions does not simplify expressions in subqueries, so we |
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 think it's a solution to resolve this.
But We also has a way to resolve this issue by supporting the subquery
in the rewriter of simplify expression
.
Why not support the subquery
in the rewriter of simplify expression?
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.
This PR just reverts the regression and adds a regression test, but I agree, would be better to handle it in simplify_expressions
. I filed #3770 for this.
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 agree #3770 is a better long term solution
LGTM thanks |
Thanks for the review @liukun4515 |
Benchmark runs are scheduled for baseline = e395e30 and contender = 8022827. 8022827 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #3760
Rationale for this change
Fixes regression introduced in #3636 where we stopped running SimplifyExpressions a second time after converting subqueries to joins, meaning that we stopped simplifying expressions in subquery filters.
What changes are included in this PR?
Are there any user-facing changes?