| From: | Alexander Korotkov <aekorotkov(at)gmail(dot)com> |
|---|---|
| To: | Tender Wang <tndrwang(at)gmail(dot)com> |
| Cc: | Andrei Lepikhov <lepihov(at)gmail(dot)com>, Kirill Reshke <reshkekirill(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, ammmkilo(at)163(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org |
| Subject: | Re: BUG #19435: Error: "No relation entry for relid 2" Triggered by Complex Join with Self-Referencing Tables |
| Date: | 2026-04-22 15:10:22 |
| Message-ID: | CAPpHfdseh9h2NSFyjWbuhoik61Ao7J0AjbuZBd_Fz+gKz98j5Q@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
On Fri, Mar 27, 2026 at 3:19 AM Tender Wang <tndrwang(at)gmail(dot)com> wrote:
> Andrei Lepikhov <lepihov(at)gmail(dot)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
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Deduplicate-RestrictInfos-differing-only-in-outer.patch | application/octet-stream | 7.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Masahiko Sawada | 2026-04-22 16:51:18 | Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" |
| Previous Message | Tom Lane | 2026-04-22 14:50:09 | Re: Potential buffer overrun in spell.c's CheckAffix() |