| 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
| 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 |