Re: Re: [PATCH] Fix segmentation fault caused by reentrancy in RI_Fkey_cascade_del (ri_triggers.c)

Hi Amit,

Thanks for v10, and for the credit in the commit message.

I tested v10 on master and on REL_14 through REL_19, built with
cassert, and make check passes on all of them, including the new
tests in foreign_key. It addresses everything we found, so it LGTM.

I found just one small thing, non-locking: the new test comment in
foreign_key.sql (and in foreign_key.out) says "(an assertion at end
of transaction checks that)", but that assertion was removed in v10,
so I would remove that part of the comment before commiting
(doesn't needs a v11).

Regards,
Lucas Jeffrey

El vie, 23 oct 2026 a las 6:58, Amit Langote
(<amitlangote09(at)gmail(dot)com>) escribió:
>
> On Fri, Oct 23, 2026 at 6:01 AM Lucas Jeffrey
> <lucas(dot)jeffrey(at)anachronics(dot)com> wrote:
> > El jue, 22 oct 2026 a las 8:18, Amit Langote (<amitlangote09(at)gmail(dot)com>) escribió:
> >> On Wed, Oct 21, 2026 at 9:02 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> >> > Thanks for the patch. I'm going to study it and post an updated
> >> > version tomorrow. I'd like to handle errors as well as Lucas's patch
> >> > does but I'd stick to using PG_TRY in all the branches instead of
> >> > resowner in some and PG_TRY in others. I believe subtransaction
> >> > aborts are handled correctly by PG_TRY-based solution but I'd need to
> >> > test that. In any case, I want to avoid hash table and resowner calls
> >> > in the per-row path and keep the tracking code as minimal as possible.
> >>
> >> Sorry, ran out of time today, so will post the patch tomorrow.
> >
> > Thanks both for the work on this. I've been iterating on Amit's v1 plus
> > Trakshan's stack, and added error handling on top. Patch attached (v9).
> >
> > Amit, does the approach used to defer freeing the plans look OK to you?
> > To keep it conservative I don't free anything while the error is being
> > handled; maybe that's not needed and we could free them in the
> > PG_FINALLY instead.
>
> Thanks for v9. I have more or less the same patch in my local tree
> that I was about to post. Yeah, it makes sense to keep plan freeing
> out of the error handling path.
>
> So, here's v10, which streamlines the stack code by putting it in
> functions instead of directly in ri_PerformCheck as in your v9. It
> also keeps the tests from your v8-0001 and adds tests for behavior on
> subxact rollbacks. You'll notice that the commit message also details
> the history of what you found, what the buildfarm reported, etc.
>
> I've also attached patch files for the back-branches, which I've
> tested down to v14. v14 has a test file conflict so it needs a
> different patch file. v15 through v18 share the same patch file.
>
> As mentioned earlier, the commit message lists you as the author, with
> Trakshan and me as co-authors, which I think reflects how this came
> together.
>
> I’d like to commit this early next week unless there are objections.
>
> --
> Thanks, Amit Langote