Re: assertion failure with unique index + partitioning + join

From: Tender Wang <tndrwang(at)gmail(dot)com>
To: Richard Guo <guofenglinux(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Alexander Kuzmenkov <akuzmenkov(at)tigerdata(dot)com>
Subject: Re: assertion failure with unique index + partitioning + join
Date: 2026-06-16 09:32:06
Message-ID: CAHewXNmFgQkHmBT15_sNVvKnOm4mH37mRRLDm6VViHodtSdkHA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Richard Guo <guofenglinux(at)gmail(dot)com> 于2026年6月16日周二 16:27写道:
>
> On Tue, Jun 16, 2026 at 10:58 AM Tender Wang <tndrwang(at)gmail(dot)com> wrote:
> > It was distributed to (rel:2, table t), but I think "[phrels]
> > Bitmapset [4 3 2]" is incorrect. After leftjoin is removed, we no
> > longer need rel=4 (outerjoin) and rel=3 (t r).
> > But the qual has already been distributed to the (rel: 2) when
> > deconstruct_jointree (). Then the qual is inherited by its child(table
> > p)
> > It seems that we have no logic to process this distributed
> > restrictinfo in remove_useless_joins().
>
> When removing the left join, remove_rel_from_query() fixes up the
> relid sets of RestrictInfos and EquivalenceMembers, and the canonical
> PlaceHolderVars, but it does not rewrite the PlaceHolderVars embedded
> in clause and EC-member expressions. This is usually fine, because
> later processing consults those relid sets rather than the embedded
> PHVs.
>
> The trouble is that such an expression can later be translated for an
> appendrel child and have its relids recomputed by pull_varnos(). If
> the embedded PlaceHolderVar's phrels still mentions the removed
> relation, pull_varnos() folds it back in, so the rebuilt clause
> references a no-longer-existent rel. That produces a parameterized
> path keyed on the removed relation, which trips the assertion.

Yes, I came to the same conclusion.

>
> I think we can fix it by stripping the removed relids from the PHVs in
> surviving rels' baserestrictinfo and in EquivalenceClass member
> expressions. See attached.

The fix I proposed was somewhat specific to this particular issue,
whereas your patch takes a more general approach.
I took a quick pass over the patch and didn't notice any obvious issues.
I won't have time to review it in detail today, but I'll try to take a
closer look tomorrow morning.

>
> (I went back and forth on instead making pull_varnos() robust to the
> stale phrels, but that turned out fragile: it can't reliably
> distinguish a removed relid from a not-yet-built appendrel child, and
> it broke unrelated partitionwise-join plans.)

Interesting, I hadn't thought of that approach.

--
Thanks,
Tender Wang

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2026-06-16 09:35:39 Re: Fix race in ReplicationSlotRelease for ephemeral slots
Previous Message Ajin Cherian 2026-06-16 09:20:01 Re: [PATCH] Preserve replication origin OIDs in pg_upgrade