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

From: Trakshan Mishra <trakshanmishra477(at)gmail(dot)com>
To: lucas(dot)jeffrey(at)anachronics(dot)com
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Re: Re: Re: [PATCH] Fix segmentation fault caused by reentrancy in RI_Fkey_cascade_del (ri_triggers.c)
Date: 2026-10-05 15:03:22
Message-ID: CACRpqq9NNye8KyLdgA7N5-MSwCpJJsXtq41+=9n7TxJHYZX64Q@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Lucas,

Thanks for the rebase. v5 is a bit more than a rebase, though, so I
went through it again rather than just checking that it applies.

Test environment:
master @ 85adc584e2b, plus v5-0001 and v5-0002
Linux x86_64, gcc 15.2.0
meson, --buildtype=debug -Dcassert=true

== What changed ==

425daf545d9 (Remove batching from RI fast-path checks) removed
AtEOSubXact_RI(), which is where v4 freed the plans whose last execution
was aborted with a subtransaction. In v5 those plans are now freed at
the start of the next ri_PreparedPlanExecutionStarted(), with
AtEOXact_RI() as the backstop. The rest of 0002 and all of 0001 are
unchanged apart from context.

I think the new placement is safe. A plan on ri_plans_to_free has
refcount 0, so no frame up the stack is executing it. It has already
been removed from ri_query_cache, so nothing can fetch it again. And it
still has its entry in the refcount table until it is freed, so its
address cannot be handed to a new plan before then. The cost on the
normal path is one empty-list check per RI query.

== Testing ==

Both patches apply cleanly with no whitespace errors, and the build has
no warnings.

Full "meson test" with v5: 360 ok, 0 failed, 50 skipped.

With only v5-0001 applied, the new test still segfaults the backend on
the cascade, and foreign_key plus the tests after it fail, as with v4.

To see which path frees each plan, I added temporary LOG lines where a
plan is marked for deletion, queued, and freed. Results:

- The DO block in the test (10 subtransaction aborts in one
transaction): 10 plans marked, 10 queued, 9 freed by the next
iteration's RI query and the last one by AtEOXact_RI() at commit.
Plan addresses do get reused across iterations, but only after the
earlier plan with that address has been freed.

- Top-level abort, 50 times, followed by an invalidation and a
successful cascade: 50 queued, all 50 freed by AtEOXact_RI() on the
abort path. No crash and no assertion failure.

- SAVEPOINT / ROLLBACK TO in a transaction block, then COMMIT with no
further RI query: the plan is freed at commit, and the Assert that
the table is empty at transaction end holds.

== Comments ==

1. The comments and the commit message say the deferred plans are freed
by "the next RI query execution". Strictly, it is the next RI query
that goes through ri_PerformCheck(). Checks that take the fast path
don't call ri_PreparedPlanExecutionStarted(), so after a savepoint
rollback followed only by plain FK inserts, the plan waits for
AtEOXact_RI(). That is harmless, but "the next ri_PerformCheck() call"
would describe it more exactly.

2. The test still covers only the subtransaction case. The top-level
abort path, where AtEOXact_RI() frees the plan after AtEOXact_SPI() has
run, is still something I ran by hand rather than something the test
checks. As before, I would not hold the patch for it.

== Summary ==

The rebased fix looks right to me, and the nits from my last mail are
gone. I am leaving the entry at Ready for Committer.

Regards,
Trakshan Mishra

On Mon, Oct 05, 2026 08:01 PM, Lucas Jeffrey <lucas(dot)jeffrey(at)anachronics(dot)com>
wrote:

> Hi hackers,
> I'll attach a new v5 version of my patch, I rebased so my patches can now
> apply cleanly in master.
> Reviews are welcomed.
> Regards,
> Lucas Jeffrey
>
>
>
> El vie, 25 sept 2026 a las 1:56, Trakshan Mishra (<
> trakshanmishra477(at)gmail(dot)com>) escribió:
>
>> Hi Lucas,
>>
>> Thanks for the quick turnaround on v4. I have retested it; everything
>> from my review is addressed, and I could not break the new bookkeeping.
>> Details below.
>>
>> Test environment:
>> master @ 89829354de1, plus v4-0001 and v4-0002
>> Linux x86_64, gcc 15.2.0
>> meson, --buildtype=debug -Dcassert=true
>>
>>
>> == Submission review ==
>>
>> Both patches apply cleanly to current master, and "git apply
>> --whitespace=warn" now reports no whitespace errors at all (it was 12).
>> typedefs.list is updated. The build is clean with no warnings.
>>
>>
>> == Coding review ==
>>
>> 1. Fixed. I rechecked this rather than take it on trust, by rerunning
>> the instrumented case from my review: a parent/child pair with ON DELETE
>> CASCADE, a BEFORE DELETE trigger on the child that raises, and ten
>> cascade deletes each caught by an EXCEPTION block. Previously the count
>> climbed and never came back:
>>
>> RI_REFCOUNT_DEBUG entries=1 plan=0x60f5822cfd30 refcount=1
>> RI_REFCOUNT_DEBUG entries=1 plan=0x60f5822cfd30 refcount=2
>> ...
>> RI_REFCOUNT_DEBUG entries=1 plan=0x60f5822cfd30 refcount=10
>>
>> With v4, all ten iterations look like this:
>>
>> RI_REFCOUNT_DEBUG entries=1 plan=0x5f5073f4c770 refcount=1
>>
>> The count is given back on every error path, and the table does not
>> grow. The ResourceOwner approach is nicer than the PG_TRY I suggested,
>> and I agree with your reasoning about subtransaction boundaries.
>>
>> 2. Fixed. The entry is now removed whenever the count reaches zero,
>> independently of plan validity.
>>
>> 3. You kept the plan pointer as the hash key and closed the hole with an
>> invariant instead: a plan is never freed while it has an entry, so its
>> address cannot be reused while it is tracked. I went looking for a way
>> around that and did not find one.
>>
>> - All three SPI_freeplan() calls now sit inside the new helpers
>> (ri_PreparedPlanDecrementRefCount, ri_PreparedPlanReleaseASAP and
>> ri_PreparedPlanFreeUnused), and ri_FetchPreparedPlan() no longer
>> frees a plan directly.
>> - The RI plans are saved with SPI_keepplan(), so SPI will not free
>> them at transaction end either.
>> - Nothing else removes entries from ri_query_cache.
>>
>> I also built with a detector in ri_PreparedPlanExecutionStarted() that
>> warns whenever HASH_ENTER finds an existing entry whose refcount is
>> zero, which is what picking up a stale entry would look like. It did
>> not fire once across the whole regression suite.
>>
>> So I am satisfied. I still mildly prefer the refcount living on
>> RI_QueryHashEntry, because then the question cannot be raised at all,
>> but that is a preference rather than an objection, and a committer's
>> call.
>>
>> 4. Fixed, Assert(entry->refcount > 0) is in place.
>>
>> 5. Fixed. The ri_InitHashTables() call is now an Assert, and the table
>> is created alongside the others.
>>
>> 6. The style points are all addressed.
>>
>>
>> == Teardown ordering ==
>>
>> The commit message relies on the resource owner having been released by
>> the time AtEOSubXact_RI() and AtEOXact_RI() run. That holds:
>> RESOURCE_RELEASE_AFTER_LOCKS is at xact.c:5419 and AtEOSubXact_RI() at
>> 5434, and on the top-level paths AtEOXact_RI() is at 2526, 2822 and
>> 3053, after the three ResourceOwnerRelease() calls in each.
>>
>> One thing worth stating explicitly: AtEOXact_SPI() runs immediately
>> before AtEOXact_RI() (xact.c:3045 and 3053 on the abort path), so
>> ri_PreparedPlanFreeUnused() calls SPI_freeplan() after SPI has been torn
>> down for the transaction. As far as I can tell that is fine, since
>> SPI_freeplan() does not touch _SPI_current, and it works in practice
>> (see below). I mention it only because it is a dependency that is not
>> obvious from reading the patch on its own.
>>
>>
>> == Test patch ==
>>
>> The identifiers are English, the surviving rows are checked, and the
>> invalidation is deterministic now -- the GRANT/REVOKE inside the trigger
>> is a much better mechanism than the 1000 temp tables. I confirmed the
>> test still catches the bug: master with only v4-0001 applied (I checked
>> that the fix was absent from the binary) segfaults on
>>
>> DELETE FROM fk_self_ref WHERE id = 1;
>>
>> so it fails loudly on an unfixed backend, which was my concern before.
>>
>> Two comments.
>>
>> a) Moving it into src/test/regress answers my question about isolation,
>> but the blast radius is worth knowing: when it crashes, the cluster
>> restarts and takes the rest of the schedule with it. On my unpatched
>> run, foreign_key plus 51 later tests failed. I think that is acceptable
>> for a regression test, since it only crashes if the bug returns.
>>
>> b) The test only covers subtransaction abort, via the PL/pgSQL exception
>> block. The top-level abort path, where AtEOXact_RI() rather than
>> AtEOSubXact_RI() does the freeing, is not exercised -- and that is the
>> path with the AtEOXact_SPI() ordering dependency above, so it is the one
>> I would most like covered. I tested it by hand: same schema, the error
>> allowed to escape to the top level, then a further invalidation and a
>> successful cascade, repeated 50 times on a cassert build. It is clean,
>> no crash and no assertion failure. But it would be better living in the
>> test than as something I ran once.
>>
>>
>> == Regression testing ==
>>
>> Full "meson test" with v4: 360 ok, 0 failed, 50 skipped.
>>
>> That run included recovery/027_stream_regress, which passed. I would
>> not read anything into that. It fails intermittently at roughly a third
>> of runs on this machine, and it is independent of your patch -- it
>> reproduces at c36a0df195d, which predates the RI fast-path series. A
>> single green run is not evidence either way. It is being tracked
>> separately:
>>
>> https://www.google.com/url?q=https://postgr.es/m/
>> CACRpqq9WihNyHJXDQp_t%2Bi-i3R2fG6v%2BNC-XX4NKn4K6h8NrwA@
>> mail.gmail.com&source=gmail&ust=1790398611321000&sa=E
>>
>>
>> == Nits ==
>>
>> - One line in the v4-0002 commit message runs to 96 columns
>> ("invalidation are released already. Hence no entry survives ...").
>> - Three added lines in ri_triggers.c reach 79 columns.
>>
>>
>> == Summary ==
>>
>> Everything from my review is addressed, and the fix looks right to me.
>> I am marking this Ready for Committer. If you would prefer to add the
>> top-level abort case to the test first, I am happy to retest.
>>
>> Regards,
>> Trakshan Mishra
>>
>> On Fri, Sep 25, 2026 12:39 AM, Lucas Jeffrey <
>> lucas(dot)jeffrey(at)anachronics(dot)com> wrote:
>>
>>> Hi, Trakshan, thanks for your review.
>>>
>>> I created a v4 of my patch which fixes items pointed out by your review.
>>>
>>> Now, my patch is using a ResourceOwner to prevent a leak if the RI check
>>> throws an exception,
>>> also, now I'm adding invalidated plans that can't be instantly deleted
>>> to a delete-later list: "ri_plans_to_free",
>>> and, now it removes the refcount table entries if the plan wasn't
>>> invalidated (the plan is not freed, just the use-count entry).
>>>
>>> The test case, now all names are in english and now is using a
>>> deterministic way of reproducing the crash, instead of relying in
>>> invalidations by another session by creating and destroying the same table
>>> in a loop.
>>> El jue, 24 sept 2026 a las 11:46, Trakshan Mishra (<
>>> trakshanmishra477(at)gmail(dot)com>) escribió:
>>>
>>>> Hi,
>>>>
>>>> A correction to my review upthread. I said I would report the
>>>> 027_stream_regress assertion separately; that was already done, four
>>>> days before the review went out:
>>>>
>>>> Intermittent Assert("plan->magic == _SPI_PLAN_MAGIC") in
>>>> 027_stream_regress
>>>> https://www.google.com/url?q=https://postgr.es/m/
>>>> CACRpqq9WihNyHJXDQp_t%2Bi-i3R2fG6v%2BNC-XX4NKn4K6h8NrwA@
>>>> mail.gmail.com&source=gmail&ust=1790347576108000&sa=E
>>>>
>>>> Apologies for the tense -- I wrote that paragraph before posting and
>>>> forgot to fix it. There is nothing new in it, I just did not want
>>>> anyone holding off on a report that already exists.
>>>>
>>>> Two things from that thread are worth repeating here, because they
>>>> bear on this patch more directly than my review made out.
>>>>
>>>> First, the assert is older than the RI work. I ran it at c36a0df195d
>>>> (2026-08-20), which predates the RI fast-path series, and it still
>>>> fails there. So it is not fallout from 6fc2a486417, e2c812f1475,
>>>> 2c45694a240 or c62b330912e, and it is independent of v2-0002. My
>>>> review only said the failure was pre-existing on current master; the
>>>> bisect point makes that a good deal firmer.
>>>>
>>>> Second, on the counts. The 5/15 and 9/15 in my review were a separate
>>>> batch from the 7 of 20 on master in the earlier thread, both at
>>>> 9e17d25e79d. Together that is 12 of 35 unpatched runs, 34%, which is
>>>> in line with the 60% I saw patched given the sample sizes. I still
>>>> would not read anything into the difference.
>>>>
>>>> No action needed here. #6825 stands where I left it, Waiting on
>>>> Author for the refcount points.
>>>>
>>>> Regards,
>>>> Trakshan Mishra
>>>>
>>>> On Thu, Sep 24, 2026 05:31 PM, Trakshan Mishra <
>>>> trakshanmishra477(at)gmail(dot)com> wrote:
>>>>
>>>>> Hi Lucas,
>>>>>
>>>>> I picked this up from the PG20-2 commitfest (#6825) as a first-time
>>>>> reviewer. Summary up front: the bug is real and reproducible, the
>>>>> patch
>>>>> does fix it, but I think the refcount bookkeeping needs another round.
>>>>>
>>>>> Test environment:
>>>>> master @ 9e17d25e79d
>>>>> Linux x86_64, gcc 15.2.0
>>>>> meson, --buildtype=debug -Dcassert=true
>>>>>
>>>>>
>>>>> == Submission review ==
>>>>>
>>>>> v3-0001 (the isolation test) applies cleanly.
>>>>>
>>>>> v2-0002 (the fix) does *not* apply to current master. It conflicts in
>>>>> the "Local data" block around ri_triggers.c:251 -- the RI fast-path
>>>>> work
>>>>> that landed since you posted (c62b330912e, 2c45694a240, e2c812f1475 and
>>>>> neighbours) restructured that area. "git apply -3" resolves it without
>>>>> a real conflict, so this is just a rebase, but a v4 on top of current
>>>>> master would help the next reviewer and cfbot.
>>>>>
>>>>> "git apply" reports 12 whitespace errors across the two patches: 4 in
>>>>> the .spec file and 8 in ri_triggers.c. Several are tabs immediately
>>>>> after an opening brace, e.g.
>>>>>
>>>>> ri_PreparedPlanExecutionStarted(SPIPlanPtr plan)
>>>>> {<tab>
>>>>>
>>>>> pgindent should clear these.
>>>>>
>>>>>
>>>>> == Feature test ==
>>>>>
>>>>> I can confirm the crash on unpatched master. Applying only v3-0001 and
>>>>> running the new isolation test:
>>>>>
>>>>> client backend (PID 23625) was terminated by signal 11: Segmentation
>>>>> fault
>>>>> DETAIL: Failed process was running: DELETE FROM
>>>>> crash_reentrancia_tabla_autoreferencial WHERE id = 1;
>>>>> LOG: terminating any other active server processes
>>>>> LOG: all server processes terminated; reinitializing
>>>>>
>>>>> With v2-0002 applied the same test passes and the server stays up. So
>>>>> the patch does address the reported crash. Thanks for the clear
>>>>> reproducer -- the advisory-lock handshake to line up the invalidation
>>>>> was a nice touch.
>>>>>
>>>>>
>>>>> == Coding review ==
>>>>>
>>>>> 1. The refcount is leaked whenever the RI query throws.
>>>>>
>>>>> In ri_PerformCheck() the increment and decrement bracket
>>>>> SPI_execute_snapshot() with no PG_TRY/PG_FINALLY:
>>>>>
>>>>> ri_PreparedPlanExecutionStarted(qplan);
>>>>> spi_result = SPI_execute_snapshot(qplan, ...);
>>>>> ri_PreparedPlanExecutionFinished(qplan);
>>>>>
>>>>> Any ereport(ERROR) from inside the RI query longjmps past the
>>>>> Finished()
>>>>> call, so the count is never given back. This is not an exotic path --
>>>>> a
>>>>> BEFORE DELETE trigger on the referencing table that raises will do it,
>>>>> and so will statement_timeout, query cancel or a deadlock during the
>>>>> cascade.
>>>>>
>>>>> I instrumented the hash table locally to check, using a parent/child
>>>>> pair with ON DELETE CASCADE where the child has a BEFORE DELETE trigger
>>>>> that raises, and ten cascade deletes each caught by an EXCEPTION block:
>>>>>
>>>>> RI_REFCOUNT_DEBUG entries=1 plan=0x60f5822cfd30 refcount=1
>>>>> RI_REFCOUNT_DEBUG entries=1 plan=0x60f5822cfd30 refcount=2
>>>>> RI_REFCOUNT_DEBUG entries=1 plan=0x60f5822cfd30 refcount=3
>>>>> ...
>>>>> RI_REFCOUNT_DEBUG entries=1 plan=0x60f5822cfd30 refcount=10
>>>>>
>>>>> The count rises monotonically and never comes back down. Once that has
>>>>> happened ri_PreparedPlanCanRelease() returns false for that plan
>>>>> forever, so ri_FetchPreparedPlan() will never SPI_freeplan() it: on the
>>>>> next invalidation it sets entry->plan = NULL and the plan is orphaned
>>>>> in
>>>>> CacheMemoryContext for the life of the backend.
>>>>>
>>>>> A PG_TRY/PG_FINALLY around the execute, or tying the decrement to
>>>>> resource-owner or subtransaction cleanup, would fix this.
>>>>>
>>>>>
>>>>> 2. Entries are never removed when the refcount drops to zero on a plan
>>>>> that is still valid.
>>>>>
>>>>> ri_PreparedPlanExecutionFinished() only does HASH_REMOVE inside
>>>>>
>>>>> if (entry->refcount == 0 && !SPI_plan_is_valid(plan))
>>>>>
>>>>> which is the uncommon case. Normally the entry stays behind with
>>>>> refcount 0 forever. I think the removal should happen whenever the
>>>>> count reaches 0, independently of plan validity.
>>>>>
>>>>>
>>>>> 3. Keying the hash table on the raw SPIPlanPtr looks fragile.
>>>>>
>>>>> Because entries outlive the plans they describe (point 2), the table
>>>>> accumulates entries keyed on pointers that have since been freed. That
>>>>> would be harmless if addresses were never reused, but they are.
>>>>> Driving
>>>>> 40 plan invalidations through ALTER TABLE, I only ever saw three
>>>>> distinct plan addresses, cycling:
>>>>>
>>>>> entries=1 plan=0x60f5822cf900
>>>>> entries=2 plan=0x60f5822ce8d0
>>>>> entries=3 plan=0x60f5822ce0b0
>>>>> ... then those same three addresses repeatedly, entries stuck at 3
>>>>>
>>>>> So a freshly created plan routinely lands on an address that already
>>>>> has
>>>>> an entry and inherits whatever refcount it was left holding.
>>>>>
>>>>> I want to be careful not to overstate this: in every path I could
>>>>> actually reach, the inherited value was 0, and I could not turn this
>>>>> into a demonstrable failure. So treat it as a design concern rather
>>>>> than a proven bug. But combined with point 1, which does leave counts
>>>>> above zero, a new plan could start life pinned and never be freed -- or
>>>>> a count could reach zero while an outer reentrant frame still holds the
>>>>> plan, which is the use-after-free this patch exists to prevent.
>>>>> Storing
>>>>> the refcount on the RI_QueryHashEntry that already owns the plan would
>>>>> sidestep the question entirely.
>>>>>
>>>>>
>>>>> 4. entry->refcount-- is unguarded.
>>>>>
>>>>> It is a uint32, so a stray extra Finished() call (or a stale entry per
>>>>> point 3) wraps it to 4294967295 rather than tripping anything. An
>>>>> Assert(entry->refcount > 0) before the decrement would catch that in
>>>>> cassert builds.
>>>>>
>>>>>
>>>>> 5. The ri_InitHashTables() call in ri_PreparedPlanExecutionStarted().
>>>>>
>>>>> if (!ri_query_plan_cache_executing_refcount)
>>>>> ri_InitHashTables();
>>>>>
>>>>> ri_InitHashTables() unconditionally recreates all four hash tables and
>>>>> re-runs both CacheRegisterSyscacheCallback() calls. If this branch
>>>>> were
>>>>> ever taken with the other caches already populated it would orphan
>>>>> ri_constraint_cache, ri_query_cache and ri_compare_cache, and register
>>>>> duplicate syscache callbacks against a limited pool. In practice
>>>>> ri_PerformCheck() is only reached after the caches exist, so the branch
>>>>> looks unreachable -- which argues for an Assert instead, or for
>>>>> splitting the refcount table's initialisation out.
>>>>>
>>>>>
>>>>> 6. Style points
>>>>>
>>>>> - "// Remove the entry" needs to be a /* */ comment.
>>>>> - "RI_QueryPlanCacheExecutingRefCountEntry* entry" should be
>>>>> "... *entry" (three occurrences).
>>>>> - "bool found" is declared in ri_PreparedPlanExecutionFinished() and
>>>>> ri_PreparedPlanCanRelease() but never read; both test !entry.
>>>>> - Several lines run to 103-131 columns.
>>>>> - The three new functions have no comment headers, unlike their
>>>>> neighbours in this file.
>>>>> - "this call can free the plan..." should start with a capital.
>>>>>
>>>>> All pgindent/typedefs.list territory rather than anything substantive.
>>>>>
>>>>>
>>>>> == Test patch ==
>>>>>
>>>>> 1. The identifiers are in Spanish -- crash_reentrancia_tabla_
>>>>> autoreferencial, nombre, padre_id, crash_reentrancia_segunda_tabla,
>>>>> valor. The rest of the tree is English, so these will need renaming.
>>>>>
>>>>> 2. The test leans on overflowing the shared invalidation queue with
>>>>> 1000
>>>>> temp table create/drops. It does reproduce reliably here (7.2s), but
>>>>> nothing makes it fail loudly if that stops being enough -- it would
>>>>> just
>>>>> start passing on an unfixed backend. Is there a way to make the
>>>>> invalidation deterministic?
>>>>>
>>>>> 3. A crash test in the isolation schedule takes down the whole cluster
>>>>> when it fails, aborting the rest of the schedule. I do not know the
>>>>> project's preference here, but it may be worth asking whether this
>>>>> belongs in src/test/isolation or as a TAP test.
>>>>>
>>>>> 4. The expected output only shows that s1_delete completed; it does not
>>>>> check the resulting table contents. Asserting the surviving rows would
>>>>> turn "did not crash" into "cascaded correctly".
>>>>>
>>>>> 5. Four of the whitespace errors above are in this file.
>>>>>
>>>>>
>>>>> == Regression testing ==
>>>>>
>>>>> Full "meson test" with the patch: 359 ok, 49 skipped, 1 failed.
>>>>>
>>>>> The failure was recovery/027_stream_regress, with
>>>>>
>>>>> TRAP: failed Assert("plan->magic == _SPI_PLAN_MAGIC"),
>>>>> File: "../src/backend/executor/spi.c", Line: 1951
>>>>> client backend was terminated by signal 6: Aborted
>>>>> DETAIL: Failed process was running: UPDATE temporal_mltrng
>>>>> SET valid_at = datemultirange(daterange('
>>>>> 2016-02-01','2016-03-01'))
>>>>> WHERE id = '[5,6)' AND ...
>>>>>
>>>>> I initially assumed the patch had caused this, since it is the same
>>>>> use-after-free shape the patch is about. It has not. Clean master
>>>>> with
>>>>> no patch applied reproduces the identical assertion. Counts over 15
>>>>> runs each:
>>>>>
>>>>> master : 5/15 runs hit the assert (33%)
>>>>> patched : 9/15 runs hit the assert (60%)
>>>>>
>>>>> Fisher exact two-tailed p = 0.27, so the difference is not significant
>>>>> at these sample sizes and I am not claiming the patch makes it worse --
>>>>> only that it does not fix it and that the failure is pre-existing. I
>>>>> will report that one separately rather than tangle it up with this
>>>>> thread.
>>>>>
>>>>> Aside from 027_stream_regress, nothing regressed.
>>>>>
>>>>>
>>>>> == Summary ==
>>>>>
>>>>> The crash is real, easy to trigger, and the patch fixes it. I would
>>>>> call the direction sound but the bookkeeping not ready: point 1 is a
>>>>> straightforward leak on a common error path, and point 3 makes me
>>>>> uneasy
>>>>> about the choice of hash key.
>>>>>
>>>>> Marking this Waiting on Author. Happy to retest a v4.
>>>>>
>>>>> On the wider question about whether a BEFORE DELETE trigger should be
>>>>> deleting rows at all -- I do not have the standing to argue that either
>>>>> way. But a backend that segfaults seems worth closing regardless of
>>>>> whether the usage is advisable, and if the consensus is that it should
>>>>> not be allowed, an explicit error would still need this same reentrancy
>>>>> information to detect the situation.
>>>>>
>>>>> Regards,
>>>>> Trakshan Mishra
>>>>>
>>>>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Joao Detomini 2026-10-05 15:04:14 Re: pg_resetwal: refuse to run when backup_label exists
Previous Message Manu 2026-10-05 15:01:05 Re: doc: Document Linux cgroup memory limits