-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Support non-tuple expression for in-subquery to join #4725
Conversation
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.
A great improvement to me.👍 Thanks @ygf11
|
||
#[tokio::test] | ||
async fn reduce_where_in_to_expr_equijoin() -> Result<()> { | ||
let ctx = create_join_context("t1_id", "t2_id", false)?; |
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.
👍
let (on, filter) = | ||
// When left is a constant expression, like 1, | ||
// the join predicate will be `1 = right_key`, it is better to add it to filter. | ||
if left_key.to_columns()?.is_empty() { | ||
let equi_expr = | ||
Expr::eq(*expr.clone(), Expr::Column(right_key)); | ||
(vec![], Some(equi_expr)) | ||
} else { |
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.
IMHO, I think it's a good improvement but maybe isn't suitable to put in this rule.🤔
It could be better to handle separate/handle on/joinfilter
in a new rule(we also can add more improvement in it).
Just a thought, I think we also can add a TODO, and handle them as a future ticket.
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.
It could be better to handle separate/handle on/joinfilter in a new rule(we also can add more improvement in it).
Agree.
There is another pr #4711 which will separate equi and non-equi predicate from filter.
We can add the left-key = right-key
to join-filter in this rule, and let the other rule do the separating works.
Got the wrong person 😆 |
why not implement this in the rule The two rule of |
Yes, I add it in new PR #4731 |
Close this pr, and I will submit a new pr based on |
Which issue does this PR close?
Closes #4724.
Rationale for this change
Currently datafusion support rewrite in-subquery to join when the expression is a column, like:
but following sql does not works:
We can rewrite these subquery to join also.
What changes are included in this PR?
In
SubqueryFilterToJoin
:where x+1 in (...)
to equi-join.where 1 in (..)
to non equi-join.Are these changes tested?
Yes
Are there any user-facing changes?