-
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
remove unalias
TableScan filters when create Physical Filter
#8404
Conversation
@@ -562,8 +562,7 @@ impl DefaultPhysicalPlanner { | |||
// doesn't know (nor should care) how the relation was | |||
// referred to in the query | |||
let filters = unnormalize_cols(filters.iter().cloned()); | |||
let unaliased: Vec<Expr> = filters.into_iter().map(unalias).collect(); |
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.
It's useless
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 we have to rely on our test coverage here, and given all the existing tests pass I agree!
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, existing tests already cover it.
BTW, theoretically and in terms of code, the alias in the filter expression does not affect the schema or output.
unalias
TableScan filtersunalias
TableScan filters in create PhysicalPlan
unalias
TableScan filters in create PhysicalPlanunalias
TableScan filters when create Physical Filter
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.
Thank you @jackwener -- this is a very nice PR 👏
@@ -562,8 +562,7 @@ impl DefaultPhysicalPlanner { | |||
// doesn't know (nor should care) how the relation was | |||
// referred to in the query | |||
let filters = unnormalize_cols(filters.iter().cloned()); | |||
let unaliased: Vec<Expr> = filters.into_iter().map(unalias).collect(); |
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 we have to rely on our test coverage here, and given all the existing tests pass I agree!
Which issue does this PR close?
Closes #.
Rationale for this change
unalias
TableScan filtersWhat changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?