Skip to content
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

Fix JOINs with complex predicates in ON (split ON expressions only by AND operator) #2534

Merged
merged 1 commit into from
May 16, 2022

Conversation

korowa
Copy link
Contributor

@korowa korowa commented May 14, 2022

Which issue does this PR close?

Close #2271.

Rationale for this change

Fix for splitting ON expression into keys and filters.

What changes are included in this PR?

At this moment extract_join_keys recursively called on both parts of any binary expression, though parse_join concatenates all filter expressions with AND operator. This PR changes its behaviour - recursive call occurs only in case of AND operator.

Are there any user-facing changes?

Correct execution plans produced for queries with complex ON conditions (in case these conditions are supported).

@github-actions github-actions bot added the datafusion Changes in the datafusion crate label May 14, 2022
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.

Makes sense to me -- thank you @korowa

FROM person \
JOIN orders ON id = customer_id OR person.age > 30";
let expected = "Projection: #person.id, #orders.order_id\
\n Filter: #person.id = #orders.customer_id OR #person.age > Int64(30)\
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@alamb alamb changed the title split ON expressions only by AND operator Fix JOINs with complex predicates in ON (split ON expressions only by AND operator) May 16, 2022
@alamb alamb merged commit ee945c3 into apache:master May 16, 2022
@alamb
Copy link
Contributor

alamb commented May 16, 2022

This is a great find. Thank you @korowa

korowa added a commit to korowa/arrow-datafusion that referenced this pull request May 18, 2022
MazterQyou pushed a commit to cube-js/arrow-datafusion that referenced this pull request May 26, 2022
MazterQyou pushed a commit to cube-js/arrow-datafusion that referenced this pull request May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datafusion Changes in the datafusion crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inner join incorrectly pushdown predicate with OR operation
2 participants