Skip to content
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

Fix incorrect predicate precedence involving OR and split predicates #12078

Merged
merged 1 commit into from
Apr 22, 2022

Conversation

wendigo
Copy link
Contributor

@wendigo wendigo commented Apr 21, 2022

If constraintExpressions contain translated OR expression and there is a split predicate, resulting predicate could be incorrect: predicate1 OR predicate2 AND splitPredicate. Instead we need to wrap both sides of OR expression with parens:
(predicate1 OR predicate2) AND splitPredicate

@cla-bot cla-bot bot added the cla-signed label Apr 21, 2022
@wendigo wendigo requested a review from findepi April 21, 2022 12:33
@wendigo wendigo force-pushed the serafin/fix-getAdditionalPredicate branch from 61239a9 to 16fa861 Compare April 21, 2022 12:41
@wendigo wendigo requested a review from findepi April 21, 2022 12:42
@wendigo wendigo force-pushed the serafin/fix-getAdditionalPredicate branch from 16fa861 to d4fdd76 Compare April 21, 2022 14:36
@wendigo wendigo requested a review from findepi April 21, 2022 14:36
@wendigo wendigo force-pushed the serafin/fix-getAdditionalPredicate branch from d4fdd76 to 56d8d19 Compare April 21, 2022 16:35
@findepi findepi merged commit 365aa9a into trinodb:master Apr 22, 2022
@findepi findepi added the no-release-notes This pull request does not require release notes entry label Apr 22, 2022
@wendigo wendigo deleted the serafin/fix-getAdditionalPredicate branch April 22, 2022 10:59
@wendigo wendigo restored the serafin/fix-getAdditionalPredicate branch April 22, 2022 10:59
@wendigo wendigo deleted the serafin/fix-getAdditionalPredicate branch April 22, 2022 10:59
@github-actions github-actions bot added this to the 379 milestone Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed no-release-notes This pull request does not require release notes entry
Development

Successfully merging this pull request may close these issues.

2 participants