public inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedFrom: Alexander Korotkov <aekorotkov@gmail.com>
To: Tender Wang <tndrwang@gmail.com>
Cc: Andrei Lepikhov <lepihov@gmail.com>
Cc: Kirill Reshke <reshkekirill@gmail.com>
Cc: Fujii Masao <masao.fujii@gmail.com>
Cc: ammmkilo@163.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #19435: Error: "No relation entry for relid 2" Triggered by Complex Join with Self-Referencing Tables
Date: Wed, 22 Apr 2026 18:10:22 +0300
Message-ID: <CAPpHfdseh9h2NSFyjWbuhoik61Ao7J0AjbuZBd_Fz+gKz98j5Q@mail.gmail.com> (raw)
In-Reply-To: <CAHewXNnt5bSWUirqV7mtkCit592j3h7VA-gOOJW_Ms0x8zv62w@mail.gmail.com>
References: <19435-3cc1a87f291129f1@postgresql.org>
<CAHewXN=LjuWz3PcyhjdbJAyo+Zs9MisPDRYnSZBUy4PMeKi+zA@mail.gmail.com>
<CALdSSPj1kTTQvmV3H3HMf5P3um8ybxoH3DaTPm+XgdYAur1Q4A@mail.gmail.com>
<CAHewXNndByMu3S+_h4LLDkXA5qrO1s=s-CE8HqUtc9vTA9yrjg@mail.gmail.com>
<CAPpHfdv6gzSTXHJxYSgB8sULadXM4wvhgoQODaOxYCJfagKNPw@mail.gmail.com>
<CAHewXN=7kDJjUcgEm+6qhaKOXuqzvhRqAAKdafNCRgn0yH7BGg@mail.gmail.com>
<CAHewXNm5OOREJ8wZ1cLJdQz7O1aQ0E1RBB55S6O138K8vBdc9g@mail.gmail.com>
<CAPpHfducqLJ=o3LkoPKGfZJVQuuei+P=2oUF6hX6rzHTZSxoyA@mail.gmail.com>
<a78fe5d4-e6b8-4b3c-9cfd-135edbb68e4c@gmail.com>
<CAPpHfduTWFCHaK8U7bDfYid5pjVA=FHG1b0nTEMFqFKHebGJxQ@mail.gmail.com>
<a498f5b8-2f17-4ee0-b021-63ff9829b45b@gmail.com>
<CALdSSPhpUdY7-5Zg38oS1uRtu5iTFzdo0R7Z2YZD603M9RpJxg@mail.gmail.com>
<5a039d60-d13b-4cf0-a807-9c7269f06831@gmail.com>
<CAPpHfdsyNYEbjjLdsa8i8Ds-5=4pFif1+uCHn3vwzx2Pq5y29A@mail.gmail.com>
<CAPpHfdsrmAg+aqpjAF4Fdp2c59-dFmwBuNLhNqrxzTguiAKf=w@mail.gmail.com>
<afb2c07f-05b7-403c-b10c-ca7390316e94@gmail.com>
<CAHewXNnt5bSWUirqV7mtkCit592j3h7VA-gOOJW_Ms0x8zv62w@mail.gmail.com>
On Fri, Mar 27, 2026 at 3:19 AM Tender Wang <tndrwang@gmail.com> wrote:
> Andrei Lepikhov <lepihov@gmail.com> 于2026年3月27日周五 03:59写道：
> >
> > On 20/3/26 15:02, Alexander Korotkov wrote:
> > > 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?
> >
> > 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.
>
> Yes, it seems too strict to require all fields to be equal, but
> skipping some fields is unsafe.
>
>
> > 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.
> >
>
> Agree.
>
> > 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)
> >
>
> The singleton issue does not seem to be the correct way; I don't dive
> deeply to cover all cases.
>
> > 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.
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
Attachments:
[application/octet-stream] v1-0001-Deduplicate-RestrictInfos-differing-only-in-outer.patch (7.9K, 2-v1-0001-Deduplicate-RestrictInfos-differing-only-in-outer.patch)
download | inline diff:
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)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: aekorotkov@gmail.com, tndrwang@gmail.com, lepihov@gmail.com, reshkekirill@gmail.com, masao.fujii@gmail.com, ammmkilo@163.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #19435: Error: "No relation entry for relid 2" Triggered by Complex Join with Self-Referencing Tables
In-Reply-To: <CAPpHfdseh9h2NSFyjWbuhoik61Ao7J0AjbuZBd_Fz+gKz98j5Q@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox