-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat: improve LiteralGuarantee for the case like (a=1 AND b=1) OR (a=2 AND b=3)
#16762
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
Conversation
(a=1 AND b=1) OR (a=2 AND b=3)
(a=1 AND b=1) OR (a=2 AND b=3)
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.
Thanks @haohuaijin -- this looks like a great start to me
I think we need a few more tests to show it doesn't incorrectly pick up literal guarantees for NOT IN / !=
terms, but otherwise I think it is good
I am sorry @haohuaijin -- I will review this more carefully soon. I just need to sit down and think through the details to make sure it doesn't have any correctness problems |
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 @haohuaijin -- I reviewed the code and tests carefully and I think this PR looks good to me.
It is a very nice improvement
Thanks for you reviews @alamb |
Which issue does this PR close?
Rationale for this change
improve LiteralGuarantee to handle the case like
(a=1 AND b=1) OR (a=2 AND b=3)
or(a IN ("foo", "bar") AND b = 5) OR (a IN ("bar") AND b=6)
What changes are included in this PR?
add the logical to extract
(a=1 AND b=1) OR (a=2 AND b=3)
toin_guarantee("a", [1, 2]), in_guarantee("b", [1, 3])
;find_common_columns
function that identifies columns present in all termsetsAre these changes tested?
yes, add some test case
Are there any user-facing changes?