-
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
Use ExprRewriter
in pre_cast_lit_in_comparison
#3260
Conversation
nice pr. |
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.
LGTM thanks @andygrove
@@ -318,6 +317,20 @@ mod tests { | |||
assert_eq!(optimize_test(expr_lt, &schema), expected); | |||
} | |||
|
|||
#[test] | |||
fn nested() { |
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 test require this pr to pass?
I think the original logic can handle this case
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.
No, this is just an additional test to make sure that this PR did not cause any regressions.
Codecov Report
@@ Coverage Diff @@
## master #3260 +/- ##
=======================================
Coverage 85.89% 85.89%
=======================================
Files 294 294
Lines 53373 53391 +18
=======================================
+ Hits 45845 45861 +16
- Misses 7528 7530 +2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Thanks for the review @liukun4515 |
Benchmark runs are scheduled for baseline = 92110dd and contender = 5ee52d0. 5ee52d0 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 #3259
Rationale for this change
This change makes the optimizer support nested expressions, including aliased expressions.
What changes are included in this PR?
Use
ExprRewriter
.Are there any user-facing changes?
More optimizations!