Support inferring new predicates to push down #15906
Draft
+124
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
logical-expr
xudong963
commented
Apr 30, 2025
| logical_plan | ||
| 01)Inner Join: t1.a = t2.a | ||
| 02)--Projection: t1.a, t1.b | ||
| 03)----Filter: __common_expr_4 >= Int64(3) AND __common_expr_4 <= Int64(5) |
There was a problem hiding this comment.
The inferred predicate, which can be pushed down to the t1 scan.
xudong963
commented
Apr 30, 2025
| /// Infers new predicates by substituting equalities. | ||
| /// For example, with predicates `t2.b = 3` and `t1.b > t2.b`, | ||
| /// we can infer `t1.b > 3`. | ||
| fn infer_predicates_from_equalities(predicates: Vec<Expr>) -> Result<Vec<Expr>> { |
There was a problem hiding this comment.
In the future, we can move the code into a dedicated optimizer rule, such as InferPredicates
There was a problem hiding this comment.
I think this might be a special case of the range analysis code in
https://docs.rs/datafusion/latest/datafusion/physical_expr/intervals/cp_solver/index.html
In other words, instead of this special case maybe we could use the cp_solver to create a more general framework for introducing inferred predicates 🤔
Now that we have predicate pushdown for ExecutionPlans maybe it is more realistic to do this
There was a problem hiding this comment.
I'll check the cp_solver (didn't notice the part of code before)
There was a problem hiding this comment.
Great suggestion. I can help if you need some directions or have any confusion
xudong963
commented
Apr 30, 2025
| @@ -131,13 +131,25 @@ pub fn normalize_sorts( | |||
| } | |||
|
|
|||
| /// Recursively replace all [`Column`] expressions in a given expression tree with | |||
| /// `Column` expressions provided by the hash map argument. | |||
| pub fn replace_col(expr: Expr, replace_map: &HashMap<&Column, &Column>) -> Result<Expr> { | |||
There was a problem hiding this comment.
I don't wanna write a similar method for the PR, so made the method generic
pub fn replace_col_with_expr(
expr: Expr,
replace_map: &HashMap<Column, &Expr>,
) -> Result<Expr> {
expr.transform(|expr| {
Ok({
if let Expr::Column(c) = &expr {
match replace_map.get(c) {
Some(new_expr) => Transformed::yes((**new_expr).to_owned()),
None => Transformed::no(expr),
}
} else {
Transformed::no(expr)
}
})
})
.data()
}
|
Perhaps running clickbench or equivalent (assuming clickbench wouldn't trigger this optimization) to showcase the difference would be good? |
|
🤖 |
|
🤖: Benchmark completed Details
|
|
Marking as draft as I think this PR is no longer waiting on feedback and I am trying to make it easier to find PRs in need of review. Please mark it as ready for review when it is ready for another look |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Rationale for this change
We can infer new predicates from existing predicates to push down to reduce IO and improve performance dramatically
What changes are included in this PR?
Infers new predicates by substituting equalities.
For example, with predicates
t2.b = 3andt1.b > t2.b, we can infert1.b > 3.See sqllogictest for another case.
Are these changes tested?
Yes
Are there any user-facing changes?