Subject: Re: BUG #19435: Error: "No relation entry for relid 2" Triggered by Complex Join with Self-Referencing Tables

From: Alexander Korotkov <[email-masked]>
Date: Wed, 18 Mar 2026 10:40:17 +0200

Hi, Tender!

On Wed, Mar 18, 2026 at 7:44 AM Tender Wang <[email-masked]> wrote:
>
> Tender Wang <[email-masked]> 于2026年3月18日周三 09:12写道：
> >
> > Alexander Korotkov <[email-masked]> 于2026年3月18日周三 01:46写道：
> > >
> > > On Tue, Mar 17, 2026 at 3:30 PM Tender Wang <[email-masked]> wrote:
> > > > Kirill Reshke <[email-masked]> 于2026年3月17日周二 21:24写道：
> > > > >
> > > > > On Tue, 17 Mar 2026 at 18:20, Tender Wang <[email-masked]> wrote:
> > > > > Yes, it looks like your analysis is valid. Will you share a patch for
> > > > > updating `clause` ?
> > > > >
> > > > Sorry, it's already the middle of the night here, so I'm afraid I
> > > > don't have much time to work on this right now.
> > >
> > > Thank you for your research.  I've written a simple draft patch.  It
> > > fixes the reported case, but I doubt it is correct in general.  I'll
> > > continue the investigation.
> > I tried the above fix, no error again.  But I got a plan like this:
> >                                         QUERY PLAN
> > -------------------------------------------------------------------------------------------
> >  Nested Loop Left Join  (cost=0.00..115164616.71 rows=7458350250 width=4)
> >    ->  Seq Scan on pg_table_a tom3  (cost=0.00..34.70 rows=2470 width=0)
> >    ->  Materialize  (cost=0.00..99509.82 rows=3019575 width=0)
> >          ->  Nested Loop Left Join  (cost=0.00..75564.95 rows=3019575 width=0)
> >                Join Filter: tom2.col_bool
> >                ->  Seq Scan on pg_table_a tom2  (cost=0.00..34.70
> > rows=2445 width=5)
> >                      Filter: ((col_bool IS NOT NULL) AND (col_bool IS NOT NULL))
> >                ->  Materialize  (cost=0.00..47.05 rows=2470 width=0)
> >                      ->  Seq Scan on pg_table_a tom0
> > (cost=0.00..34.70 rows=2470 width=0)
> > (9 rows)
> >
> > Filter: ((col_bool IS NOT NULL) AND (col_bool IS NOT NULL))
> > This makes me unhappy.
> > Your patch gets the same plan.
> >
> In replace_relid_callback(), we add NullTest to rinfo, but it is not a
> logical equal check by restrict_infos_logically_equal().
> I think for baserestrictinfo, we can just use rinfo->clause, no need
> to check the equality of RestrictInfo.
>
> I tried this way, the plan looks as follows:
>                                         QUERY PLAN
> -------------------------------------------------------------------------------------------
>  Nested Loop Left Join  (cost=0.00..115776846.35 rows=7498006100 width=4)
>    ->  Seq Scan on pg_table_a tom3  (cost=0.00..34.70 rows=2470 width=0)
>    ->  Materialize  (cost=0.00..100038.47 rows=3035630 width=0)
>          ->  Nested Loop Left Join  (cost=0.00..75966.32 rows=3035630 width=0)
>                Join Filter: tom2.col_bool
>                ->  Seq Scan on pg_table_a tom2  (cost=0.00..34.70
> rows=2458 width=5)
>                      Filter: (col_bool IS NOT NULL)
>                ->  Materialize  (cost=0.00..47.05 rows=2470 width=0)
>                      ->  Seq Scan on pg_table_a tom0
> (cost=0.00..34.70 rows=2470 width=0)
> (9 rows)
>
> No redundant filter anymore.
>
> Please see the attached patch.

What about being more generic and call ChangeVarNodes_walker() for the
node in ChangeVarNodesWalkExpression()?  It also works with out case
and avoids code duplication.

Changes in restrict_infos_logically_equal() makes me a bit uneasy.  I
see, restictinfo's are different by their outer_relids.  Why
outer_relids doesn't matter when required_relids is singleton?  More
general when do outer_relids matter for add_non_redundant_clauses() if
we're putting restictinfo's into a single list anyway?

------
Regards,
Alexander Korotkov
Supabase

---

From: Tender Wang <[email-masked]>
Date: Wed, 18 Mar 2026 17:46:51 +0800

> and avoids code duplication.

Works for me.

> we're putting restictinfo's into a single list anyway?

(gdb) call nodeToString(binfo_candidates )
{RESTRICTINFO :clause {NULLTEST :arg {VAR :varno 3 :varattno 2
:vartype 16 :vartypmod -1 :varcollid 0 :varnullingrels (b)
:varlevelsup 0 :varreturningtype 0 :varnosyn 3 :varattnosyn 2
:location -1} :nulltesttype 1 :argisrow false :location -1}
:is_pushed_down true :can_join false :pseudoconstant false :has_clone
false :is_clone false :leakproof false :has_volatile 0 :security_level
0 :num_base_rels 1 :clause_relids (b 3) :required_relids (b 3)
:incompatible_relids (b) :outer_relids (b 5) :left_relids (b)
:right_relids (b) :orclause <> :rinfo_serial 3 :eval_cost.startup -1
:eval_cost.per_tuple 0 :norm_selec -1 :outer_selec -1 :mergeopfamilies
<> :left_em <> :right_em <> :outer_is_left false :hashjoinoperator 0
:left_bucketsize -1 :right_bucketsize -1 :left_mcvfreq -1
:right_mcvfreq -1 :left_hasheqoperator 0 :right_hasheqoperator 0}

{RESTRICTINFO :clause {NULLTEST :arg {VAR :varno 3 :varattno 2
:vartype 16 :vartypmod -1 :varcollid 0 :varnullingrels (b)
:varlevelsup 0 :varreturningtype 0 :varnosyn 3 :varattnosyn 2
:location -1} :nulltesttype 1 :argisrow false :location -1}
:is_pushed_down true :can_join true :pseudoconstant false :has_clone
false :is_clone false :leakproof false :has_volatile 2 :security_level
0 :num_base_rels 1 :clause_relids (b 3) :required_relids (b 3)
:incompatible_relids (b) :outer_relids (b) :left_relids (b 3)
:right_relids (b 3) :orclause <> :rinfo_serial 6 :eval_cost.startup -1
:eval_cost.per_tuple 0 :norm_selec -1 :outer_selec -1 :mergeopfamilies
<> :left_em <> :right_em <> :outer_is_left true :hashjoinoperator 91
:left_bucketsize -1 :right_bucketsize -1 :left_mcvfreq -1
:right_mcvfreq -1 :left_hasheqoperator 91 :right_hasheqoperator 91})"

The first is tom1.col_bool IS NOT NULL,  its outer_relids (b 5)  is
not empty because it's in the nullable side.
The second is added in replace_relid_callback().  Some fields do not match.

In this case, tom1.col_bool IS NOT NULL becomes a filter clause, not a
join clause, and is safe to only check rinfo->clause.
I think it's better to add is_pushed_down == true check, for example:
...
-       if (bms_membership(a->required_relids) == BMS_SINGLETON)
+       if (bms_membership(a->required_relids) == BMS_SINGLETON &&
+               a->is_pushed_down &&
+               b->is_pushed_down)
...

Any thought?

-- 
Thanks,
Tender Wang

---

From: Andrei Lepikhov <[email-masked]>
Date: Wed, 18 Mar 2026 13:18:27 +0100

> and avoids code duplication.
I’ve reached the same conclusion. We lost a possible case when the 
RestrictInfo→clause contains a bare Var that isn’t pushed into either 
the left or right subtree.
I think we can fix this by replacing the expression walker with 
ChangeVarNodes_walker().
What is the reason for the second change? Tender, can you show us how to 
reproduce the issue so we can support your update to 
restrict_infos_logically_equal? If we include it, we should add a test.

-- 
regards, Andrei Lepikhov,
pgEdge

diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index fe89754a73c..610edf10284 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -744,9 +744,7 @@ ChangeVarNodes(Node *node, int rt_index, int new_index, int sublevels_up)
 bool
 ChangeVarNodesWalkExpression(Node *node, ChangeVarNodes_context *context)
 {
-	return expression_tree_walker(node,
-								  ChangeVarNodes_walker,
-								  (void *) context);
+	return ChangeVarNodes_walker(node, context);
 }
 
 /*

---

From: Alexander Korotkov <[email-masked]>
Date: Wed, 18 Mar 2026 14:21:34 +0200

> I think we can fix this by replacing the expression walker with

I see.  This is even better.

> restrict_infos_logically_equal? If we include it, we should add a test.

I think Tender already shown this in [1].  The same qual is present
twice in the plan.

Links.
1. https://www.postgresql.org/message-id/CAHewXN%3D7kDJjUcgEm%2B6qhaKOXuqzvhRqAAKdafNCRgn0yH7BGg%40mail.gmail.com

------
Regards,
Alexander Korotkov
Supabase

---

From: Andrei Lepikhov <[email-masked]>
Date: Wed, 18 Mar 2026 14:31:00 +0100

> I think Tender already shown this in [1].  The same qual is present
Got it. I suggest making this a separate commit to keep the history 
clear. Let me share a draft with a test case for the bug fix first.

From 86e65e16c76fe404cb5a16dd0d89a583da9b62e6 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email-masked]>
Date: Wed, 18 Mar 2026 14:23:46 +0100
Subject: [PATCH] Fix self-join removal to update bare Var references in join
 clauses

Self-join removal failed to update Var nodes when the join clause was a
bare Var (e.g., ON t1.bool_col) rather than an expression containing
Vars.  ChangeVarNodesWalkExpression() used expression_tree_walker(),
which descends into child nodes but does not process the top-level node
itself.  When a bare Var referencing the removed relation appeared as
the clause, its varno was left unchanged, leading to "no relation entry
for relid N" errors.

Fix by calling ChangeVarNodes_walker() directly instead of
expression_tree_walker(), so the top-level node is also processed.

Bug: #19435
Author: Alexander Korotkov <[email-masked]>
Reviewed-by: Tender Wang <[email-masked]>
Reviewed-by: Andrei Lepikhov <[email-masked]>
Reviewed-by: Kirill Reshke <[email-masked]>
Discussion: https://www.postgresql.org/message-id/flat/19435-3cc1a87f291129f1%40postgresql.org
---
 src/backend/rewrite/rewriteManip.c |  4 +---
 src/test/regress/expected/join.out | 16 ++++++++++++++++
 src/test/regress/sql/join.sql      |  6 ++++++
 3 files changed, 23 insertions(+), 3 deletions(-)

 /*
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index ea3dabff77c..bf758f881ac 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -8091,6 +8091,22 @@ WHERE q0.a = 1;
    ->  Seq Scan on sj n1
 (7 rows)
 
+-- BUG #19435 Do not forget to replace relid in bare Var join clause
+ALTER TABLE sl ADD COLUMN cool_bool boolean;
+EXPLAIN (COSTS OFF)
+SELECT 1 AS c1 FROM sl sl1 LEFT JOIN (sl AS sl2 NATURAL JOIN sl AS sl3)
+  ON sl2.cool_bool LEFT JOIN sl AS sl4 ON sl2.cool_bool;
+                                                        QUERY PLAN                                                         
+---------------------------------------------------------------------------------------------------------------------------
+ Nested Loop Left Join
+   ->  Seq Scan on sl sl1
+   ->  Nested Loop Left Join
+         Join Filter: sl3.cool_bool
+         ->  Seq Scan on sl sl3
+               Filter: (cool_bool AND (a IS NOT NULL) AND (b IS NOT NULL) AND (c IS NOT NULL) AND (cool_bool IS NOT NULL))
+         ->  Seq Scan on sl sl4
+(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 ad90c326c00..faf07f7c4ab 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -3156,6 +3156,12 @@ SELECT * FROM
 (SELECT n2.a FROM sj n1, sj n2 WHERE n1.a <> n2.a) q0, sl
 WHERE q0.a = 1;
 
+-- BUG #19435 Do not forget to replace relid in bare Var join clause
+ALTER TABLE sl ADD COLUMN cool_bool boolean;
+EXPLAIN (COSTS OFF)
+SELECT 1 AS c1 FROM sl sl1 LEFT JOIN (sl AS sl2 NATURAL JOIN sl AS sl3)
+  ON sl2.cool_bool LEFT JOIN sl AS sl4 ON sl2.cool_bool;
+
 -- 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.51.0

---

From: Alexander Korotkov <[email-masked]>
Date: Wed, 18 Mar 2026 16:38:15 +0200

> clear. Let me share a draft with a test case for the bug fix first.

Yes, I was also thinking about splitting this into two distinct
commit.  The patch you've attached looks good for me.  I'm going to
push and backpatch it if no objections.  And let's continue the
investigation on restrict_infos_logically_equal().

------
Regards,
Alexander Korotkov
Supabase

---

From: Kirill Reshke <[email-masked]>
Date: Wed, 18 Mar 2026 22:55:22 +0500

> clear. Let me share a draft with a test case for the bug fix first.

Hi!
Is `cool_bool` a typo of `col_bool` in regression test ?

-- 
Best regards,
Kirill Reshke

---

From: Tender Wang <[email-masked]>
Date: Thu, 19 Mar 2026 09:11:10 +0800

> investigation on restrict_infos_logically_equal().

Agree.
And the patch seems to have forgotten to add "Reported by".

-- 
Thanks,
Tender Wang

---

From: Andrei Lepikhov <[email-masked]>
Date: Thu, 19 Mar 2026 08:18:03 +0100

> Is `cool_bool` a typo of `col_bool` in regression test ?

This is actually a happy coincidence, not a typo. We made this mistake 
during initial development because I didn't realise that a boolean 
operation on a bool variable is never wrapped in an expression 
structure. It's a 'cool' example that shows a rare edge case.

 > And the patch seems to have forgotten to add "Reported by".

Yeah, let the committer manage the award part.

---

From: Alexander Korotkov <[email-masked]>
Date: Thu, 19 Mar 2026 15:03:17 +0200

> Yeah, let the committer manage the award part.

I've revised the patch.  Renamed cool_bool to cool_col, added
"Reported by", and revised authors list according to my opinion.

------
Regards,
Alexander Korotkov
Supabase

---

From: Alexander Korotkov <[email-masked]>
Date: Fri, 20 Mar 2026 16:02:10 +0200

> "Reported by", and revised authors list according to my opinion.

OK. I've pushed this.  Let's go back to
restrict_infos_logically_equal().  I'm still not convinced that we
need to check if required_relids is singleton.  Why we can ignore
outer_relids for singleton, but can't do if, for instance, two
relations involved?

------
Regards,
Alexander Korotkov
Supabase

---

From: Tom Lane <[email-masked]>
Date: Fri, 20 Mar 2026 10:30:06 -0400

Alexander Korotkov <[email-masked]> writes:
> OK. I've pushed this.

I don't love this patch.  It fixes the functional problem, but it
does nothing to fix the underlying cause of that problem, namely
the abysmal under-documentation of the ChangeVarNodesXXX functions.
There are very specific assumptions about whether the recursion
is starting at a Query or not, and the code will do the wrong
thing if invoked at the wrong node level.

In particular, imagine that ChangeVarNodes_walker is invoked
directly on a Query node, something that ChangeVarNodesExtended
is careful not to do.  It will increment sublevels_up immediately
and thus process the contents of the Query with sublevels_up==1,
meaning it will not recognize local Vars as needing adjustment.

At the very least we need to add comments, but I wonder if we
don't actually need an Assert that ChangeVarNodesWalkExpression
is not invoked directly on a Query.  It would have done the
right thing before this patch, but now it won't.  That's an
okay tradeoff for fixing the bare-Var case, but not documenting
what you did is not okay.

			regards, tom lane

---

From: Tom Lane <[email-masked]>
Date: Fri, 20 Mar 2026 16:26:13 -0400

> what you did is not okay.

After further contemplation I've decided that an Assert would be
wrong, because it's not impossible that a callback would want
to invoke this on a sub-Query --- for instance, if it wanted to
short-circuit ChangeVarNodes's processing of a SubLink node,
it would need to do that.  The key point is that if we do see a
Query node here, we will treat it as a sub-query not a top-level
query, which also justifies skipping the work that
ChangeVarNodesExtended does on a top-level Query.  So we just
need a comment explaining that.  I'm thinking about the attached.

(BTW, by this reasoning the previous implementation of
ChangeVarNodesWalkExpression was doubly wrong, since it would
have done the wrong thing at a Query node as well as a Var node.)

			regards, tom lane

diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index 7249ffbfb36..dc803a17037 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -672,7 +672,7 @@ ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
  * value indicating if the given node should be skipped from further processing
  * by ChangeVarNodes_walker.  The callback is called only for expressions and
  * other children nodes of a Query processed by a walker.  Initial processing
- * of the root Query doesn't involve the callback.
+ * of the root Query node doesn't invoke the callback.
  */
 void
 ChangeVarNodesExtended(Node *node, int rt_index, int new_index,
@@ -737,9 +737,16 @@ ChangeVarNodes(Node *node, int rt_index, int new_index, int sublevels_up)
 }
 
 /*
- * ChangeVarNodesWalkExpression - process expression within the custom
- *								  callback provided to the
- *								  ChangeVarNodesExtended.
+ * ChangeVarNodesWalkExpression - process subexpression within a callback
+ *								  function passed to ChangeVarNodesExtended.
+ *
+ * This is intended to be used by a callback that needs to recursively
+ * process subexpressions of some node being visited by an outer
+ * ChangeVarNodesExtended call (not letting ChangeVarNodes_walker do that).
+ * Hence, we invoke ChangeVarNodes_walker directly.  This means that if
+ * the passed Node is a Query node, it will be treated as a sub-Query,
+ * so sublevels_up will be incremented immediately.  Do not apply this
+ * to a top-level Query node, or you'll likely get wrong results.
  */
 bool
 ChangeVarNodesWalkExpression(Node *node, ChangeVarNodes_context *context)

---

From: Tender Wang <[email-masked]>
Date: Sun, 22 Mar 2026 13:15:56 +0800

Hi Tom,

> have done the wrong thing at a Query node as well as a Var node.)
Thanks for pointing this out. The attached looks good to me.

Do you have some advice about that the same qual is present
twice in the plan, see [1].
Should we do something in restrict_infos_logically_equal().
Please take a look.
-- 
Thanks,
Tender Wang

---

From: Alexander Korotkov <[email-masked]>
Date: Mon, 23 Mar 2026 23:28:45 +0200

Hi, Tom!

> have done the wrong thing at a Query node as well as a Var node.)

Thank you so much for caring about this.  I agree that this kind of
changes should go with proper comments.

Please, consider my additions to the comment.  They explain why we use
ChangeVarNodes_walker() instead of expression_tree_walker(), and gives
a bit more details about difference in processing of top-level Query
and subquery.

------
Regards,
Alexander Korotkov
Supabase

---

From: Tom Lane <[email-masked]>
Date: Mon, 23 Mar 2026 17:34:31 -0400

> a bit more details about difference in processing of top-level Query

I'm fine with this, although in

+ * ... We invoke ChangeVarNodes_walker directly rather than
+ * expression_tree_walker

I would write "rather than via expression_tree_walker".

			regards, tom lane

---

From: Andrei Lepikhov <[email-masked]>
Date: Thu, 26 Mar 2026 20:59:13 +0100

> outer_relids for singleton, but can't do if, for instance, two

Let's continue. In the attachment, the Tender's proposal that I changed 
a little and added some tests.

As you can see in the tests, the SINGLETON limitation keeps duplicates 
of clauses like 'a.x + b.y = c'.
This example shows the main flaw of this approach. Introducing the 
restrict_infos_logically_equal(), we do a little more job than just the 
equal() routine could because of the context where we call this function 
and on which clauses.
But skipping all other RestrictInfo fields except required_relids seems 
excessive. - see the example with security_level difference - ignoring 
its value, we potentially might remove the clause with enforced security 
level in favour of an unsecured one.
That's more, further some new optimisations might introduce more fields 
into RestrictInfo that should be checked to correctly decide on the 
equality, and we may forget to arrange this specific place.

So, formally it works, and making the following replacement, we close 
the singleton issue:

-       if (bms_membership(a->required_relids) == BMS_SINGLETON &&
-               a->security_level == b->security_level)
+       if (bms_equal(a->required_relids, b->required_relids) &&
+               a->security_level == b->security_level &&
+               a->is_pushed_down == b->is_pushed_down)

but I'm unsure, in general, that this approach is conservative enough. 
Maybe we shouldn’t change this logic and invent one more optimisation 
‘deduplication’ stage later, before the selectivity estimation stage.

From 01b9dc8b728bef49104fe1652841770eddb0c45c Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email-masked]>
Date: Thu, 26 Mar 2026 12:15:01 +0100
Subject: [PATCH] Improve RestrictInfo deduplication after self-join
 elimination

After self-join elimination rewrites varnos, two RestrictInfos can
end up with identical clauses but different metadata (outer_relids,
rinfo_serial).  The previous restrict_infos_logically_equal()
compared full RestrictInfo structs, missing these duplicates and
leaving redundant filter conditions in the plan.

For base restrictions (singleton required_relids), compare only the
clause expression, which is sufficient since these are pushed-down
filters that don't carry join-semantic meaning.  Retain the full
struct comparison for join clauses (non-singleton required_relids)
to avoid incorrectly merging clauses at different join levels.

Also check security_level to prevent merging an RLS policy qual with a
user-written ON clause that happen to look identical after SJE.

Author: Tender Wang <[email-masked]>
Author: Alexander Korotkov <[email-masked]>
Reviewed-by: Andrei Lepikhov <[email-masked]>
Discussion: https://www.postgresql.org/message-id/19435-3cc1a87f291129f1%40postgresql.org
---
 src/backend/optimizer/plan/analyzejoins.c | 16 ++++-
 src/test/regress/expected/join.out        | 78 +++++++++++++++++++++++
 src/test/regress/sql/join.sql             | 48 ++++++++++++++
 3 files changed, 139 insertions(+), 3 deletions(-)

diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index 12e9ed0d0c7..ed35c51a38f 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -1635,9 +1635,19 @@ 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;
+	if (bms_membership(a->required_relids) == BMS_SINGLETON &&
+		a->security_level == b->security_level)
+	{
+		Assert(a->is_pushed_down && b->is_pushed_down);
+
+		result = equal(a->clause, b->clause);
+	}
+	else
+	{
+		a->rinfo_serial = b->rinfo_serial;
+		result = equal(a, b);
+		a->rinfo_serial = saved_rinfo_serial;
+	}
 
 	return result;
 }
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 84872c6f04e..88766e5e626 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -8108,6 +8108,84 @@ SELECT 1 AS c1 FROM sl sl1 LEFT JOIN (sl AS sl2 NATURAL JOIN sl AS sl3)
          ->  Seq Scan on sl sl4
 (7 rows)
 
+-- SJE: after self-join removal merges sl7 and sl6, the JOIN
+-- produces a pushed-down (bool_col IS NOT NULL) that duplicates the ON
+-- clause.  Verify that clause deduplication removes the duplicate, leaving
+-- a single Filter condition.
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+  sl AS sl5 LEFT JOIN (sl AS sl6 NATURAL JOIN sl AS sl7)
+    ON sl6.bool_col IS NOT NULL
+  LEFT JOIN sl AS sl8
+    ON sl6.bool_col;
+                                                 QUERY PLAN                                                 
+------------------------------------------------------------------------------------------------------------
+ Nested Loop Left Join
+   ->  Seq Scan on sl sl5
+   ->  Nested Loop Left Join
+         Join Filter: sl7.bool_col
+         ->  Seq Scan on sl sl7
+               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 sl8
+(7 rows)
+
+-- SJE: multi-relation clause deduplication after self-join removal.
+-- After SJE removes sl4, both ON clauses collapse to the same expression
+-- but with non-singleton required_relids (referencing sl2 and the kept rel).
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+  sl sl1 LEFT JOIN sl sl2
+    LEFT JOIN (sl AS sl4 NATURAL JOIN sl AS sl3)
+      ON (sl4.b + sl2.b) IS NOT NULL
+    ON (sl3.b + sl2.b) IS NOT NULL;
+                                                 QUERY PLAN                                                 
+------------------------------------------------------------------------------------------------------------
+ Nested Loop Left Join
+   ->  Seq Scan on sl sl1
+   ->  Nested Loop
+         Join Filter: (((sl3.b + sl2.b) IS NOT NULL) AND ((sl3.b + sl2.b) IS NOT NULL))
+         ->  Seq Scan on sl sl2
+         ->  Seq Scan on sl sl3
+               Filter: ((a IS NOT NULL) AND (b IS NOT NULL) AND (c IS NOT NULL) AND (bool_col IS NOT NULL))
+(7 rows)
+
+-- SJE: clause deduplication must not merge clauses with different
+-- security_level values. An RLS policy qual and a
+-- user-written ON clause may look identical after SJE
+-- rewrites varnos, but removing the RLS qual would break the security
+-- barrier guarantee.
+ALTER TABLE sl ENABLE ROW LEVEL SECURITY;
+ALTER TABLE sl FORCE ROW LEVEL SECURITY;
+CREATE POLICY sl_policy ON sl USING (bool_col IS NOT NULL);
+CREATE ROLE regress_sje_user LOGIN;
+GRANT SELECT ON sl TO regress_sje_user;
+SET ROLE regress_sje_user;
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+  sl sl1 LEFT JOIN (
+    (sl AS sl2 NATURAL JOIN sl AS sl3)
+    LEFT JOIN sl sl4
+    ON sl2.bool_col IS NOT NULL
+  ) ON sl2.bool_col IS NOT NULL;
+                                                              QUERY PLAN                                                               
+---------------------------------------------------------------------------------------------------------------------------------------
+ Nested Loop Left Join
+   ->  Seq Scan on sl sl1
+         Filter: (bool_col IS NOT NULL)
+   ->  Nested Loop Left Join
+         Join Filter: (sl3.bool_col IS NOT NULL)
+         ->  Seq Scan on sl sl3
+               Filter: ((bool_col IS NOT NULL) AND (bool_col IS NOT NULL) AND (a IS NOT NULL) AND (b IS NOT NULL) AND (c IS NOT NULL))
+         ->  Seq Scan on sl sl4
+               Filter: (bool_col IS NOT NULL)
+(9 rows)
+
+RESET ROLE;
+DROP POLICY sl_policy ON sl;
+ALTER TABLE sl DISABLE ROW LEVEL SECURITY;
+ALTER TABLE sl NO FORCE ROW LEVEL SECURITY;
+REVOKE SELECT ON sl FROM regress_sje_user;
+DROP ROLE regress_sje_user;
 -- 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 30b479dda7c..864d95694f9 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -3162,6 +3162,54 @@ 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;
 
+-- SJE: after self-join removal merges sl7 and sl6, the JOIN
+-- produces a pushed-down (bool_col IS NOT NULL) that duplicates the ON
+-- clause.  Verify that clause deduplication removes the duplicate, leaving
+-- a single Filter condition.
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+  sl AS sl5 LEFT JOIN (sl AS sl6 NATURAL JOIN sl AS sl7)
+    ON sl6.bool_col IS NOT NULL
+  LEFT JOIN sl AS sl8
+    ON sl6.bool_col;
+
+-- SJE: multi-relation clause deduplication after self-join removal.
+-- After SJE removes sl4, both ON clauses collapse to the same expression
+-- but with non-singleton required_relids (referencing sl2 and the kept rel).
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+  sl sl1 LEFT JOIN sl sl2
+    LEFT JOIN (sl AS sl4 NATURAL JOIN sl AS sl3)
+      ON (sl4.b + sl2.b) IS NOT NULL
+    ON (sl3.b + sl2.b) IS NOT NULL;
+
+-- SJE: clause deduplication must not merge clauses with different
+-- security_level values. An RLS policy qual and a
+-- user-written ON clause may look identical after SJE
+-- rewrites varnos, but removing the RLS qual would break the security
+-- barrier guarantee.
+ALTER TABLE sl ENABLE ROW LEVEL SECURITY;
+ALTER TABLE sl FORCE ROW LEVEL SECURITY;
+CREATE POLICY sl_policy ON sl USING (bool_col IS NOT NULL);
+CREATE ROLE regress_sje_user LOGIN;
+GRANT SELECT ON sl TO regress_sje_user;
+SET ROLE regress_sje_user;
+
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+  sl sl1 LEFT JOIN (
+    (sl AS sl2 NATURAL JOIN sl AS sl3)
+    LEFT JOIN sl sl4
+    ON sl2.bool_col IS NOT NULL
+  ) ON sl2.bool_col IS NOT NULL;
+
+RESET ROLE;
+DROP POLICY sl_policy ON sl;
+ALTER TABLE sl DISABLE ROW LEVEL SECURITY;
+ALTER TABLE sl NO FORCE ROW LEVEL SECURITY;
+REVOKE SELECT ON sl FROM regress_sje_user;
+DROP ROLE regress_sje_user;
+
 -- 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.51.0

---

From: Tender Wang <[email-masked]>
Date: Fri, 27 Mar 2026 09:19:29 +0800

> level in favour of an unsecured one.

Yes, it seems too strict to require all fields to be equal, but
skipping some fields is unsafe.

Agree.

> +               a->is_pushed_down == b->is_pushed_down)

The singleton issue does not seem to be the correct way; I don't dive
deeply to cover all cases.

-- 
Thanks,
Tender Wang

---

From: Alexander Korotkov <[email-masked]>
Date: Wed, 22 Apr 2026 18:10:22 +0300

> > ‘deduplication’ stage later, before the selectivity estimation stage.

I have another approach about to deduplication of RestrictInfo's.  The
field, which differs in this case, is outer_relids.  AFAICS,
outer_relids and incompatible_relids serves as the restriction on what
we can do with RestrictInfo.  So, what we can do is to ignore both
outer_relids and incompatible_relids during comparison, but compose a
union of their values for remaining RestrictInfo.  That means that
remaining RestrictInfo will ancest all the restrictions, and that
should be safe.

What do you think?

------
Regards,
Alexander Korotkov
Supabase

---

From: Tender Wang <[email-masked]>
Date: Thu, 23 Apr 2026 16:26:48 +0800

> remaining RestrictInfo will ancest all the restrictions, and that

It seems workable and safer than the previous solution.

-- 
Thanks,
Tender Wang

---

From: Andrei Lepikhov <[email-masked]>
Date: Fri, 22 May 2026 12:16:17 +0200

> remaining RestrictInfo will ancest all the restrictions, and that

Thank you for all the work you’ve put into de-duplicating clauses.

I agree that using the union of outer_relids and incompatible_relids is the
strictest common constraint. There shouldn’t be any issues, so this approach
should work.

However, the new function relies on a hand-picked list of "semantic" fields. If
someone adds another field to RestrictInfo, this function could break without
warning unless they remember to update it. We should add comment hooks that say,
"If you add a field here, update analyzejoins.c too."

Also, de-duplication happens in several places. If we change the logic in
add_non_redundant_clauses, maybe we should review the update_eclasses() code as
well.

---

From: Alexander Korotkov <[email-masked]>
Date: Mon, 25 May 2026 22:26:51 +0300

Hi, Andrei!

> add_non_redundant_clauses, maybe we should review the update_eclasses() code as

Please, check the updated patch.  Now restrict_infos_logically_equal()
uses old save/restore approach.  Also added guardian comment to
RestrictInfo.  update_eclasses() switched to use
restrict_infos_logically_equal() to compare RestrictInfos.

------
Regards,
Alexander Korotkov
Supabase