v3-0001-Don-t-constrain-self-join-removal-due-to-PHVs.patch

From dd5ad58abc04beefde1ab3755f3d58451d879be0 Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Mon, 20 Nov 2023 16:14:24 +0800
Subject: [PATCH v3] Don't constrain self join removal due to PHVs

---
 src/backend/optimizer/plan/analyzejoins.c | 38 +++++++++++++---------
 src/test/regress/expected/join.out        | 39 ++++++++++++++++-------
 src/test/regress/sql/join.sql             | 10 ++++--
 3 files changed, 58 insertions(+), 29 deletions(-)

diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index b9be19a687..9360177af8 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -453,7 +453,7 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
 	{
 		PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(l);
 
-		Assert(!bms_is_member(relid, phinfo->ph_lateral));
+		Assert(sjinfo == NULL || !bms_is_member(relid, phinfo->ph_lateral));
 		if (bms_is_subset(phinfo->ph_needed, joinrelids) &&
 			bms_is_member(relid, phinfo->ph_eval_at) &&
 			!bms_is_member(ojrelid, phinfo->ph_eval_at))
@@ -472,6 +472,8 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
 			phinfo->ph_needed = replace_relid(phinfo->ph_needed, relid, subst);
 			phinfo->ph_needed = replace_relid(phinfo->ph_needed, ojrelid, subst);
 			/* ph_needed might or might not become empty */
+			phinfo->ph_lateral = replace_relid(phinfo->ph_lateral, relid, subst);
+			/* ph_lateral might or might not be empty */
 			phv->phrels = replace_relid(phv->phrels, relid, subst);
 			phv->phrels = replace_relid(phv->phrels, ojrelid, subst);
 			Assert(!bms_is_empty(phv->phrels));
@@ -1456,7 +1458,16 @@ replace_varno_walker(Node *node, ReplaceVarnoContext *ctx)
 		}
 		return false;
 	}
-	if (IsA(node, RestrictInfo))
+	else if (IsA(node, PlaceHolderVar))
+	{
+		PlaceHolderVar	   *phv = (PlaceHolderVar *) node;
+
+		phv->phrels = replace_relid(phv->phrels, ctx->from, ctx->to);
+		phv->phnullingrels = replace_relid(phv->phnullingrels, ctx->from, ctx->to);
+
+		/* fall through to recurse into the placeholder's expression */
+	}
+	else if (IsA(node, RestrictInfo))
 	{
 		RestrictInfo *rinfo = (RestrictInfo *) node;
 		int			relid = -1;
@@ -1658,6 +1669,15 @@ sje_walker(Node *node, ReplaceVarnoContext *ctx)
 		}
 		return false;
 	}
+	else if (IsA(node, PlaceHolderVar))
+	{
+		PlaceHolderVar	   *phv = (PlaceHolderVar *) node;
+
+		phv->phrels = replace_relid(phv->phrels, ctx->from, ctx->to);
+		phv->phnullingrels = replace_relid(phv->phnullingrels, ctx->from, ctx->to);
+
+		/* fall through to recurse into the placeholder's expression */
+	}
 	return expression_tree_walker(node, sje_walker, (void *) ctx);
 }
 
@@ -2125,20 +2145,8 @@ remove_self_joins_one_group(PlannerInfo *root, Relids relids)
 			joinrelids = bms_add_member(joinrelids, k);
 
 			/*
-			 * Be safe to do not remove tables participated in complicated PH
+			 * PHVs should not impose any constraints on removing self joins.
 			 */
-			foreach(lc, root->placeholder_list)
-			{
-				PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(lc);
-
-				/* there isn't any other place to eval PHV */
-				if (bms_is_subset(phinfo->ph_eval_at, joinrelids) ||
-					bms_is_subset(phinfo->ph_needed, joinrelids) ||
-					bms_is_member(r, phinfo->ph_lateral))
-					break;
-			}
-			if (lc)
-				continue;
 
 			/*
 			 * At this stage, joininfo lists of inner and outer can contain
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 2c73270143..69427287ff 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -6821,20 +6821,37 @@ on true;
                            Filter: (id IS NOT NULL)
 (8 rows)
 
--- Check that SJE does not remove self joins if a PHV references the removed
--- rel laterally.
-explain (costs off)
+-- Check that PHVs do not impose any constraints on removing self joins
+explain (verbose, costs off)
 select * from emp1 t1 join emp1 t2 on t1.id = t2.id left join
     lateral (select t1.id as t1id, * from generate_series(1,1) t3) s on true;
-                    QUERY PLAN                     
----------------------------------------------------
+                        QUERY PLAN                        
+----------------------------------------------------------
  Nested Loop Left Join
-   ->  Nested Loop
-         ->  Seq Scan on emp1 t1
-         ->  Index Scan using emp1_pkey on emp1 t2
-               Index Cond: (id = t1.id)
-   ->  Function Scan on generate_series t3
-(6 rows)
+   Output: t2.id, t2.code, t2.id, t2.code, (t2.id), t3.t3
+   ->  Seq Scan on public.emp1 t2
+         Output: t2.id, t2.code
+         Filter: (t2.id IS NOT NULL)
+   ->  Function Scan on pg_catalog.generate_series t3
+         Output: t3.t3, t2.id
+         Function Call: generate_series(1, 1)
+(8 rows)
+
+explain (verbose, costs off)
+select * from generate_series(1,10) t1(id) left join
+    lateral (select t1.id as t1id, t2.id from emp1 t2 join emp1 t3 on t2.id = t3.id)
+on true;
+                      QUERY PLAN                      
+------------------------------------------------------
+ Nested Loop Left Join
+   Output: t1.id, (t1.id), t3.id
+   ->  Function Scan on pg_catalog.generate_series t1
+         Output: t1.id
+         Function Call: generate_series(1, 10)
+   ->  Seq Scan on public.emp1 t3
+         Output: t3.id, t1.id
+         Filter: (t3.id IS NOT NULL)
+(8 rows)
 
 -- We can remove the join even if we find the join can't duplicate rows and
 -- the base quals of each side are different.  In the following case we end up
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 8a8a63bd2f..9d6fce21de 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -2600,12 +2600,16 @@ select * from emp1 t1 left join
         on true)
 on true;
 
--- Check that SJE does not remove self joins if a PHV references the removed
--- rel laterally.
-explain (costs off)
+-- Check that PHVs do not impose any constraints on removing self joins
+explain (verbose, costs off)
 select * from emp1 t1 join emp1 t2 on t1.id = t2.id left join
     lateral (select t1.id as t1id, * from generate_series(1,1) t3) s on true;
 
+explain (verbose, costs off)
+select * from generate_series(1,10) t1(id) left join
+    lateral (select t1.id as t1id, t2.id from emp1 t2 join emp1 t3 on t2.id = t3.id)
+on true;
+
 -- We can remove the join even if we find the join can't duplicate rows and
 -- the base quals of each side are different.  In the following case we end up
 -- moving quals over to s1 to make it so it can't match any rows.
-- 
2.31.0