public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Langote <[email protected]>
To: Etsuro Fujita <[email protected]>
Cc: Matheus Alcantara <[email protected]>
Cc: Ayush Tiwari <[email protected]>
Cc: Rafia Sabih <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Amit Langote <[email protected]>
Subject: Re: BUG #19484: Segmentation fault triggered by FDW
Date: Wed, 24 Jun 2026 10:43:32 +0900
Message-ID: <CA+HiwqEhe7-v5Q0-oOoW3RaO4voYcGK-JfinbYEWXwutDGSOtQ@mail.gmail.com> (raw)
In-Reply-To: <CA+HiwqHTsZmceNHxVfmD0VVN-F1eMSuVQYVDWfsz3OSe+qoO7A@mail.gmail.com>
References: <[email protected]>
<CAJTYsWXY9C3B-7NZw72OKen2L2rZt=c-t6=kjTJzgj=ZaNPe8g@mail.gmail.com>
<[email protected]>
<CA+FpmFc10Dtnr9dr=xqK2cNYQ4-1VuOyvOT7-eqXPmiQJoyPew@mail.gmail.com>
<[email protected]>
<CAJTYsWWUfgCZau2fvrZVruXmC+dyOx45L=TyaGzXdywsywDyHQ@mail.gmail.com>
<[email protected]>
<CA+HiwqHVMY1a5E+6Ei369+RqQ8qKSSgon_C=3yBk-TPJDwZ2nQ@mail.gmail.com>
<CA+HiwqHjUO0uXFZ2dECJiNime6B1FDL5+mJ=XN+1XgzkY=m9DA@mail.gmail.com>
<CAPmGK16GNiwcnAxM3XU=ZTOP_8oiv7_FOUwo5hZp8b+qfv2L7g@mail.gmail.com>
<CA+HiwqEVwEPSE4yMvuwVT5bb+ziMHLfqd_HuiQPAFAixNHbhrQ@mail.gmail.com>
<CA+HiwqHO0Bbi6cyk4F+UGvZEV3NEo7o0YT3-Js7K7hvasoWBVw@mail.gmail.com>
<CAPmGK16fALJqbyR1Oir7-TsN8k_yAR-JcOg5nWOUmDmivTmDtA@mail.gmail.com>
<CAPmGK14Qh_Wb4GNPNkZ1o9=vTcMKShzPNSWa=wNf9vJpg8qT+Q@mail.gmail.com>
<CA+HiwqHRDsYDqJ-7f5P_QpJo=U1BYzMnfDnNOr6Vx3Rw36O0iQ@mail.gmail.com>
<CAPmGK15nR_pePb9QAMViq+AGuWeJSqPnLTJVLsr96aU2TFmcmw@mail.gmail.com>
<CA+HiwqFT0VDOCAdN1bcVZTOF0oBbR4kzLF-OwEcgPOf01me-Kw@mail.gmail.com>
<CAPmGK15nx4a_QkTcbD2BwsDuDPtbS25Pzmq_sc-xM4EitoHaxw@mail.gmail.com>
<CA+HiwqHTsZmceNHxVfmD0VVN-F1eMSuVQYVDWfsz3OSe+qoO7A@mail.gmail.com>
On Wed, Jun 24, 2026 at 9:00 Amit Langote <[email protected]> wrote:
> On Tue, Jun 23, 2026 at 5:27 PM Etsuro Fujita <[email protected]>
> wrote:
> > On Mon, Jun 22, 2026 at 5:28 PM Amit Langote <[email protected]>
> wrote:
> > > On Fri, Jun 19, 2026 at 8:59 PM Etsuro Fujita <[email protected]>
> wrote:
> >
> > > > +-- Runtime pruning of result relations must keep ModifyTable's
> per-relation
> > > > +-- FDW arrays (fdwPrivLists, fdwDirectModifyPlans) aligned with the
> kept
> > > > +-- resultRelations. Otherwise BeginForeignModify() reads the wrong
> > > > +-- fdw_private and segfaults.
> > > >
> > > > This comment isn't 100% correct, because this issue happens with
> > > > DirectModify as well. I think we could expand the comment to mention
> > > > that as well, but the comment is already too much/detailed IMO; I
> > > > don't think we add such a comment for a test case, so how about
> > > > simplifying the comment like this: "Test that direct modify and
> > > > foreign modify work with runtime pruning of result relations (bug
> > > > #19484)"
> > >
> > > I agree that the details shouldn't leak into test code and have been
> > > trying to follow that principle. Also, bug numbers shouldn't be
> > > mentioned because one can always use `git blame` to find them.
> > > However, since different committers have different styles, I'm fine
> > > with it; kept in the attached updated patch.
> >
> > Thanks!
> >
> > > > @@ -1446,6 +1446,12 @@ typedef struct ModifyTableState
> > > > int mt_nrels; /* number of entries in
> resultRelInfo[] */
> > > > ResultRelInfo *resultRelInfo; /* info about target relation(s)
> */
> > > >
> > > > + /*
> > > > + * Re-indexed fdw private data lists, aligned with
> resultRelInfo[] after
> > > > + * pruning
> > > > + */
> > > > + List *mt_fdwPrivLists;
> > > >
> > > > For v18, I think we should put this at the end of the
> ModifyTableState
> > > > struct, to avoid ABI breakage.
> > >
> > > Good point on ABI. Rather than placing it differently per branch, how
> > > about putting mt_fdwPrivLists at the end of the struct alongside
> > > mt_updateColnosLists/mt_mergeActionLists/mt_mergeJoinConditions, which
> > > are the same kind of unpruned-filtered lists? That keeps it ABI-safe
> > > and lets master and 18 share an identical change. I'd fold it into
> > > their existing comment too.
> >
> > That's a good idea! So +1
> >
> > > Updated patch attached.
> >
> > LGTM.
>
> Pushed, thanks for checking.
crake is now red on REL_18:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2026-06-24%2000%3A02%3A03
It reports sizeof(ModifyTableState) grew by 8 bytes.
That’s from adding mt_fdwPrivLists at the end of the struct. Placing it
last keeps existing field offsets stable, which is what extensions reading
the node rely on, but it does grow the struct, which is what the checker
flags. I believe we’ve added trailing struct members in back branches
before for exactly this reason. Is a flagged sizeof increase acceptable
here under that precedent, or do we now want to avoid any flagged ABI
change on a back branch?
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. master
would keep the field as committed, so the branches would differ slightly.
I’m inclined toward the no-field version for REL_18 to keep the farm green,
unless someone would rather keep the same code in both branches and accept
the size change. I’ll hold off briefly for any thoughts before doing that.
- Amit
>
view thread (32+ messages) latest in thread
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19484: Segmentation fault triggered by FDW
In-Reply-To: <CA+HiwqEhe7-v5Q0-oOoW3RaO4voYcGK-JfinbYEWXwutDGSOtQ@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