-
Notifications
You must be signed in to change notification settings - Fork 104
New planner rule added to push predicates down in the query representation #3324
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
New planner rule added to push predicates down in the query representation #3324
Conversation
...om/apple/foundationdb/record/query/plan/cascades/predicates/PredicateWithValueAndRanges.java
Outdated
Show resolved
Hide resolved
...c/main/java/com/apple/foundationdb/record/query/plan/cascades/predicates/QueryPredicate.java
Outdated
Show resolved
Hide resolved
...main/java/com/apple/foundationdb/record/query/plan/cascades/rules/PredicatePushDownRule.java
Outdated
Show resolved
Hide resolved
...main/java/com/apple/foundationdb/record/query/plan/cascades/rules/PredicatePushDownRule.java
Outdated
Show resolved
Hide resolved
...main/java/com/apple/foundationdb/record/query/plan/cascades/rules/PredicatePushDownRule.java
Outdated
Show resolved
Hide resolved
...main/java/com/apple/foundationdb/record/query/plan/cascades/rules/PredicatePushDownRule.java
Outdated
Show resolved
Hide resolved
...main/java/com/apple/foundationdb/record/query/plan/cascades/rules/PredicatePushDownRule.java
Outdated
Show resolved
Hide resolved
...main/java/com/apple/foundationdb/record/query/plan/cascades/rules/PredicatePushDownRule.java
Outdated
Show resolved
Hide resolved
...main/java/com/apple/foundationdb/record/query/plan/cascades/rules/PredicatePushDownRule.java
Show resolved
Hide resolved
…ownRule an exploratory rule and then modifies the memoizer logic to allow a collection of expressions to all be memoized together.
…sions during predicate push down rule
graphExpansionBuilder.addResultColumn(resultColumn(nameValue, "nameNew")); | ||
graphExpansionBuilder.addResultColumn(resultColumn(restNoValue, "restNoNew")); | ||
qun = Quantifier.forEach(Reference.of(graphExpansionBuilder.build().buildSelect())); | ||
qun = forEach(selectWithPredicates(qun, |
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.
This change here shows how the new utility methods can make the existing tests more concise. I only updated this one and testSimpleExistentialPredicateOnSimpleIndex
as a proof of concept, as shortening the rest isn't really a goal in this PR, but you can get similar results with the other tests here
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.
Want to create an issue for that?
This introduces a new query planner rule that will be a part of a larger framework to canonicalize query representations. The rule pushes down predicates to ensure that they appear as low in the tree as possible. So, for example, suppose you had a query like:
This will produce a new query expression where the
d = ?
is pushed lower in the graph, something like:This simplification helps us during planning to push down predicate execution to the lowest part of the graph where they can go.
Note that for the moment, this rule has not been added to the default
PlanningRuleSet
. The reason for this is that some of the rewrite rules will have weird overlap with some of the other rules, and so we want to rewrite the query in one phase and then plan and optimize it in another. The framework for that is not there yet