#120644 `RIGHT JOIN` over a joined left side: parallel replicas throw LOGICAL_ERROR

Describe what's wrong

A query offloaded to parallel replicas that contains a RIGHT JOIN whose left table expression is itself a JOIN (or a comma/CROSS JOIN) fails with Code: 49 ... Expected JOIN table expression to be table, table function, query or union node. Actual INNER JOIN ... ON a.number = b.key. The same query returns rows with enable_parallel_replicas = 0, and the same query with LEFT instead of RIGHT on the nested join returns rows with parallel replicas on. All settings at their defaults except the ones needed to enable parallel replicas.

Root cause: RewriteJoinToGlobalJoinVisitor::enterImpl (src/Storages/buildQueryTreeForShard.cpp:1158) sets JoinLocality::Global on a RIGHT JOIN without checking that the side it thereby elects for materialization can be shipped as a temporary table. getSupportingParallelReplicasQueries has exactly that guard (src/Planner/findParallelReplicasQuery.cpp:161-166: the left node type must be TABLE/QUERY/UNION), but it only vets joins on the join-tree path; rewriteJoinToGlobalJoin also visits joins inside expression subqueries (WHERE key IN (SELECT ...)), which the candidate walk never inspects.

Analysis details (evidence, affected locations, impact)

Why we believe this is a bug: buildQueryPlanForParallelReplicas (src/Planner/findParallelReplicasQuery.cpp:627) -> rewriteJoinToGlobalJoin -> RewriteJoinToGlobalJoinVisitor::enterImpl (src/Storages/buildQueryTreeForShard.cpp:1158) -> materializedSideCanStayLocal (:1122), which via getMaterializedTableExpressionNode (:85-91) now inspects the LEFT expression of a RIGHT JOIN; an ineligible storage there makes it return false, so the join is marked JoinLocality::Global -> buildQueryTreeForShard (:953-956) takes getLeftTableExpressionNodeTyped() as the side to materialize -> getSubqueryFromTableExpression (:787-790) throws because a JoinNode is none of TABLE / TABLE_FUNCTION / QUERY / UNION / ARRAY_JOIN.

Affected locations:

src/Storages/buildQueryTreeForShard.cpp:1158 — enterImpl globalizes the join from materializedSideCanStayLocal, with no node-type check on the side that will be materialized
src/Storages/buildQueryTreeForShard.cpp:87 — getMaterializedTableExpressionNode returns the LEFT expression for JoinKind::Right
src/Storages/buildQueryTreeForShard.cpp:955 — the shipped-tree rewrite takes getLeftTableExpressionNodeTyped() for a global RIGHT JOIN
src/Storages/buildQueryTreeForShard.cpp:787 — getSubqueryFromTableExpression throws LOGICAL_ERROR for a JOIN / CROSS_JOIN node
src/Planner/findParallelReplicasQuery.cpp:166 — the guard that exists on the join-tree path but not in the rewrite

Impact: LOGICAL_ERROR (code 49) on valid SQL with default settings: the query is refused and the server logs it through ForcedCriticalErrorsLogger. Debug and sanitizer builds abort on a LOGICAL_ERROR, so the AST fuzzer and Stress jobs redden - the same failure mode this PR exists to remove. A comma join in the same position (FROM numbers(10) AS a, t AS b RIGHT JOIN t AS c ON ...) hits the identical throw (Actual , ...).

Does it reproduce on most recent release?

Yes — confirmed on current master (commit 2d19d1b6e073).

How to reproduce

-- Test: a RIGHT JOIN whose left side is itself a JOIN, inside a query read with parallel replicas.

DROP TABLE IF EXISTS t_05218_a;
DROP TABLE IF EXISTS t_05218_b;

CREATE TABLE t_05218_a (key UInt64) ENGINE = MergeTree ORDER BY key;
INSERT INTO t_05218_a SELECT number FROM numbers(10);

CREATE TABLE t_05218_b (key UInt64) ENGINE = MergeTree ORDER BY key;
INSERT INTO t_05218_b SELECT number FROM numbers(10);

SET enable_analyzer = 1;
SET automatic_parallel_replicas_mode = 0;
SET enable_parallel_replicas = 1, max_parallel_replicas = 2,
    cluster_for_parallel_replicas = 'test_cluster_one_shard_three_replicas_localhost',
    parallel_replicas_for_non_replicated_merge_tree = 1;

SELECT '-- nested LEFT JOIN with a joined left side';
SELECT count() FROM (SELECT key FROM t_05218_a WHERE key IN (
    SELECT c.key FROM numbers(10) AS a JOIN t_05218_b AS b ON a.number = b.key LEFT JOIN t_05218_b AS c ON b.key = c.key
)) AS l;

SELECT '-- nested RIGHT JOIN with a plain left side';
SELECT count() FROM (SELECT key FROM t_05218_a WHERE key IN (
    SELECT c.key FROM t_05218_b AS b RIGHT JOIN t_05218_b AS c ON b.key = c.key
)) AS l;

SELECT '-- nested RIGHT JOIN with a joined left side';
SELECT count() FROM (SELECT key FROM t_05218_a WHERE key IN (
    SELECT c.key FROM numbers(10) AS a JOIN t_05218_b AS b ON a.number = b.key RIGHT JOIN t_05218_b AS c ON b.key = c.key
)) AS l;

DROP TABLE t_05218_a;
DROP TABLE t_05218_b;

Expected behavior

Expected output of the reproducer above:

-- nested LEFT JOIN with a joined left side
10
-- nested RIGHT JOIN with a plain left side
10
-- nested RIGHT JOIN with a joined left side
10

Error message and/or stacktrace

Actual output of the reproducer above on master (2d19d1b6e073):

-- nested LEFT JOIN with a joined left side
10
-- nested RIGHT JOIN with a plain left side
10
-- nested RIGHT JOIN with a joined left side
Code: 49. DB::Exception: Received from localhost:19010. DB::Exception: Expected JOIN table expression to be table, table function, query or union node. Actual  INNER JOIN ... ON a.number = b.key. (LOGICAL_ERROR)

Suggested fix

Mirror the candidate walk's guard in the rewrite: in RewriteJoinToGlobalJoinVisitor::enterImpl / materializedSideCanStayLocal, leave the join local when the materialized side's node type is JOIN or CROSS_JOIN, since it cannot be turned into a temporary table. Trade-off: that keeps the pre-existing parallel_replicas_prefer_local_join = 0 path broken (there should_use_global_join is true regardless of the side). Closing both requires teaching getSubqueryFromTableExpression to wrap a JOIN/CROSS_JOIN node in a subquery the way its ARRAY_JOIN branch already does.

Additional context

Open risks:

The throw itself predates this PR: with parallel_replicas_prefer_local_join = 0 every visited join is globalized, so the same query already threw before the change (reproduced on this binary at plj=0 for a nested join whose sides are all MergeTree). What this PR changes is that it is now reachable at the DEFAULT value 1.
The mirrored case the PR fixes by accident - an ineligible storage on the nested RIGHT JOIN's RIGHT side - now returns rows where it previously threw, so the net effect on affected shapes is a trade, not a pure regression.

Found during automated review of PR #113515; whether that PR introduced it could not be established, so nobody is tagged. Severity P1 · Finding h_pr113515_001