-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Fix empty join conditions query failure #25713
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 empty join conditions query failure #25713
Conversation
7c045a9 to
3c044a2
Compare
3c044a2 to
884346b
Compare
core/trino-main/src/main/java/io/trino/sql/planner/iterative/rule/PushJoinIntoTableScan.java
Outdated
Show resolved
Hide resolved
8a28693 to
60003a5
Compare
plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java
Outdated
Show resolved
Hide resolved
16b962a to
f43bce5
Compare
|
Oracle ci failure is unrelated, PTAL @Praveen2112 |
f43bce5 to
b872e5a
Compare
Praveen2112
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.
% nit
plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultQueryBuilder.java
Outdated
Show resolved
Hide resolved
38e6564 to
5a326fd
Compare
|
Fixed the nit and rebased - @Praveen2112 please merge once CI is done |
5a326fd to
5555be7
Compare
5555be7 to
aaf3194
Compare
Praveen2112
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.
I have one question on what’s special for LEFT and FULL join
|
|
||
| protected static boolean expectJoinPushdownOnEmptyProjection(JoinOperator joinOperator) | ||
| { | ||
| return joinOperator == LEFT_JOIN || joinOperator == FULL_JOIN; |
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.
Any specific reason on pushdown is applied for LEFT or FULL join ?
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 'decorative' join query shape looks slightly different for different join operators, e.g. the join filter is retained in the join node for the LEFT JOIN:
SELECT n.name FROM postgresql.tpch.nation n LEFT JOIN postgresql.tpch.orders o ON n.regionkey = 1;
LeftJoin[filter = (regionkey = bigint '1'), distribution = REPLICATED]
│ Layout: [name:varchar(25)]
│ Distribution: REPLICATED
├─ TableScan[table = postgresql:tpch.nation tpch.tpch.nation columns=[name:varchar(25):varchar, regionkey:bigint:int8]]
│ Layout: [name:varchar(25), regionkey:bigint]
└─ LocalExchange[partitioning = SINGLE]
└─ RemoteSource[sourceFragmentIds = [1]]
Fragment 1 [SOURCE]
Output partitioning: BROADCAST []
TableScan[table = postgresql:tpch.orders tpch.tpch.orders columns=[]]
Layout: []On the other hand, for RIGHT JOIN, condition is pushed down into constraint on the scan:
SELECT n.name FROM postgresql.tpch.nation n RIGHT JOIN postgresql.tpch.orders o ON n.regionkey = 1
RightJoin[distribution = PARTITIONED]
│ Layout: [name:varchar(25)]
│ Distribution: PARTITIONED
├─ RemoteSource[sourceFragmentIds = [1]]
│ Layout: [name:varchar(25)]
└─ LocalExchange[partitioning = SINGLE]
│ Layout: []
└─ RemoteSource[sourceFragmentIds = [2]]
Layout: []
Fragment 1 [SOURCE]
Output partitioning: HASH []
TableScan[table = postgresql:tpch.nation tpch.tpch.nation constraint on [regionkey] columns=[name:varchar(25):varchar]]
Layout: [name:varchar(25)]
Fragment 2 [SOURCE]
Output partitioning: HASH []
TableScan[table = postgresql:tpch.orders tpch.tpch.orders columns=[]]
Layout: []There is actually no join condition to push down, so applyJoin returns empty-handed.
|
@SemionPar Can you please update the release notes. |
Description
Fix
"joinConditions is empty"error.Additional context and related issues
Bisected to 91a29a7
Query of this shape results in
Joins with effectively empty filters should not be pushed down.
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: