refactor(binder): refactor BoundSelect #2220
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2220 +/- ##
==========================================
+ Coverage 71.00% 71.03% +0.02%
==========================================
Files 654 654
Lines 82894 82892 -2
==========================================
+ Hits 58862 58879 +17
+ Misses 24032 24013 -19
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
| /// The names returned by this [`BoundSelect`]. | ||
| pub fn names(&self) -> Vec<String> { | ||
| self.aliases | ||
| pub fn create( |
There was a problem hiding this comment.
Feels unnecessary to have a separate pub create. The only place to create BoundXxx is always bind_xxx.
| /// The types returned by this [`BoundQuery`]. | ||
| pub fn data_types(&self) -> Vec<DataType> { | ||
| self.body.data_types() | ||
| self.schema().data_types() | ||
| } |
There was a problem hiding this comment.
Shall we remove this to be consistent with others?
There was a problem hiding this comment.
I think it might be ok, because the BoundQuery is the top of the Bound tree.
There was a problem hiding this comment.
Not a big deal. But someone may still add names on BoundQuery or data_types+names on BoundSetExpr/... later for the same reason: .data_types() is shorter than .schema().data_types(). Anyways we still achieve the goal on making select.schema consistent with select_exprs and aliases.
What's changed and what's your intention?
BoundQueryBoundSetExprBoundSelectBoundValuesnamesdata_typesand other method for the above structBoundSelect, and ban constructing it with values directly, for itsschemaandselect_itemsshould be consistentChecklist
Refer to a related PR or issue link (optional)
#1958