Skip to content

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

Merged
merged 22 commits into from
May 21, 2025

Conversation

alecgrieser
Copy link
Collaborator

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:

SELECT a, b, c
   FROM (SELECT a, b, c, d FROM T)
   WHERE d = ?
   ORDER BY a, b

This will produce a new query expression where the d = ? is pushed lower in the graph, something like:

SELECT a, b, c
   FROM (SELECT a, b, c, d FROM T where d = ?)
   ORDER BY a, b

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

@alecgrieser alecgrieser added the enhancement New feature or request label Apr 28, 2025
@alecgrieser alecgrieser changed the title Create planner rule to push predicates down in the query representation New planner rule added to push predicates down in the query representation Apr 28, 2025
@alecgrieser alecgrieser marked this pull request as ready for review April 28, 2025 13:23
@alecgrieser alecgrieser requested a review from normen662 April 28, 2025 13:23
graphExpansionBuilder.addResultColumn(resultColumn(nameValue, "nameNew"));
graphExpansionBuilder.addResultColumn(resultColumn(restNoValue, "restNoNew"));
qun = Quantifier.forEach(Reference.of(graphExpansionBuilder.build().buildSelect()));
qun = forEach(selectWithPredicates(qun,
Copy link
Collaborator Author

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

Copy link
Contributor

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?

@alecgrieser alecgrieser merged commit fba36f6 into FoundationDB:main May 21, 2025
5 checks passed
@alecgrieser alecgrieser deleted the canon-predicate-pushdown branch May 21, 2025 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants