Skip to content

Conversation

@Dandandan
Copy link
Contributor

@Dandandan Dandandan commented Jul 3, 2021

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.

Copy link
Member

@houqp houqp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@Dandandan
Copy link
Contributor Author

@alamb do you mind to take a look?

Copy link
Contributor

@alamb alamb left a 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_id

And expect that that will error with unsupported


if filter.is_empty() {
join.build()
} else if join_type == JoinType::Inner {
Copy link
Contributor

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(
Copy link
Contributor

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 👍

Dandandan and others added 2 commits July 7, 2021 08:11
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
@alamb alamb merged commit 0368f59 into apache:master Jul 7, 2021
@houqp houqp added the enhancement New feature or request label Jul 31, 2021
andygrove added a commit to andygrove/datafusion that referenced this pull request Jan 31, 2025
…che#660)

* Move temporal expressions to spark-expr crate

* reduce public api

* reduce public api

* update imports in benchmarks

* fmt

* remove unused dep
unkloud pushed a commit to unkloud/datafusion that referenced this pull request Mar 23, 2025
…che#660)

* Move temporal expressions to spark-expr crate

* reduce public api

* reduce public api

* update imports in benchmarks

* fmt

* remove unused dep
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.

Join condition should allow non-equijoin expressions

3 participants