-
Notifications
You must be signed in to change notification settings - Fork 5.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
Remove cross join if one side of input is single row constant #23081
Remove cross join if one side of input is single row constant #23081
Conversation
b80af5a
to
05be389
Compare
0fcb97b
to
8e24401
Compare
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.
Please add a couple of tests with maps and arrays literals where we see the most impact
...in/java/com/facebook/presto/sql/planner/iterative/rule/RemoveCrossJoinWithConstantInput.java
Outdated
Show resolved
Hide resolved
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.
Add tests with cross join a constant with filter after the join
...in/java/com/facebook/presto/sql/planner/iterative/rule/RemoveCrossJoinWithConstantInput.java
Show resolved
Hide resolved
8e24401
to
8d193e7
Compare
Added these tests |
8d193e7
to
519a6cf
Compare
...in/java/com/facebook/presto/sql/planner/iterative/rule/RemoveCrossJoinWithConstantInput.java
Outdated
Show resolved
Hide resolved
519a6cf
to
4744923
Compare
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.
Looks good.
Added some nits. Feel free to skip if the suggestions doesn't make sense.
...in/java/com/facebook/presto/sql/planner/iterative/rule/RemoveCrossJoinWithConstantInput.java
Outdated
Show resolved
Hide resolved
...in/java/com/facebook/presto/sql/planner/iterative/rule/RemoveCrossJoinWithConstantInput.java
Outdated
Show resolved
Hide resolved
4744923
to
d0e47dc
Compare
d0e47dc
to
d6f7af2
Compare
d6f7af2
to
cf57bb0
Compare
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.
Looks good.
Description
Fix #23074
When one side of cross join inputs is single row with constant values, we can remove this cross join and replace it with a project node. This will simplify the plan, and enable more following optimizations, such as filter pushdown etc.
Motivation and Context
We observed savings enabled by this optimization in our workload.
Impact
We observed savings enabled by this optimization in our workload.
Test Plan
Unit tests
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.