Re: BUG #19484: Segmentation fault triggered by FDW

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
Cc: Richard Guo <guofenglinux(at)gmail(dot)com>, Matheus Alcantara <matheusssilv97(at)gmail(dot)com>, Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>, Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>, 798604270(at)qq(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org, Amit Langote <amitlan(at)postgresql(dot)org>
Subject: Re: BUG #19484: Segmentation fault triggered by FDW
Date: 2026-06-24 11:20:29
Message-ID: CA+HiwqFd61sXgUWijKg8DfRQDtwYEGSdxuEPDb8rq22rxzxN_g@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Fujita-san,

On Wed, Jun 24, 2026 at 6:15 PM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote:
> On Wed, Jun 24, 2026 at 1:25 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > On Wed, Jun 24, 2026 at 11:23 AM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> > > On Wed, Jun 24, 2026 at 10:43 AM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > > > If we’d rather avoid it, I have a no-field alternative for REL_18: show_modifytable_info() is the only reader of the re-indexed list, and it can recompute the mapping from node->resultRelations and node->fdwPrivLists, both plan-ordered and untouched by pruning.
> > >
> > > This sounds like a nice solution. Since show_modifytable_info() is
> > > only used for EXPLAIN, I guess the recompute overhead on the
> > > re-indexed list should be fine.
> >
> > Thanks for chiming in, here is the patch to do so.
>
> Thanks for the patch!
>
> + /*
> + * node->fdwPrivLists is indexed by the original, pre-pruning
> + * result relation order and is parallel to node->resultRelations.
> + * Initial pruning may have dropped earlier relations, so the kept
> + * index j need not match the original position; find this
> + * relation's entry by its range table index instead.
> + */
> + forboth(lc1, node->resultRelations, lc2, node->fdwPrivLists)
> + {
> + if (lfirst_int(lc1) == (int) rti)
> + {
> + fdw_private = (List *) lfirst(lc2);
> + break;
> + }
> + }
>
> I think it's good to skip this for efficiency, when there are no
> pruned result relations.

Thanks for the review. v2 attached implements your suggestion: it
indexes node->fdwPrivLists directly with j when no result relations
were pruned, and only falls back to matching by range table index when
pruning appears to have dropped some.

> Other than that the patch looks good to me.
>
> I didn't know pg_rewrite's use of ModifyTableState, which was
> different than I expected. Sorry for that.

No need to apologize at all. Tom's point about it was new to me too.

I will push v2 to REL_18 tomorrow barring objections.

--
Thanks, Amit Langote

Attachment Content-Type Size
v2-0001-Avoid-ABI-break-in-ModifyTableState-from-the-FDW-.patch application/octet-stream 4.5 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Hüseyin Demir 2026-06-24 12:19:47 Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table
Previous Message Laurenz Albe 2026-06-24 09:56:58 Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table