PR #25713 Fix empty join conditions query failure

SemionPar23 days ago (edited 10 days ago)

Description

Fix "joinConditions is empty" error.

Additional context and related issues

Bisected to 91a29a7

SELECT a.c_num FROM left a LEFT JOIN right b ON DATE '2025-03-19' = b.c_date;

Query of this shape results in

Caused by: com.google.common.base.VerifyException: joinConditions is empty
	at com.google.common.base.Verify.verify(Verify.java:126)
	at io.trino.plugin.jdbc.DefaultQueryBuilder.prepareJoinQuery(DefaultQueryBuilder.java:126)
	at io.trino.plugin.jdbc.BaseJdbcClient.implementJoin(BaseJdbcClient.java:537)
	at io.trino.plugin.postgresql.PostgreSqlClient.lambda$implementJoin$13(PostgreSqlClient.java:1074)
	at io.trino.plugin.jdbc.JdbcJoinPushdownUtil.implementJoinCostAware(JdbcJoinPushdownUtil.java:46)
	at io.trino.plugin.postgresql.PostgreSqlClient.implementJoin(PostgreSqlClient.java:1068)
	at io.trino.plugin.jdbc.ForwardingJdbcClient.implementJoin(ForwardingJdbcClient.java:220)
	at io.trino.plugin.jdbc.jmx.StatisticsAwareJdbcClient.lambda$implementJoin$20(StatisticsAwareJdbcClient.java:239)
	at io.trino.plugin.jdbc.jmx.JdbcApiStats.wrap(JdbcApiStats.java:34)
	at io.trino.plugin.jdbc.jmx.StatisticsAwareJdbcClient.implementJoin(StatisticsAwareJdbcClient.java:239)
	at io.trino.plugin.jdbc.CachingJdbcClient.implementJoin(CachingJdbcClient.java:293)
	at io.trino.plugin.jdbc.CachingJdbcClient.implementJoin(CachingJdbcClient.java:293)
	at io.trino.plugin.jdbc.DefaultJdbcMetadata.applyJoin(DefaultJdbcMetadata.java:497)
	at io.trino.plugin.base.classloader.ClassLoaderSafeConnectorMetadata.applyJoin(ClassLoaderSafeConnectorMetadata.java:1042)
	at io.trino.tracing.TracingConnectorMetadata.applyJoin(TracingConnectorMetadata.java:1206)
	at io.trino.metadata.MetadataManager.applyJoin(MetadataManager.java:1957)
	at io.trino.tracing.TracingMetadata.applyJoin(TracingMetadata.java:988)
	at io.trino.sql.planner.iterative.rule.PushJoinIntoTableScan.apply(PushJoinIntoTableScan.java:149)
	at io.trino.sql.planner.iterative.rule.PushJoinIntoTableScan.apply(PushJoinIntoTableScan.java:68)
	at io.trino.sql.planner.iterative.IterativeOptimizer.transform(IterativeOptimizer.java:209)
	at io.trino.sql.planner.iterative.IterativeOptimizer.exploreNode(IterativeOptimizer.java:176)
	at io.trino.sql.planner.iterative.IterativeOptimizer.exploreGroup(IterativeOptimizer.java:139)
	at io.trino.sql.planner.iterative.IterativeOptimizer.exploreChildren(IterativeOptimizer.java:259)
	at io.trino.sql.planner.iterative.IterativeOptimizer.exploreGroup(IterativeOptimizer.java:141)
	at io.trino.sql.planner.iterative.IterativeOptimizer.optimize(IterativeOptimizer.java:123)
	at io.trino.sql.planner.LogicalPlanner.runOptimizer(LogicalPlanner.java:309)
	at io.trino.sql.planner.LogicalPlanner.plan(LogicalPlanner.java:270)
	at io.trino.sql.planner.LogicalPlanner.plan(LogicalPlanner.java:239)
	at io.trino.sql.planner.LogicalPlanner.plan(LogicalPlanner.java:234)
	at io.trino.execution.SqlQueryExecution.doPlanQuery(SqlQueryExecution.java:486)
	at io.trino.execution.SqlQueryExecution.planQuery(SqlQueryExecution.java:466)
	at io.trino.execution.SqlQueryExecution.start(SqlQueryExecution.java:404)
	at io.trino.execution.SqlQueryManager.createQuery(SqlQueryManager.java:264)
	at io.trino.dispatcher.LocalDispatchQuery.startExecution(LocalDispatchQuery.java:145)
	at io.trino.dispatcher.LocalDispatchQuery.lambda$waitForMinimumWorkers$2(LocalDispatchQuery.java:129)
	at io.airlift.concurrent.MoreFutures.lambda$addSuccessCallback$12(MoreFutures.java:568)
	at io.airlift.concurrent.MoreFutures$3.onSuccess(MoreFutures.java:543)
	at com.google.common.util.concurrent.Futures$CallbackListener.run(Futures.java:1133)
	at io.trino.$gen.Trino_testversion____20250430_152414_75.run(Unknown Source)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
	at java.base/java.lang.Thread.run(Thread.java:1583)

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:

## Section
* Fix some things. ({issue}`issuenumber`)

cla-bot added cla-signed

github-actions added postgresql

SemionPar force pushed from 7c045a9f to 3c044a28 23 days ago

SemionPar force pushed from 3c044a28 to 884346be 10 days ago

SemionPar changed the title ~~[wip] Fix empty join conditions query failure~~ Fix empty join conditions query failure 10 days ago

SemionPar marked this pull request as ready for review 10 days ago

SemionPar requested a review from

anusudarsan 10 days ago

SemionPar requested a review from

chenjian2664 10 days ago

SemionPar requested a review from

ebyhr 10 days ago

ebyhr requested a review from

Praveen2112 10 days ago

ebyhr requested a review from

kasiafi 10 days ago

chenjian2664 approved these changes on 2025-05-14

Praveen2112 commented on 2025-05-14

Conversation is marked as resolved

Show resolved

core/trino-main/src/main/java/io/trino/sql/planner/iterative/rule/PushJoinIntoTableScan.java

107	107	}
108	108
109	109	Expression effectiveFilter = getEffectiveFilter(joinNode);
	110	if (effectiveFilter.equals(Booleans.TRUE)) {

Praveen21129 days ago

I'm afraid we might need some additional checks beyond TRUE - What if SELECT n.name FROM nation n %s orders o ON n.comment is null i.e apply a condition on probe side ?

Praveen21129 days ago

Why are we specific to this effective filter ?

chenjian26649 days ago

The joinCondition later is derived from this effectiveFilter.
The joinConditions is empty is comes from the result DefaultJdbcMetadata#applyJoin#extractConjuncts(joinCondition), that means the results of the ConnectorExpressionTranslator.translateConjuncts is all TRUE.

chenjian26649 days ago👍 1

Maybe put a check in DefaultJdbcMetadata#applyJoin is more direct:

        if (joinConditions.build().isEmpty()) {
            return Optional.empty();
        }

SemionPar9 days ago

I'm afraid we might need some additional checks beyond TRUE - What if SELECT n.name FROM nation n %s orders o ON n.comment is null i.e apply a condition on probe side ?

Great suggestion! Added more assertions and it revealed that there is another issue here as well.

For LEFT JOIN with lone predicate ON n.comment is null on the probe side, there is an error on pushdown:

--trino
SET SESSION postgresql.join_pushdown_strategy = 'eager';
SELECT n.name FROM postgresql.tpch.nation n LEFT JOIN postgresql.tpch.orders o ON n.comment is null;
--postgresql
SELECT "name_0" FROM (SELECT "name_0", "comment_1",  FROM (SELECT "name" AS "name_0", "comment" AS "comment_1" FROM (SELECT "name", "comment" FROM "tpch"."nation") l) l LEFT JOIN (SELECT  FROM (SELECT 1 x FROM "tpch"."orders") r) r ON (("comment_1") IS NULL)) o
---------------------------------------------------^
--error
--double-checking with sqlserver
SELECT "name_0" FROM (SELECT "name_0", "comment_1",  FROM (SELECT "name" AS "name_0", "comment" AS "comment_1" FROM (SELECT "name", "comment" FROM "database_950c7af8f1894a50beef3d652468ffeb"."dbo"."nation") l) l LEFT JOIN (SELECT  FROM (SELECT 1 x FROM "database_950c7af8f1894a50beef3d652468ffeb"."dbo"."orders") r) r ON (("comment_1") IS NULL)) o
--same issue

This is with&without my change to PushJoinIntoTableScan.

Lone predicate on the build side is fine:

--trino
SET SESSION postgresql.join_pushdown_strategy = 'eager';
SELECT n.name FROM postgresql.tpch.nation n LEFT JOIN postgresql.tpch.orders o ON o.comment is null
--postgresql, 2 scans, join not pushded down
SELECT 1 x FROM "tpch"."orders" WHERE "comment" IS NULL
SELECT "name" FROM "tpch"."nation"

SemionPar9 days ago

Why are we specific to this effective filter ?

IIUC empty effective filter (=BOOLEAN::TRUE) is what causing joinConditions is empty error downstream

SemionPar8 days ago

Maybe put a check in DefaultJdbcMetadata#applyJoin is more direct

Great idea @chenjian2664 - applied!

Adding separate commit with query builder fix to SELECT "name_0" FROM (SELECT "name_0", "comment_1", FROM ... misshaped query.

SemionPar force pushed from 884346be to 2d9a3f12 8 days ago

SemionPar force pushed from 8a28693e to 60003a5a 8 days ago

anusudarsan approved these changes on 2025-05-15

chenjian2664 commented on 2025-05-16

Conversation is marked as resolved

Show resolved

SemionPar force pushed from 60003a5a to 16b962a4 7 days ago

SemionPar force pushed from 16b962a4 to f43bce5d 4 days ago

SemionPar4 days ago

Oracle ci failure is unrelated, PTAL @Praveen2112

Fix failure on effectively empty join conditions

eba4027e

SemionPar force pushed from f43bce5d to b872e5a9 3 days ago

SemionPar requested a review from

Praveen2112 3 days ago

Praveen2112 approved these changes on 2025-05-21

Praveen21122 days ago

% nit

Conversation is marked as resolved

Show resolved

Fix the join query if one side is not projected

38e6564a

SemionPar force pushed from b872e5a9 to 38e6564a 2 days ago

690	690	joinConditions.add(converted.get());
691	691	}
692	692
	693	ImmutableList<ParameterizedExpression> convertedJoinConditions = joinConditions.build();

131	131	// The joinConditions and output columns are aliased to use unique names.
132		"SELECT %s, %s FROM (SELECT %s FROM (%s) l) l %s (SELECT %s FROM (%s) r) r ON %s",
133		formatProjectionAliases(client, leftProjections.values()),
134		formatProjectionAliases(client, rightProjections.values()),
	132	"SELECT %s FROM (SELECT %s FROM (%s) l) l %s (SELECT %s FROM (%s) r) r ON %s",
	133	formatProjectionAliases(
	134	client, ImmutableList.<String>builder()

trino
Fix empty join conditions query failure
#25713

Open

Fix empty join conditions query failure #25713

Description

Additional context and related issues

Release notes

trino Fix empty join conditions query failure #25713 Open

Fix empty join conditions query failure #25713

Description

Additional context and related issues

Release notes

trino
Fix empty join conditions query failure
#25713

Open