-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Allow non-equijoin filters in join condition #660
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
Conversation
houqp
left a comment
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.
👍
|
@alamb do you mind to take a look? |
alamb
left a comment
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.
Nice work @Dandandan -- the only other thing I would suggest is a negative case
Something like this with a LEFT join:
SELECT t1_id, t1_name, t2_name FROM t1 LEFT JOIN t2 ON t1_id = t2_id AND t2_name >= 'y' ORDER BY t1_idAnd expect that that will error with unsupported
|
|
||
| if filter.is_empty() { | ||
| join.build() | ||
| } else if join_type == JoinType::Inner { |
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 is the right behavior.
| if filter.is_empty() { | ||
| join.build() | ||
| } else if join_type == JoinType::Inner { | ||
| join.filter( |
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 is a fancy way of creating an AND chain 👍
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
…che#660) * Move temporal expressions to spark-expr crate * reduce public api * reduce public api * update imports in benchmarks * fmt * remove unused dep
…che#660) * Move temporal expressions to spark-expr crate * reduce public api * reduce public api * update imports in benchmarks * fmt * remove unused dep
Which issue does this PR close?
Closes #659
Rationale for this change
In queries, some expressions can be added that can not be handled by a equi-join. Those should be supported by moving them to a filter instead.
This is only done for inner join for now, supporting this for other (left/right) joins will require a bit more work.
What changes are included in this PR?
Changes the planner to add the unsupported expressions to a filter instead of returning an error.
This almost makes TPC-H query 13 pass, except from some small unsupported / ignored syntax.
Are there any user-facing changes?
No, should just be a bit more flexible.