v1-0001-Fix-how-we-build-join-relids-for-child-join.patch

From deb03160ca7d28ac1c322b2eeb7264e7c5817def Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Fri, 21 Jul 2023 16:13:15 +0800
Subject: [PATCH v1] Fix how we build join relids for child join

---
 src/backend/optimizer/path/joinrels.c | 14 +++++----
 src/backend/optimizer/util/relnode.c  | 14 +++++----
 src/test/regress/expected/join.out    | 43 +++++++++++++++++++++++++++
 src/test/regress/sql/join.sql         | 24 +++++++++++++++
 4 files changed, 83 insertions(+), 12 deletions(-)

diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c
index 2feab2184f..a3f94be1d6 100644
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -1640,13 +1640,15 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
 											   child_rel1->relids,
 											   child_rel2->relids);
 
-		/* Build correct join relids for child join */
-		child_joinrelids = bms_union(child_rel1->relids, child_rel2->relids);
-		child_joinrelids = add_outer_joins_to_relids(root, child_joinrelids,
-													 child_sjinfo, NULL);
-
 		/* Find the AppendRelInfo structures */
-		appinfos = find_appinfos_by_relids(root, child_joinrelids, &nappinfos);
+		appinfos = find_appinfos_by_relids(root,
+										   bms_union(child_rel1->relids,
+													 child_rel2->relids),
+										   &nappinfos);
+
+		/* Build correct join relids for child join */
+		child_joinrelids = adjust_child_relids(joinrel->relids,
+											   nappinfos, appinfos);
 
 		/*
 		 * Construct restrictions applicable to the child join from those
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index 15e3910b79..32e8cb449c 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -871,10 +871,15 @@ build_child_join_rel(PlannerInfo *root, RelOptInfo *outer_rel,
 	/* The parent joinrel should have consider_partitionwise_join set. */
 	Assert(parent_joinrel->consider_partitionwise_join);
 
+	/* Compute information needed for mapping Vars to the child rel */
+	appinfos = find_appinfos_by_relids(root,
+									   bms_union(outer_rel->relids,
+												 inner_rel->relids),
+									   &nappinfos);
+
 	joinrel->reloptkind = RELOPT_OTHER_JOINREL;
-	joinrel->relids = bms_union(outer_rel->relids, inner_rel->relids);
-	joinrel->relids = add_outer_joins_to_relids(root, joinrel->relids, sjinfo,
-												NULL);
+	joinrel->relids = adjust_child_relids(parent_joinrel->relids,
+										  nappinfos, appinfos);
 	joinrel->rows = 0;
 	/* cheap startup cost is interesting iff not all tuples to be retrieved */
 	joinrel->consider_startup = (root->tuple_fraction > 0);
@@ -935,9 +940,6 @@ build_child_join_rel(PlannerInfo *root, RelOptInfo *outer_rel,
 	/* Compute information relevant to foreign relations. */
 	set_foreign_rel_properties(joinrel, outer_rel, inner_rel);
 
-	/* Compute information needed for mapping Vars to the child rel */
-	appinfos = find_appinfos_by_relids(root, joinrel->relids, &nappinfos);
-
 	/* Set up reltarget struct */
 	build_child_join_reltarget(root, parent_joinrel, joinrel,
 							   nappinfos, appinfos);
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 9b8638f286..e12221c623 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2687,6 +2687,49 @@ select * from onek t1
                            Filter: (two = t2.two)
 (11 rows)
 
+--
+-- check a case where we built wrong join relids for child join
+--
+begin;
+create temp table part_tbl (a integer) partition by range (a);
+create temp table part_tbl1 partition of part_tbl for values from (0) to (100);
+create temp table part_tbl2 partition of part_tbl for values from (100) to (200);
+insert into part_tbl values (10), (110);
+analyze part_tbl;
+set enable_partitionwise_join to on;
+explain (costs off)
+select * from part_tbl t1
+    left join part_tbl t2 on t1.a = t2.a
+    left join part_tbl t3 on t2.a = t3.a;
+                  QUERY PLAN                  
+----------------------------------------------
+ Append
+   ->  Nested Loop Left Join
+         Join Filter: (t2_1.a = t3_1.a)
+         ->  Nested Loop Left Join
+               Join Filter: (t1_1.a = t2_1.a)
+               ->  Seq Scan on part_tbl1 t1_1
+               ->  Seq Scan on part_tbl1 t2_1
+         ->  Seq Scan on part_tbl1 t3_1
+   ->  Nested Loop Left Join
+         Join Filter: (t2_2.a = t3_2.a)
+         ->  Nested Loop Left Join
+               Join Filter: (t1_2.a = t2_2.a)
+               ->  Seq Scan on part_tbl2 t1_2
+               ->  Seq Scan on part_tbl2 t2_2
+         ->  Seq Scan on part_tbl2 t3_2
+(15 rows)
+
+select * from part_tbl t1
+    left join part_tbl t2 on t1.a = t2.a
+    left join part_tbl t3 on t2.a = t3.a;
+  a  |  a  |  a  
+-----+-----+-----
+  10 |  10 |  10
+ 110 | 110 | 110
+(2 rows)
+
+rollback;
 --
 -- check a case where we formerly got confused by conflicting sort orders
 -- in redundant merge join path keys
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 3e5032b04d..ee927e338e 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -550,6 +550,30 @@ select * from onek t1
       (select * from onek t3 where t3.two = t2.two offset 0) s
       on t2.unique1 = 1;
 
+--
+-- check a case where we built wrong join relids for child join
+--
+begin;
+
+create temp table part_tbl (a integer) partition by range (a);
+create temp table part_tbl1 partition of part_tbl for values from (0) to (100);
+create temp table part_tbl2 partition of part_tbl for values from (100) to (200);
+insert into part_tbl values (10), (110);
+analyze part_tbl;
+
+set enable_partitionwise_join to on;
+
+explain (costs off)
+select * from part_tbl t1
+    left join part_tbl t2 on t1.a = t2.a
+    left join part_tbl t3 on t2.a = t3.a;
+
+select * from part_tbl t1
+    left join part_tbl t2 on t1.a = t2.a
+    left join part_tbl t3 on t2.a = t3.a;
+
+rollback;
+
 --
 -- check a case where we formerly got confused by conflicting sort orders
 -- in redundant merge join path keys
-- 
2.31.0