v1-0001-Deduplicate-RestrictInfos-differing-only-in-outer.patch

From cad833802bc1a31c06fd80c3cded9fb6f5abbe19 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Wed, 22 Apr 2026 17:28:23 +0300
Subject: [PATCH v1] Deduplicate RestrictInfos differing only in outer_relids
 in SJE

During self-join elimination, an EC-derived IS NOT NULL clause from an
inner join and an identical clause originating from an enclosing outer
join's ON clause may both be moved to the surviving relation.  They
are logically equivalent but carry different outer_relids /
incompatible_relids (placement constraints, not semantic content), so
restrict_infos_logically_equal() used to keep both, producing a
duplicated filter in the plan.

Compare only the fields that describe the filter's semantics and
placement level (clause, clause_relids, required_relids,
security_level, is_pushed_down, has_clone, is_clone).  When a
duplicate is found, merge outer_relids and incompatible_relids of the
two RestrictInfos by union into the surviving one, so the kept clause
carries the strictest placement allowed by either original.

Bug: #19435
---
 src/backend/optimizer/plan/analyzejoins.c | 68 +++++++++++++++++------
 src/test/regress/expected/join.out        | 20 +++++++
 src/test/regress/sql/join.sql             | 10 ++++
 3 files changed, 82 insertions(+), 16 deletions(-)

diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index 03056bdf3e0..65e71519b30 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -1651,25 +1651,44 @@ update_eclasses(EquivalenceClass *ec, int from, int to)
 }
 
 /*
- * "Logically" compares two RestrictInfo's ignoring the 'rinfo_serial' field,
- * which makes almost every RestrictInfo unique.  This type of comparison is
- * useful when removing duplicates while moving RestrictInfo's from removed
- * relation to remaining relation during self-join elimination.
+ * Compare two RestrictInfos for logical equivalence during self-join
+ * elimination dedup.  Two clauses are logically equivalent if they apply
+ * the same test at the same semantic placement level, i.e. their clause
+ * content and the fields that determine when/where the filter must run
+ * all match.
  *
- * XXX: In the future, we might remove the 'rinfo_serial' field completely and
- * get rid of this function.
+ * outer_relids and incompatible_relids may legitimately differ between
+ * two such clauses: they are placement *constraints* rather than semantic
+ * content, and the same logical filter can arise from different levels of
+ * the join tree (e.g. an EC-derived IS NOT NULL from an inner join vs. an
+ * original ON-clause from an enclosing outer join) with different
+ * constraint sets.  Callers must combine these constraints (by union)
+ * when treating one clause as redundant with another, so that the
+ * surviving clause carries the strictest placement allowed by either
+ * original.  See add_non_redundant_clauses().
+ *
+ * rinfo_serial is skipped because it is only an identifier.  Fields
+ * marked pg_node_attr(equal_ignore) in the struct definition (cached
+ * costs, selectivities, EC back-pointers, derived relid subsets) are
+ * likewise skipped since they are either caches or redundant with fields
+ * that are checked here.
  */
 static bool
 restrict_infos_logically_equal(RestrictInfo *a, RestrictInfo *b)
 {
-	int			saved_rinfo_serial = a->rinfo_serial;
-	bool		result;
-
-	a->rinfo_serial = b->rinfo_serial;
-	result = equal(a, b);
-	a->rinfo_serial = saved_rinfo_serial;
-
-	return result;
+	if (a->security_level != b->security_level)
+		return false;
+	if (a->is_pushed_down != b->is_pushed_down)
+		return false;
+	if (a->has_clone != b->has_clone)
+		return false;
+	if (a->is_clone != b->is_clone)
+		return false;
+	if (!bms_equal(a->clause_relids, b->clause_relids))
+		return false;
+	if (!bms_equal(a->required_relids, b->required_relids))
+		return false;
+	return equal(a->clause, b->clause);
 }
 
 /*
@@ -1682,6 +1701,13 @@ restrict_infos_logically_equal(RestrictInfo *a, RestrictInfo *b)
  * would have been better to avoid calling the equal() function here, but
  * it's the only way to detect duplicated inequality expressions.
  *
+ * When a candidate is found to be logically equivalent to an already-kept
+ * clause but carries different outer_relids / incompatible_relids, those
+ * placement-constraint sets are merged by union into the surviving
+ * clause.  The union is the strictest common constraint: any placement
+ * allowed by both originals remains allowed, so keeping only the merged
+ * clause reproduces the same filtering without relaxing any prohibition.
+ *
  * (*keep_rinfo_list) is given by pointer because it might be altered by
  * distribute_restrictinfo_to_rels().
  */
@@ -1705,9 +1731,19 @@ add_non_redundant_clauses(PlannerInfo *root,
 
 			if (src == rinfo ||
 				(rinfo->parent_ec != NULL &&
-				 src->parent_ec == rinfo->parent_ec) ||
-				restrict_infos_logically_equal(rinfo, src))
+				 src->parent_ec == rinfo->parent_ec))
+			{
+				is_redundant = true;
+				break;
+			}
+
+			if (restrict_infos_logically_equal(rinfo, src))
 			{
+				src->outer_relids = bms_union(src->outer_relids,
+											  rinfo->outer_relids);
+				src->incompatible_relids =
+					bms_union(src->incompatible_relids,
+							  rinfo->incompatible_relids);
 				is_redundant = true;
 				break;
 			}
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 78bf022f7b4..2635e26afd8 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -8147,6 +8147,26 @@ SELECT 1 AS c1 FROM sl sl1 LEFT JOIN (sl AS sl2 NATURAL JOIN sl AS sl3)
          ->  Seq Scan on sl sl4
 (7 rows)
 
+-- An EC-derived IS NOT NULL (from the NATURAL JOIN's self-join removal) may
+-- collide with an identical IS NOT NULL originating from an enclosing outer
+-- join's ON clause.  The two clauses differ only in outer_relids, so they
+-- must be merged (by union) rather than kept as duplicates.  Bug #19435.
+EXPLAIN (COSTS OFF)
+SELECT 1 AS c1 FROM (sl AS sl0 RIGHT JOIN
+  ((sl AS sl1 NATURAL JOIN sl AS sl2)
+   RIGHT JOIN sl AS sl3 ON sl1.bool_col IS NOT NULL)
+  ON sl1.bool_col);
+                                                 QUERY PLAN                                                 
+------------------------------------------------------------------------------------------------------------
+ Nested Loop Left Join
+   ->  Seq Scan on sl sl3
+   ->  Nested Loop Left Join
+         Join Filter: sl2.bool_col
+         ->  Seq Scan on sl sl2
+               Filter: ((bool_col IS NOT NULL) AND (a IS NOT NULL) AND (b IS NOT NULL) AND (c IS NOT NULL))
+         ->  Seq Scan on sl sl0
+(7 rows)
+
 -- Check optimization disabling if it will violate special join conditions.
 -- Two identical joined relations satisfies self join removal conditions but
 -- stay in different special join infos.
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index fae19113cef..89c54a6e172 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -3185,6 +3185,16 @@ EXPLAIN (COSTS OFF)
 SELECT 1 AS c1 FROM sl sl1 LEFT JOIN (sl AS sl2 NATURAL JOIN sl AS sl3)
   ON sl2.bool_col LEFT JOIN sl AS sl4 ON sl2.bool_col;
 
+-- An EC-derived IS NOT NULL (from the NATURAL JOIN's self-join removal) may
+-- collide with an identical IS NOT NULL originating from an enclosing outer
+-- join's ON clause.  The two clauses differ only in outer_relids, so they
+-- must be merged (by union) rather than kept as duplicates.  Bug #19435.
+EXPLAIN (COSTS OFF)
+SELECT 1 AS c1 FROM (sl AS sl0 RIGHT JOIN
+  ((sl AS sl1 NATURAL JOIN sl AS sl2)
+   RIGHT JOIN sl AS sl3 ON sl1.bool_col IS NOT NULL)
+  ON sl1.bool_col);
+
 -- Check optimization disabling if it will violate special join conditions.
 -- Two identical joined relations satisfies self join removal conditions but
 -- stay in different special join infos.
-- 
2.39.5 (Apple Git-154)