-
Notifications
You must be signed in to change notification settings - Fork 3.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
opt: improve various composite-ness checks #55194
Conversation
Use `CanBeCompositeSensitive` instead of disallowing any filters involving composite columns. Release note: None
Release note: None
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.
Reviewed 2 of 2 files at r1, 11 of 11 files at r2, 2 of 2 files at r3.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @mgartner and @RaduBerinde)
pkg/sql/opt/norm/join_funcs.go, line 337 at r3 (raw file):
allowCompositeEncoding := !memo.CanBeCompositeSensitive(c.mem.Metadata(), src) // Map each column in src to one column in dst. We choose an arbitrary column
[nit] add a blank line above this
pkg/sql/opt/norm/join_funcs.go, line 408 at r3 (raw file):
// // TODO(rytaft): In the future, we may want to allow the mapping if the // filter involves a comparison operator, such as x < 5.
you can remove this TODO :)
(and update the comment)
Use CanBeCompositeSensitive to allow remapping insensitive join filters. Release note: None
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.
TFTR!
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @mgartner and @rytaft)
pkg/sql/opt/norm/join_funcs.go, line 337 at r3 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
[nit] add a blank line above this
Done.
pkg/sql/opt/norm/join_funcs.go, line 408 at r3 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
you can remove this TODO :)
(and update the comment)
Nice catch, done!
5bc1751
to
d215271
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.
Reviewed 1 of 1 files at r4.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @mgartner)
bors r+ |
Build failed (retrying...): |
Build succeeded: |
opt: improve composite check for PushFilterIntoSetOp
Use
CanBeCompositeSensitive
instead of disallowing any filtersinvolving composite columns.
Release note: None
opt: improve composite check for Project FDs
Release note: None
opt: improve composite check for join filter remapping
Use CanBeCompositeSensitive to allow remapping insensitive join
filters.
Release note: None