Re: BUG #19484: Segmentation fault triggered by FDW

Fujita-san,

On Thu, Jun 25, 2026 at 2:02 AM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote:
> On Wed, Jun 24, 2026 at 8:20 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > On Wed, Jun 24, 2026 at 6:15 PM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote:
> > > + /*
> > > + * 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.
>
> Thank you for doing that work!
>
> This might be nitpicking, but:
>
> + if (list_length(node->resultRelations) == mtstate->mt_nrels)
> + fdw_private = (List *) list_nth(node->fdwPrivLists, j);
> + else
> + {
> + Index rti = resultRelInfo->ri_RangeTableIndex;
> + ListCell *lc1;
> + ListCell *lc2;
> +
> + fdw_private = NIL;
> + forboth(lc1, node->resultRelations, lc2, node->fdwPrivLists)
> + {
> + if (lfirst_int(lc1) == (int) rti)
> + {
> + fdw_private = (List *) lfirst(lc2);
> + break;
> + }
> + }
> + }
>
> I'd put the if-test outside of the outer loop to save cycles.

Right, it's loop-invariant. v3 attached computes a boolean
(nopruning), like labeltargets, once before the loop and uses it
inside.

> Other than that v2 looks good to me.
>
> (The forboth loop actually causes an n-squared calculation, but it's
> done only when pruning occurs, in which case the number of remaining
> result relations would be reduced, so that wouldn't be a problem.)

Right, though strictly the inner forboth scans node->resultRelations,
which pruning leaves at its original length, so it's the original
relation count that bounds the scan rather than the reduced one.
Either way it's EXPLAIN-only with small counts, so it's not a concern.

Thanks again for the review.

--
Thanks, Amit Langote