-
Notifications
You must be signed in to change notification settings - Fork 600
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
feat(planner): integrate with PlanRoot #726
Conversation
Codecov Report
@@ Coverage Diff @@
## main #726 +/- ##
============================================
+ Coverage 72.35% 72.42% +0.06%
Complexity 2758 2758
============================================
Files 911 911
Lines 52925 52953 +28
Branches 1783 1783
============================================
+ Hits 38295 38351 +56
+ Misses 13738 13710 -28
Partials 892 892
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
b2ce791
to
1731f5e
Compare
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
rust/frontend/src/optimizer/mod.rs
Outdated
let (exprs, expr_aliases) = self | ||
.out_fields | ||
.ones() | ||
.zip_eq(self.schema.fields) | ||
.map(|(index, field)| { | ||
( | ||
InputRef::new(index, field.data_type).to_expr_impl(), | ||
Some(field.name), | ||
) | ||
}) | ||
.unzip(); |
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 I must have read this code piece somewhere..., Take a look at impl ColPrunable for LogicalFilter {
. You can do refactoring in another pr if they are repeated.
What's changed and what's your intention?
The planner will return
PlanRoot
rather thanPlanRef
for insert and query, as it is the entry point for the optimization phase. Additionally,PlanRoot::as_subplan
would transform it back in cases where it is actually a subtree of a bigger plan rather than the root.Checklist
Refer to a related PR or issue link (optional)