-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
Support materialized and aliased columns in joins #25634
Support materialized and aliased columns in joins #25634
Conversation
082cc4d
to
2949cd1
Compare
CI failures on
Details
Details
|
src/Interpreters/JoinedTables.h
Outdated
@@ -30,14 +30,13 @@ class JoinedTables | |||
} | |||
|
|||
StoragePtr getLeftTableStorage(); | |||
bool resolveTables(); | |||
bool resolveTables(bool include_all_columns); |
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 ever get called with different flag values for the same JoinedTables object? Maybe we could make it a const field of JoinedTables instead.
ContextPtr context, | ||
bool include_all) |
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'd make a ColumnTypeBit enum { Normal, Alias, Materialized } and pass here a bitset which_columns
parameterized by this enum, built by caller. Might be easier to understand what it is ultimately going to return. Not sure.
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 overall
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Support materialized and aliased columns in joins, close #13274