From 802c20938f04d677d44ca0105c1589efb89a4cc5 Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Fri, 17 Mar 2023 10:29:27 +0800
Subject: [PATCH v2] Check group_clause_relids to see if a clause is computable

---
 src/backend/optimizer/plan/initsplan.c    | 15 ++++++++++++
 src/backend/optimizer/util/relnode.c      |  2 +-
 src/backend/optimizer/util/restrictinfo.c |  1 +
 src/include/nodes/pathnodes.h             |  6 +++++
 src/test/regress/expected/join.out        | 28 +++++++++++++++++++++++
 src/test/regress/sql/join.sql             | 10 ++++++++
 6 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index c6c21a7338..d9527caa01 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -123,6 +123,7 @@ static void distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 									bool allow_equivalence,
 									bool has_clone,
 									bool is_clone,
+									Relids group_clause_relids,
 									List **postponed_oj_qual_list);
 static bool check_redundant_nullability_qual(PlannerInfo *root, Node *clause);
 static Relids get_join_domain_min_rels(PlannerInfo *root, Relids domain_relids);
@@ -2100,6 +2101,14 @@ distribute_quals_to_rels(PlannerInfo *root, List *clauses,
 						 List **postponed_oj_qual_list)
 {
 	ListCell   *lc;
+	Relids		group_clause_relids = NULL;
+
+	/*
+	 * Retrieve all relids mentioned within the clauses if they are clone
+	 * clauses.
+	 */
+	if (has_clone || is_clone)
+		group_clause_relids = pull_varnos(root, (Node *) clauses);
 
 	foreach(lc, clauses)
 	{
@@ -2115,6 +2124,7 @@ distribute_quals_to_rels(PlannerInfo *root, List *clauses,
 								allow_equivalence,
 								has_clone,
 								is_clone,
+								group_clause_relids,
 								postponed_oj_qual_list);
 	}
 }
@@ -2147,6 +2157,8 @@ distribute_quals_to_rels(PlannerInfo *root, List *clauses,
  *		EquivalenceClass
  * 'has_clone': has_clone property to assign to the qual
  * 'is_clone': is_clone property to assign to the qual
+ * 'group_clause_relids': relids actually referenced in all the clauses from
+ *		the same jointree node if a cloned clause, otherwise NULL
  * 'postponed_oj_qual_list': if not NULL, non-degenerate outer join clauses
  *		should be added to this list instead of being processed (list entries
  *		are just the bare clauses)
@@ -2170,6 +2182,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 						bool allow_equivalence,
 						bool has_clone,
 						bool is_clone,
+						Relids group_clause_relids,
 						List **postponed_oj_qual_list)
 {
 	Relids		relids;
@@ -2393,6 +2406,8 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 	/* Apply appropriate clone marking, too */
 	restrictinfo->has_clone = has_clone;
 	restrictinfo->is_clone = is_clone;
+	/* and set the group clause relids */
+	restrictinfo->group_clause_relids = group_clause_relids;
 
 	/*
 	 * If it's a join clause, add vars used in the clause to targetlists of
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index 68fd033595..06d280e477 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -1311,7 +1311,7 @@ subbuild_joinrel_restrictlist(PlannerInfo *root,
 				Assert(!RINFO_IS_PUSHED_DOWN(rinfo, joinrel->relids));
 				if (!bms_is_subset(rinfo->required_relids, both_input_relids))
 					continue;
-				if (!clause_is_computable_at(root, rinfo->clause_relids,
+				if (!clause_is_computable_at(root, rinfo->group_clause_relids,
 											 both_input_relids))
 					continue;
 			}
diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index c44bd2f815..16eadc8410 100644
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -119,6 +119,7 @@ make_restrictinfo_internal(PlannerInfo *root,
 	restrictinfo->can_join = false; /* may get set below */
 	restrictinfo->security_level = security_level;
 	restrictinfo->outer_relids = outer_relids;
+	restrictinfo->group_clause_relids = NULL;
 
 	/*
 	 * If it's potentially delayable by lower-level security quals, figure out
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index d61a62da19..33d84959f6 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2553,6 +2553,12 @@ typedef struct RestrictInfo
 	Relids		left_relids pg_node_attr(equal_ignore);
 	Relids		right_relids pg_node_attr(equal_ignore);
 
+	/*
+	 * Relids actually referenced in all the clauses from the same jointree
+	 * node.  This field is set only for clauses that have multiple versions.
+	 */
+	Relids		group_clause_relids pg_node_attr(copy_as_scalar, equal_ignore, read_write_ignore);
+
 	/*
 	 * Modified clause with RestrictInfos.  This field is NULL unless clause
 	 * is an OR clause.
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 5d59ed7890..824561c2eb 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -5173,6 +5173,34 @@ from int8_tbl t1
          ->  Seq Scan on onek t4
 (13 rows)
 
+-- another example of wrong outer join qual placement
+explain (costs off)
+select *
+from int8_tbl t1
+  left join (int8_tbl t2 left join onek t3 on t2.q1 > t3.unique1)
+    on t1.q2 = t2.q2
+  left join onek t4
+    on t2.q2 = 1 and t3.unique2 = 1
+where t1.q2 = coalesce(t2.q2,1);
+                          QUERY PLAN                          
+--------------------------------------------------------------
+ Nested Loop Left Join
+   Join Filter: ((t2.q2 = 1) AND (t3.unique2 = 1))
+   ->  Nested Loop Left Join
+         ->  Hash Left Join
+               Hash Cond: (t1.q2 = t2.q2)
+               Filter: (t1.q2 = COALESCE(t2.q2, '1'::bigint))
+               ->  Seq Scan on int8_tbl t1
+               ->  Hash
+                     ->  Seq Scan on int8_tbl t2
+         ->  Bitmap Heap Scan on onek t3
+               Recheck Cond: (t2.q1 > unique1)
+               ->  Bitmap Index Scan on onek_unique1
+                     Index Cond: (unique1 < t2.q1)
+   ->  Materialize
+         ->  Seq Scan on onek t4
+(15 rows)
+
 -- More tests of correct placement of pseudoconstant quals
 -- simple constant-false condition
 explain (costs off)
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index a630f58b57..be1e03982e 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1862,6 +1862,16 @@ from int8_tbl t1
   left join onek t4
     on t2.q2 < t3.unique2;
 
+-- another example of wrong outer join qual placement
+explain (costs off)
+select *
+from int8_tbl t1
+  left join (int8_tbl t2 left join onek t3 on t2.q1 > t3.unique1)
+    on t1.q2 = t2.q2
+  left join onek t4
+    on t2.q2 = 1 and t3.unique2 = 1
+where t1.q2 = coalesce(t2.q2,1);
+
 -- More tests of correct placement of pseudoconstant quals
 
 -- simple constant-false condition
-- 
2.31.0

