Resolve empty relation opt for join types #11066
Merged
Conversation
This file contains 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
Merge branch 'main' into fill-out-empty-relation-opt-for-rest-join-types
5 tasks
alamb
approved these changes
Jun 23, 2024
| @@ -142,13 +146,19 @@ impl OptimizerRule for PropagateEmptyRelation { | |||
| schema: join.schema.clone(), | |||
| }), | |||
| )), | |||
| JoinType::LeftAnti if right_empty => { | |||
There was a problem hiding this comment.
I double checked that an ANTI join (e.g. a NOT IN) is true when there are no tuples (aka the right side is emprt)
postgres=# select * from foo;
x
-----
1
2
NaN
(3 rows)
postgres=# select * from bar;
y
---
(0 rows)
postgres=# select x NOT IN (SELECT y from bar) from foo;
?column?
----------
t
t
t
(3 rows)
postgres=# select x from foo where NOT EXISTS (SELECT y from bar);
x
-----
1
2
NaN
(3 rows)Thus I think this is a valid rewrite
There was a problem hiding this comment.
Nice, thanks for the double check!
jonahgao
reviewed
Jun 23, 2024
| JoinType::RightAnti if right_empty => Ok(Transformed::yes( | ||
| LogicalPlan::EmptyRelation(EmptyRelation { | ||
| produce_one_row: false, | ||
| schema: join.schema.clone(), | ||
| }), | ||
| )), | ||
| _ => Ok(Transformed::no(LogicalPlan::Join(join.clone()))), | ||
| _ => Ok(Transformed::no(plan)), |
jonahgao
reviewed
Jun 23, 2024
jonahgao
approved these changes
Jun 23, 2024
tshauck
approved these changes
Jun 23, 2024
Co-authored-by: Jonah Gao <jonahgao@msn.com>
…n-opt-for-join-types
…n-opt-for-join-types
|
I merged this PR up from main and plan to merge it when CI passes |
|
Merged. Thanks @LorrensP-2158466 @alamb @tshauck |
findepi
pushed a commit
to findepi/datafusion
that referenced
this pull request
Jul 16, 2024
* handle left/right anti with empty join_table & added tests for those * add tests and only resolve cases 1,2,3 of issue * forgot to change a test * Update datafusion/optimizer/src/propagate_empty_relation.rs Co-authored-by: Jonah Gao <jonahgao@msn.com> --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> Co-authored-by: Jonah Gao <jonahgao@msn.com>
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?
Almost closes #10967:
Rationale for this change
Currently the PropagateEmptyRelation relation doesn't optimize all join types
This PR closes following cases
What changes are included in this PR?
Added match statements that resolve the above cases.
Are these changes tested?
Yes these changes are tested with unit tests and 3 added slt tests.
Are there any user-facing changes?
No