Skip to content

Fix join condition lost after pull up sublink to join - #1961

Merged
leborchuk merged 1 commit into
apache:REL_2_STABLEfrom
Alena0704:rel2-fix-join-cond-lost-pullup
Sep 9, 2026
Merged

Fix join condition lost after pull up sublink to join#1961
leborchuk merged 1 commit into
apache:REL_2_STABLEfrom
Alena0704:rel2-fix-join-cond-lost-pullup

Conversation

@Alena0704

@Alena0704 Alena0704 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fix join condition lost after pull up sublink to join.

After pulling up the sublink to join, the raw join condition may get lost in the rewritten query, potentially leading to incorrect results. Within the SubqueryToJoinWalker() function, we address this issue by adding an 'else' branch to prevent the loss of join clauses and keep them in their original positions.

(cherry picked from open-gpdb commit c06d16b)

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change (fix or feature with breaking changes)
  • Documentation update

Breaking Changes

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

Additional Context

CI Skip Instructions


@Alena0704

Copy link
Copy Markdown
Contributor Author

When a correlated aggregate subquery is pulled up into a join, SubqueryToJoinWalker() records only two kinds of quals: non-correlated branches found inside an AND BoolExpr, and correlated equality OpExprs that become the grouping key. A single non-correlated qual matches neither branch — it falls through to the final return and is recorded nowhere. RemoveInnerJoinQuals() then clears je->quals, and subselect->jointree->quals is overwritten with what the walker collected, so the predicate is gone from the rewritten query.

A one-predicate JOIN ... ON clause is exactly such a qual, so its condition is silently dropped and the aggregate is computed over an unrestricted join:

set optimizer = off;

create table o (a int, d int);  insert into o  values (2, 1);
create table i1(a int);         insert into i1 values (1);
create table i2(a int);         insert into i2 values (1), (2);

select * from o where o.a > (select max(i2.a) from i1 join i2 on i2.a = i1.a where i1.a = o.d);
postgres=# create table o (a int, d int);  insert into o  values (2, 1);
NOTICE:  Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Apache Cloudberry data distribution key for this table.
HINT:  The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
CREATE TABLE
INSERT 0 1
postgres=# create table i1(a int);         insert into i1 values (1);
NOTICE:  Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Apache Cloudberry data distribution key for this table.
HINT:  The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
CREATE TABLE
INSERT 0 1
postgres=# create table i2(a int);         insert into i2 values (1), (2);
NOTICE:  Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Apache Cloudberry data distribution key for this table.
HINT:  The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
CREATE TABLE
INSERT 0 2
postgres=# 
postgres=# -- returns (2,1) as a SubPlan, returns nothing when pulled up
postgres=# select * from o where o.a > (select max(i2.a) from i1 join i2 on i2.a = i1.a where i1.a = o.d);
 a | d 
---+---
 2 | 1
(1 row)

postgres=# set optimizer = off;
SET
postgres=# select * from o where o.a > (select max(i2.a) from i1 join i2 on i2.a = i1.a where i1.a = o.d);
 a | d 
---+---
(0 rows)

i1.a = 1, so ON leaves only i2.a = 1 and the subquery is 1; 2 > 1 holds and the row must be returned. Instead the query returns nothing, because a Nested Loop with no Join Cond producing 2 rows instead of 1, which makes max come out as 2.

The fix adds an else branch that keeps such quals where they were. Only the Postgres planner is affected (optimizer=off, or an ORCA fallback); ORCA does its own decorrelation and preserves the ON condition.

@leborchuk

Copy link
Copy Markdown
Contributor

It looks like something wrong with subselect.out

--- /__w/cloudberry/cloudberry/src/test/regress/expected/subselect.out	2026-09-04 05:52:26.139317294 -0700
+++ /__w/cloudberry/cloudberry/src/test/regress/results/subselect.out	2026-09-04 05:52:26.176315671 -0700
@@ -2076,41 +2071,44 @@
   );
 QUERY PLAN
 ___________
- Gather Motion 3:1  (slice4; segments: 3)
+ Gather Motion 3:1  (slice1; segments: 3)
    Output: tl1.a, tl1.b, tl1.c, tl1.d
    ->  Hash Join
          Output: tl1.a, tl1.b, tl1.c, tl1.d
+         Inner Unique: true
          Hash Cond: ((tl1.b = "Expr_SUBQUERY".csq_c1) AND (tl1.c = "Expr_SUBQUERY".csq_c0))
-         ->  Redistribute Motion 3:3  (slice1; segments: 3)
+         ->  Seq Scan on public.tl1
                Output: tl1.a, tl1.b, tl1.c, tl1.d
-               Hash Key: tl1.c
-               ->  Seq Scan on public.tl1
-                     Output: tl1.a, tl1.b, tl1.c, tl1.d
          ->  Hash
                Output: "Expr_SUBQUERY".csq_c1, "Expr_SUBQUERY".csq_c0
-               ->  Subquery Scan on "Expr_SUBQUERY"
+               ->  Broadcast Motion 3:3  (slice2; segments: 3)
                      Output: "Expr_SUBQUERY".csq_c1, "Expr_SUBQUERY".csq_c0
-                     ->  HashAggregate
-                           Output: tl2.b, max((max(tl2.a)))
-                           Group Key: tl2.b
-                           ->  Redistribute Motion 3:3  (slice3; segments: 3)
-                                 Output: tl2.b, (max(tl2.a))
-                                 Hash Key: tl2.b
-                                 ->  HashAggregate
-                                       Output: tl2.b, max(tl2.a)
-                                       Group Key: tl2.b
-                                       ->  Hash Join
-                                             Output: tl2.b, tl2.a
-                                             Hash Cond: (tl4.d = tl2.d)
-                                             ->  Broadcast Motion 3:3  (slice2; segments: 3)
-                                                   Output: tl4.d, tl4.a
-                                                   ->  Seq Scan on public.tl4
-                                                         Output: tl4.d, tl4.a
-                                             ->  Hash
-                                                   Output: tl2.b, tl2.a, tl2.d
-                                                   ->  Seq Scan on public.tl2
-                                                         Output: tl2.b, tl2.a, tl2.d
-GP_IGNORE:(36 rows)
+                     ->  Subquery Scan on "Expr_SUBQUERY"
+                           Output: "Expr_SUBQUERY".csq_c1, "Expr_SUBQUERY".csq_c0
+                           ->  Finalize GroupAggregate
+                                 Output: tl2.b, max(tl2.a)
+                                 Group Key: tl2.b
+                                 ->  Sort
+                                       Output: tl2.b, (PARTIAL max(tl2.a))
+                                       Sort Key: tl2.b
+                                       ->  Redistribute Motion 3:3  (slice3; segments: 3)
+                                             Output: tl2.b, (PARTIAL max(tl2.a))
+                                             Hash Key: tl2.b
+                                             ->  Streaming Partial HashAggregate
+                                                   Output: tl2.b, PARTIAL max(tl2.a)
+                                                   Group Key: tl2.b
+                                                   ->  Hash Join
+                                                         Output: tl2.b, tl2.a
+                                                         Hash Cond: (tl4.d = tl2.d)
+                                                         ->  Broadcast Motion 3:3  (slice4; segments: 3)
+                                                               Output: tl4.d
+                                                               ->  Seq Scan on public.tl4
+                                                                     Output: tl4.d
+                                                         ->  Hash
+                                                               Output: tl2.b, tl2.a, tl2.d
+                                                               ->  Seq Scan on public.tl2
+                                                                     Output: tl2.b, tl2.a, tl2.d
+GP_IGNORE:(39 rows)

@Alena0704
Alena0704 force-pushed the rel2-fix-join-cond-lost-pullup branch from 07fc7d0 to 0b53dfc Compare September 7, 2026 13:07
@Alena0704

Copy link
Copy Markdown
Contributor Author

It looks like something wrong with subselect.out

--- /__w/cloudberry/cloudberry/src/test/regress/expected/subselect.out	2026-09-04 05:52:26.139317294 -0700
+++ /__w/cloudberry/cloudberry/src/test/regress/results/subselect.out	2026-09-04 05:52:26.176315671 -0700
@@ -2076,41 +2071,44 @@
   );
 QUERY PLAN
 ___________
- Gather Motion 3:1  (slice4; segments: 3)
+ Gather Motion 3:1  (slice1; segments: 3)
    Output: tl1.a, tl1.b, tl1.c, tl1.d
    ->  Hash Join
          Output: tl1.a, tl1.b, tl1.c, tl1.d
+         Inner Unique: true
          Hash Cond: ((tl1.b = "Expr_SUBQUERY".csq_c1) AND (tl1.c = "Expr_SUBQUERY".csq_c0))
-         ->  Redistribute Motion 3:3  (slice1; segments: 3)
+         ->  Seq Scan on public.tl1
                Output: tl1.a, tl1.b, tl1.c, tl1.d
-               Hash Key: tl1.c
-               ->  Seq Scan on public.tl1
-                     Output: tl1.a, tl1.b, tl1.c, tl1.d
          ->  Hash
                Output: "Expr_SUBQUERY".csq_c1, "Expr_SUBQUERY".csq_c0
-               ->  Subquery Scan on "Expr_SUBQUERY"
+               ->  Broadcast Motion 3:3  (slice2; segments: 3)
                      Output: "Expr_SUBQUERY".csq_c1, "Expr_SUBQUERY".csq_c0
-                     ->  HashAggregate
-                           Output: tl2.b, max((max(tl2.a)))
-                           Group Key: tl2.b
-                           ->  Redistribute Motion 3:3  (slice3; segments: 3)
-                                 Output: tl2.b, (max(tl2.a))
-                                 Hash Key: tl2.b
-                                 ->  HashAggregate
-                                       Output: tl2.b, max(tl2.a)
-                                       Group Key: tl2.b
-                                       ->  Hash Join
-                                             Output: tl2.b, tl2.a
-                                             Hash Cond: (tl4.d = tl2.d)
-                                             ->  Broadcast Motion 3:3  (slice2; segments: 3)
-                                                   Output: tl4.d, tl4.a
-                                                   ->  Seq Scan on public.tl4
-                                                         Output: tl4.d, tl4.a
-                                             ->  Hash
-                                                   Output: tl2.b, tl2.a, tl2.d
-                                                   ->  Seq Scan on public.tl2
-                                                         Output: tl2.b, tl2.a, tl2.d
-GP_IGNORE:(36 rows)
+                     ->  Subquery Scan on "Expr_SUBQUERY"
+                           Output: "Expr_SUBQUERY".csq_c1, "Expr_SUBQUERY".csq_c0
+                           ->  Finalize GroupAggregate
+                                 Output: tl2.b, max(tl2.a)
+                                 Group Key: tl2.b
+                                 ->  Sort
+                                       Output: tl2.b, (PARTIAL max(tl2.a))
+                                       Sort Key: tl2.b
+                                       ->  Redistribute Motion 3:3  (slice3; segments: 3)
+                                             Output: tl2.b, (PARTIAL max(tl2.a))
+                                             Hash Key: tl2.b
+                                             ->  Streaming Partial HashAggregate
+                                                   Output: tl2.b, PARTIAL max(tl2.a)
+                                                   Group Key: tl2.b
+                                                   ->  Hash Join
+                                                         Output: tl2.b, tl2.a
+                                                         Hash Cond: (tl4.d = tl2.d)
+                                                         ->  Broadcast Motion 3:3  (slice4; segments: 3)
+                                                               Output: tl4.d
+                                                               ->  Seq Scan on public.tl4
+                                                                     Output: tl4.d
+                                                         ->  Hash
+                                                               Output: tl2.b, tl2.a, tl2.d
+                                                               ->  Seq Scan on public.tl2
+                                                                     Output: tl2.b, tl2.a, tl2.d
+GP_IGNORE:(39 rows)

Thanks for catching this - the failure is in the expected output, not in the fix itself.

This patch was cherry-picked from open-gpdb (a Greenplum 6 fork on a PG 9.4-era core), and I forgot to regenerate it against Cloudberry (PG 14.8). The two planners simply print a different plan for the same query. I updated it and now it is fine.

After pulling up the sublink to join, the raw join condition may get
lost in the rewritten query, potentially leading to incorrect results.
Within the SubqueryToJoinWalker() function, we address this issue
by adding an 'else' branch to prevent the loss of join clauses and
keep them in their original positions.

The expected files were regenerated on Cloudberry: the plans the new
test prints differ from the ones the upstream commit carried.

(cherry picked from open-gpdb commit c06d16b)
@Alena0704
Alena0704 force-pushed the rel2-fix-join-cond-lost-pullup branch from 0b53dfc to 6a601ed Compare September 7, 2026 17:26

@yjhjstz yjhjstz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@leborchuk leborchuk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@leborchuk
leborchuk merged commit bc97eba into apache:REL_2_STABLE Sep 9, 2026
285 of 304 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants