public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Langote <[email protected]>
To: Chao Li <[email protected]>
Cc: Evan Montgomery-Recht <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3
Date: Thu, 9 Apr 2026 17:40:49 +0900
Message-ID: <CA+HiwqFWLR01NjK5Y+MKiyaqg64ThVS7UYKK3ZBVNHvtm=3-ug@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CA+HiwqF4C0ws3cO+z5cLkPuvwnAwkSp7sfvgGj3yQ=Li6KNMqA@mail.gmail.com>
<CA+HiwqGM6nvAV5O+=Nr+BXMPWOma0oeCRVzVP0XiLE8zX5TVAg@mail.gmail.com>
<CA+HiwqGMaovCUgDbGxVGnK0Mrivr+ph3YE2Ws+47-ugyPb4f7g@mail.gmail.com>
<CAFj8pRDaiBe_GOLk_yyYHTtPiDAAaLOM8u1-=Q3ZgXBTH+1igg@mail.gmail.com>
<CA+HiwqGA5Ay_MR0eJEEbt4j6WrVh4F+AasTp8yCbs5aJLOJn6Q@mail.gmail.com>
<CAEG8a3JM=NoqiTK0V6S9FNxZPvy1+C5F7rfafTtPKBVWnunL-g@mail.gmail.com>
<CA+HiwqEyiLCY6MTLbOJXDdLNNQLaURYHvdW797MQgbjEK9od4Q@mail.gmail.com>
<CAEG8a3+VBpwPf1Rm-ECD90whM9b3YnGhux5CVXdsL6khiBfzRQ@mail.gmail.com>
<CA+HiwqF2UHzF0sKCp-F2a-U29rqh_9ZPy=f1h+Fh_=M8efj3pg@mail.gmail.com>
<CAEG8a3L9Ew-WL8sxLROVOcypeaENPmd8qCmMvz4geoGL1TDGCA@mail.gmail.com>
<CAEG8a3+nUFQo4sdPQF9xy0J73J8RFJ5U9A5+_kMosGDaZ+1sXA@mail.gmail.com>
<[email protected]>
<CAEG8a3JyKdizWvYsF+z_mA1BKy=dpW11iKVMOG=bk6Tbz6M1Bw@mail.gmail.com>
<CAEG8a3+Hf4tvvbts29_k_AFhWQmRYfEo_SW4C5FY_140iKghBw@mail.gmail.com>
<CA+HiwqFV-PY-3BxM6j5TaAiC3AwedDxo-6vwRSbvygg3zF+xAQ@mail.gmail.com>
<CA+HiwqHpaisS-e+0gAgzh92qZAFxncAJMmmTRZZN=efoeTPgOg@mail.gmail.com>
<CA+HiwqFwZ6WLRbY8R7VC7JVp5Jot6ktZOkr9wDxTjoK=W1SgdQ@mail.gmail.com>
<CA+HiwqF_Kz=R8juHJBiOATvabWSOugK4VaGOxoJ_n=E2c7UM9w@mail.gmail.com>
<CA+HiwqHCB7kcbspkhaLN9enoj5x=ehzhFM4PXDgWUUP8Px41GA@mail.gmail.com>
<[email protected]>
<CA+HiwqHpVtP485wEKuXdOkdoZWhvVvfFH40-og07Jp3MPx21eg@mail.gmail.com>
<CAEG8a3JWHkJSXe9nNcVK7wnYKUEqWuMGFDhy5BynB_9tEjmEUg@mail.gmail.com>
<CA+HiwqFjfumKrWy03q5M309xJJVYt0WgGfH6AZ8BjFhSwppwsQ@mail.gmail.com>
<CAEG8a3JjP1LaKSv-r3AMJLRyLMzENJrKshWsDvDouMPM_sizmA@mail.gmail.com>
<CA+HiwqFQ+ZA7hSOygv4uv_t75B3r0_gosjadetCsAEoaZwTu6g@mail.gmail.com>
<CA+HiwqHdB0r8z6Mut13BxpYNq2W-os+Arju4mcZbCyU9PeaVog@mail.gmail.com>
<CAEG8a3K5ayVNkCDnK9OtNb+4OY0chJtW6ObgEOSFjqyymQda8Q@mail.gmail.com>
<CA+HiwqGJYCgEs_F-LBtrRdx-Y969LMr-OVogjFXU6U-0q5bOwQ@mail.gmail.com>
<CA+HiwqF2Ma6R_QyjfPBvFreYbezFpGcwASJE0a2bM+Y0jvof+g@mail.gmail.com>
<[email protected]>
<CA+HiwqGFRAH2O5bGpNMErFopFyn-2-Zu3+5y+BFQim9TE8z+Pw@mail.gmail.com>
<[email protected]>
<CA+HiwqFx=aciJYkkaviyTutUm303QXz6GtXSqzG7nfd4MAzddQ@mail.gmail.com>
<CA+HiwqH39QU7vGVx65JH1e0nzVvQc5eVmuY7=qyj0T_+b-HO3A@mail.gmail.com>
<[email protected]>
<CA+HiwqF+jAyHUiNzpR+vRBbpeCiVAdFU52-ffTGko9Zit317oA@mail.gmail.com>
<CAEg7pwcKf01FmDqFAf-Hzu_pYnMYScY_Otid-pe9uw3BJ6gq9g@mail.gmail.com>
<CA+HiwqFK8rXTNknxV0MMe++7W08g3kY6eeLK21A1xCrK2Wuk8Q@mail.gmail.com>
<CA+HiwqFdkJGBU9QmYkm6056SuhunGk7yFCuVP=2vBejJa+qhGg@mail.gmail.com>
<CA+HiwqFt4NGTNk7BinOsHHM48E9zGAa852vCfGoSe1bbL=JNFQ@mail.gmail.com>
<[email protected]>
Hi,
On Thu, Apr 9, 2026 at 4:40 PM Chao Li <[email protected]> wrote:
> > On Apr 8, 2026, at 22:26, Amit Langote <[email protected]> wrote:
> > On Wed, Apr 8, 2026 at 6:58 PM Amit Langote <[email protected]> wrote:
> >> On Wed, Apr 8, 2026 at 10:23 AM Amit Langote <[email protected]> wrote:
> >>> On Tue, Apr 7, 2026 at 10:00 PM Evan Montgomery-Recht
> >>> <[email protected]> wrote:
> >>>> The patch also adds a test module (test_spi_func) with a C function
> >>>> that executes SQL via SPI_connect/SPI_execute/SPI_finish, since this
> >>>> crash cannot be triggered from PL/pgSQL. The test exercises the
> >>>> C-level SPI INSERT with multiple FK constraints, FK violations, and
> >>>> nested PL/pgSQL-calls-C-SPI (matching the PostGIS call pattern).
> >>
> >> I applied only the test module changes and it passes (without
> >> crashing) even without your proposed fix. It seems that's because the
> >> C function in test_spi_func calling SPI is using the same resource
> >> owner as the parent SELECT. I think you'd need to create a resource
> >> owner manually in the spi_exec() C function to reproduce the crash, as
> >> done in the attached 0001, which contains the src/test changes
> >> extracted from your patch modified as described, including renaming
> >> the C function to spi_exec_sql().
> >>
> >> Also, the test cases that call spi_exec() (_sql()) directly from a
> >> SELECT don't actually exercise the crash path because there is no
> >> outer trigger-firing loop active. query_depth is 0 inside the inner
> >> SPI's AfterTriggerEndQuery, so the old guard wouldn't suppress the
> >> callback there anyway. The critical case requires spi_exec_sql() to be
> >> called from inside an AFTER trigger, where query_depth > 0 causes the
> >> guard to defer the callback past the inner resource owner's lifetime.
> >> I've added that test case. I kept your original test cases as they
> >> still provide useful coverage of C-level SPI FK behavior even if they
> >> don't exercise the crash path specifically. Maybe your original
> >> PostGIS test suite that hit the crash did have the right structure,
> >> but that's not reflected in the patch as far as I can tell.
> >>
> >> I've also renamed the module to test_spi_resowner to better reflect
> >> what it's about.
> >>
> >> For the fix, I have a different proposal. As you observed, the
> >> query_depth > 0 early return in FireAfterTriggerBatchCallbacks() means
> >> that the nested SPI's callbacks get called under the outer resource
> >> owner, which may not be the same as the one that SPI used. I think it
> >> was a mistake to have that early return in the first place. Instead we
> >> could remember for each callback what firing level it should be called
> >> at, so the nested SPI's callbacks fire before returning to the parent
> >> level and parent-level callbacks fire when the parent level completes.
> >> I have implemented that in the attached 0002 along with transaction
> >> boundary cleanup of callbacks, which passes the check-world for me,
> >> but I'll need to stare some more at it before committing.
> >>
> >> Let me know if this also fixes your own in-house test suite or if you
> >> have any other suggestions or if you think I am missing something.
> >
> > One more cleanup patch attached as 0003: afterTriggerFiringDepth was
> > added by commit 5c54c3ed1 as a file-static variable, which in
> > hindsight should have been a field in AfterTriggersData alongside the
> > other per-transaction after-trigger state. This patch makes that
> > correction.
> >
> > One alternative design worth considering for 0002: storing
> > batch_callbacks per query level in AfterTriggersQueryData rather than
> > as a single list in AfterTriggersData, so callbacks naturally live at
> > the query level where they were registered and get cleaned up with
> > AfterTriggerFreeQuery on abort. Deferred constraints still need a
> > top-level list in AfterTriggersData since they fire outside any query
> > level. FireAfterTriggerBatchCallbacks() takes a list parameter and the
> > caller passes either the query-level or top-level list as appropriate.
> > This eliminates the need for firing_depth-matched firing entirely. I
> > did that in 0004. I think I like it over 0002. Will look more
> > closely tomorrow morning.
> A few comments on v3:
Thanks for the review.
> 1 - 0002
> ```
> static void
> FireAfterTriggerBatchCallbacks(void)
> {
> + List *remaining = NIL;
> + List *to_fire = NIL;
> ListCell *lc;
>
> - if (afterTriggers.query_depth > 0)
> - return;
> + /* remaining and to_fire lists must survive until callbacks complete */
> + MemoryContext oldcxt = MemoryContextSwitchTo(TopTransactionContext);
> ```
>
> I think remaining and to_fire should stay in the same context of afterTriggers.batch_callbacks, so instead of hard coding TopTransactionContext, we can use GetMemoryChunkContext(afterTriggers.batch_callbacks), which makes the intention explicit.
I'm dropping 0002 or have merged 0004 into it so this memory context
switch is no longer present.
> 2 - 0004, I noticed one potential problem, although I am not sure whether it can really happen in practice. This version stores callback items at the individual query depth, and FireAfterTriggerBatchCallbacks() now iterates the callback list for that depth and invokes each callback directly. My concern is that if one of those callbacks needs to register a new callback, that would append a new item to the same list while it is being iterated. That seems unsafe to me, because list append may create a new list structure underneath. If that happens, we may end up modifying the list being traversed, which does not look safe.
>
> This problem doesn’t exist in 0002, because 0002 splits afterTriggers.batch_callbacks into remaining and to_fire, and reset afterTriggers.batch_callbacks = remaining before running callbacks. But the problem is, if a callback registers a new callback, the new callback goes to afterTriggers.batch_callbacks, so it won’t get executed.
>
> From this perspective, I would assume a callback should not be allowed to register a new callback. Can you please help confirm?
Good point on the re-entrant registration concern. I've added a
firing_batch_callbacks flag to AfterTriggersData that prevents
callbacks from registering new callbacks during
FireAfterTriggerBatchCallbacks(), with an Assert in
RegisterAfterTriggerBatchCallback() to enforce it. That should keep
the list being iterated from being modified.
The attached patches are updated accordingly. 0001 is the main fix
incorporating the per-query-level storage design, the transaction
boundary cleanup, and the firing_batch_callbacks guard. 0002 is a
followup that moves afterTriggerFiringDepth into AfterTriggersData as
a minor cleanup of 5c54c3ed1b9. Barring further feedback I plan to
commit 0001 and 0002 shortly. For 0003, I need to check on the policy
around adding new test modules during feature freeze before committing
it.
--
Thanks, Amit Langote
Attachments:
[application/octet-stream] v4-0002-Move-afterTriggerFiringDepth-into-AfterTriggersDa.patch (5.6K, 2-v4-0002-Move-afterTriggerFiringDepth-into-AfterTriggersDa.patch)
download | inline diff:
From 4bd1ded2c80d1c1294af5e6a190debafd4866ceb Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Thu, 9 Apr 2026 13:46:45 +0900
Subject: [PATCH v4 2/3] Move afterTriggerFiringDepth into AfterTriggersData
The static variable afterTriggerFiringDepth introduced by commit
5c54c3ed1b9 is logically part of the after-trigger state. Move it
into AfterTriggersData as firing_depth, alongside query_depth and
the other per-transaction after-trigger state. Also update its
comment to accurately reflect its sole remaining purpose: signaling
to AfterTriggerIsActive() that after-trigger firing is active.
Discussion: https://postgr.es/m/CA+HiwqFt4NGTNk7BinOsHHM48E9zGAa852vCfGoSe1bbL=JNFQ@mail.gmail.com
---
src/backend/commands/trigger.c | 36 +++++++++++++++++-----------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 9c6125623e0..28187fe8c06 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -3898,6 +3898,13 @@ typedef struct AfterTriggersData
* for deferred constraints */
bool firing_batch_callbacks; /* true when in
* FireAfterTriggersBatchCallbacks() */
+
+ /*
+ * Incremented around the trigger-firing loops in AfterTriggerEndQuery,
+ * AfterTriggerFireDeferred, and AfterTriggerSetState. Used by
+ * AfterTriggerIsActive() to signal that after-trigger firing is active.
+ */
+ int firing_depth;
} AfterTriggersData;
struct AfterTriggersQueryData
@@ -3944,13 +3951,6 @@ typedef struct AfterTriggerCallbackItem
static AfterTriggersData afterTriggers;
-/*
- * Incremented before invoking afterTriggerInvokeEvents(). Used by
- * AfterTriggerIsActive() to determine whether batch callbacks will fire,
- * so that RI trigger functions can take the batched fast path.
- */
-static int afterTriggerFiringDepth = 0;
-
static void AfterTriggerExecute(EState *estate,
AfterTriggerEvent event,
ResultRelInfo *relInfo,
@@ -5110,6 +5110,7 @@ AfterTriggerBeginXact(void)
*/
afterTriggers.firing_counter = (CommandId) 1; /* mustn't be 0 */
afterTriggers.query_depth = -1;
+ afterTriggers.firing_depth = 0;
afterTriggers.batch_callbacks = NIL;
afterTriggers.firing_batch_callbacks = false;
@@ -5125,7 +5126,6 @@ AfterTriggerBeginXact(void)
Assert(afterTriggers.events.head == NULL);
Assert(afterTriggers.trans_stack == NULL);
Assert(afterTriggers.maxtransdepth == 0);
- Assert(afterTriggerFiringDepth == 0);
}
@@ -5197,7 +5197,7 @@ AfterTriggerEndQuery(EState *estate)
*/
qs = &afterTriggers.query_stack[afterTriggers.query_depth];
- afterTriggerFiringDepth++;
+ afterTriggers.firing_depth++;
for (;;)
{
if (afterTriggerMarkEvents(&qs->events, &afterTriggers.events, true))
@@ -5246,7 +5246,7 @@ AfterTriggerEndQuery(EState *estate)
AfterTriggerFreeQuery(&afterTriggers.query_stack[afterTriggers.query_depth]);
afterTriggers.query_depth--;
- afterTriggerFiringDepth--;
+ afterTriggers.firing_depth--;
}
@@ -5345,7 +5345,7 @@ AfterTriggerFireDeferred(void)
* Run all the remaining triggers. Loop until they are all gone, in case
* some trigger queues more for us to do.
*/
- afterTriggerFiringDepth++;
+ afterTriggers.firing_depth++;
while (afterTriggerMarkEvents(events, NULL, false))
{
CommandId firing_id = afterTriggers.firing_counter++;
@@ -5357,7 +5357,7 @@ AfterTriggerFireDeferred(void)
/* Flush any fast-path batches accumulated by the triggers just fired. */
FireAfterTriggerBatchCallbacks(afterTriggers.batch_callbacks);
- afterTriggerFiringDepth--;
+ afterTriggers.firing_depth--;
/*
* We don't bother freeing the event list or batch_callbacks, since
@@ -5425,7 +5425,7 @@ AfterTriggerEndXact(bool isCommit)
/* No more afterTriggers manipulation until next transaction starts. */
afterTriggers.query_depth = -1;
- afterTriggerFiringDepth = 0;
+ afterTriggers.firing_depth = 0;
list_free_deep(afterTriggers.batch_callbacks);
afterTriggers.batch_callbacks = NIL;
@@ -6083,7 +6083,7 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt)
AfterTriggerEventList *events = &afterTriggers.events;
bool snapshot_set = false;
- afterTriggerFiringDepth++;
+ afterTriggers.firing_depth++;
while (afterTriggerMarkEvents(events, NULL, true))
{
CommandId firing_id = afterTriggers.firing_counter++;
@@ -6117,7 +6117,7 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt)
* Flush any fast-path batches accumulated by the triggers just fired.
*/
FireAfterTriggerBatchCallbacks(afterTriggers.batch_callbacks);
- afterTriggerFiringDepth--;
+ afterTriggers.firing_depth--;
list_free_deep(afterTriggers.batch_callbacks);
afterTriggers.batch_callbacks = NIL;
@@ -6843,7 +6843,7 @@ RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback,
* Must be called while afterTriggers is active; callbacks registered
* outside a trigger-firing context would never fire.
*/
- Assert(afterTriggerFiringDepth > 0);
+ Assert(afterTriggers.firing_depth > 0);
Assert(!afterTriggers.firing_batch_callbacks);
oldcxt = MemoryContextSwitchTo(TopTransactionContext);
item = palloc(sizeof(AfterTriggerCallbackItem));
@@ -6874,7 +6874,7 @@ FireAfterTriggerBatchCallbacks(List *callbacks)
{
ListCell *lc;
- Assert(afterTriggerFiringDepth > 0);
+ Assert(afterTriggers.firing_depth > 0);
afterTriggers.firing_batch_callbacks = true;
foreach(lc, callbacks)
{
@@ -6896,5 +6896,5 @@ FireAfterTriggerBatchCallbacks(List *callbacks)
bool
AfterTriggerIsActive(void)
{
- return afterTriggerFiringDepth > 0;
+ return afterTriggers.firing_depth > 0;
}
--
2.47.3
[application/octet-stream] v4-0001-Fix-RI-fast-path-crash-under-nested-C-level-SPI.patch (9.3K, 3-v4-0001-Fix-RI-fast-path-crash-under-nested-C-level-SPI.patch)
download | inline diff:
From 2343a90020cf2445dd574d7ca43ea4d460820a74 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Thu, 9 Apr 2026 17:39:25 +0900
Subject: [PATCH v4 1/3] Fix RI fast-path crash under nested C-level SPI
When a C-language function uses SPI_connect/SPI_execute/SPI_finish to
INSERT into a table with FK constraints, the FK AFTER triggers
register ri_FastPathEndBatch as a batch callback and open PK relations
under the SPI portal's resource owner. FireAfterTriggerBatchCallbacks
was suppressed at that point by the query_depth > 0 guard, deferring
teardown to the outer query's AfterTriggerEndQuery. By then the SPI
portal's resource owner had been released, decrementing the cached
relations' refcounts to zero. ri_FastPathTeardown then crashed
attempting to close them:
TRAP: failed Assert("rel->rd_refcnt > 0")
Fix by storing batch callbacks at the level where they should fire:
in AfterTriggersQueryData.batch_callbacks for immediate constraints
(fired by AfterTriggerEndQuery) and in AfterTriggersData.batch_callbacks
for deferred constraints (fired by AfterTriggerFireDeferred and
AfterTriggerSetState). RegisterAfterTriggerBatchCallback() routes the
callback to the current query-level list when query_depth >= 0, and to
the top-level list otherwise. FireAfterTriggerBatchCallbacks() takes a
list parameter and simply iterates and invokes it; memory cleanup is
handled by the caller. This replaces the query_depth > 0 guard and
the firing_depth field in AfterTriggerCallbackItem with natural
list-level scoping. The firing_depth counter in AfterTriggersData is
retained solely for AfterTriggerIsActive().
Also add firing_batch_callbacks to AfterTriggersData to detect and
prevent re-entrant callback registration during
FireAfterTriggerBatchCallbacks(), which would be unsafe as it could
modify the list being iterated. The flag is reset at transaction and
subtransaction boundaries to handle cases where an error thrown by a
callback is caught and the subtransaction is rolled back.
While at it, ensure callbacks are properly accounted for at all
transaction boundaries, as cleanup of b7b27eb41a5c: discard any
remaining top-level callbacks on both commit and abort in
AfterTriggerEndXact(), and clean up query-level callbacks in
AfterTriggerFreeQuery().
Note that ri_PerformCheck() calls SPI with fire_triggers=false, which
skips AfterTriggerBeginQuery/EndQuery for that SPI command. Any
callbacks registered by triggers fired during that SPI command land at
the outer query's level and fire when the outer query completes, which
is the correct behavior.
Reported-by: Evan Montgomery-Recht <[email protected]>
Analyzed-by: Evan Montgomery-Recht <[email protected]>
Author: Amit Langote <[email protected]>
Reviewed-by: Chao Li <[email protected]>
Discussion: https://postgr.es/m/CAEg7pwcKf01FmDqFAf-Hzu_pYnMYScY_Otid-pe9uw3BJ6gq9g@mail.gmail.com
---
src/backend/commands/trigger.c | 70 ++++++++++++++++++++++------------
1 file changed, 45 insertions(+), 25 deletions(-)
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index c41005ba44e..9c6125623e0 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -3894,7 +3894,10 @@ typedef struct AfterTriggersData
AfterTriggersTransData *trans_stack; /* array of structs shown below */
int maxtransdepth; /* allocated len of above array */
- List *batch_callbacks; /* List of AfterTriggerCallbackItem */
+ List *batch_callbacks; /* List of AfterTriggerCallbackItem;
+ * for deferred constraints */
+ bool firing_batch_callbacks; /* true when in
+ * FireAfterTriggersBatchCallbacks() */
} AfterTriggersData;
struct AfterTriggersQueryData
@@ -3902,6 +3905,7 @@ struct AfterTriggersQueryData
AfterTriggerEventList events; /* events pending from this query */
Tuplestorestate *fdw_tuplestore; /* foreign tuples for said events */
List *tables; /* list of AfterTriggersTableData, see below */
+ List *batch_callbacks; /* List of AfterTriggerCallbackItem */
};
struct AfterTriggersTransData
@@ -3980,7 +3984,7 @@ static SetConstraintState SetConstraintStateAddItem(SetConstraintState state,
Oid tgoid, bool tgisdeferred);
static void cancel_prior_stmt_triggers(Oid relid, CmdType cmdType, int tgevent);
-static void FireAfterTriggerBatchCallbacks(void);
+static void FireAfterTriggerBatchCallbacks(List *callbacks);
/*
* Get the FDW tuplestore for the current trigger query level, creating it
@@ -5107,6 +5111,7 @@ AfterTriggerBeginXact(void)
afterTriggers.firing_counter = (CommandId) 1; /* mustn't be 0 */
afterTriggers.query_depth = -1;
afterTriggers.batch_callbacks = NIL;
+ afterTriggers.firing_batch_callbacks = false;
/*
* Verify that there is no leftover state remaining. If these assertions
@@ -5233,11 +5238,9 @@ AfterTriggerEndQuery(EState *estate)
/*
* Fire batch callbacks before releasing query-level storage and before
* decrementing query_depth. Callbacks may do real work (index probes,
- * error reporting) and rely on query_depth still reflecting the current
- * batch level so that nested calls from SPI inside AFTER triggers are
- * correctly suppressed by FireAfterTriggerBatchCallbacks's depth guard.
+ * error reporting).
*/
- FireAfterTriggerBatchCallbacks();
+ FireAfterTriggerBatchCallbacks(qs->batch_callbacks);
/* Release query-level-local storage, including tuplestores if any */
AfterTriggerFreeQuery(&afterTriggers.query_stack[afterTriggers.query_depth]);
@@ -5300,6 +5303,9 @@ AfterTriggerFreeQuery(AfterTriggersQueryData *qs)
*/
qs->tables = NIL;
list_free_deep(tables);
+
+ list_free_deep(qs->batch_callbacks);
+ qs->batch_callbacks = NIL;
}
@@ -5349,13 +5355,14 @@ AfterTriggerFireDeferred(void)
}
/* Flush any fast-path batches accumulated by the triggers just fired. */
- FireAfterTriggerBatchCallbacks();
+ FireAfterTriggerBatchCallbacks(afterTriggers.batch_callbacks);
afterTriggerFiringDepth--;
/*
- * We don't bother freeing the event list, since it will go away anyway
- * (and more efficiently than via pfree) in AfterTriggerEndXact.
+ * We don't bother freeing the event list or batch_callbacks, since
+ * they will go away anyway (and more efficiently than via pfree) in
+ * AfterTriggerEndXact.
*/
if (snap_pushed)
@@ -5419,6 +5426,10 @@ AfterTriggerEndXact(bool isCommit)
afterTriggers.query_depth = -1;
afterTriggerFiringDepth = 0;
+
+ list_free_deep(afterTriggers.batch_callbacks);
+ afterTriggers.batch_callbacks = NIL;
+ afterTriggers.firing_batch_callbacks = false;
}
/*
@@ -5565,6 +5576,9 @@ AfterTriggerEndSubXact(bool isCommit)
}
}
}
+
+ /* Reset in case a callback threw an error while firing. */
+ afterTriggers.firing_batch_callbacks = false;
}
/*
@@ -5719,6 +5733,7 @@ AfterTriggerEnlargeQueryState(void)
qs->events.tailfree = NULL;
qs->fdw_tuplestore = NULL;
qs->tables = NIL;
+ qs->batch_callbacks = NIL;
++init_depth;
}
@@ -6101,8 +6116,10 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt)
/*
* Flush any fast-path batches accumulated by the triggers just fired.
*/
- FireAfterTriggerBatchCallbacks();
+ FireAfterTriggerBatchCallbacks(afterTriggers.batch_callbacks);
afterTriggerFiringDepth--;
+ list_free_deep(afterTriggers.batch_callbacks);
+ afterTriggers.batch_callbacks = NIL;
if (snapshot_set)
PopActiveSnapshot();
@@ -6827,42 +6844,45 @@ RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback,
* outside a trigger-firing context would never fire.
*/
Assert(afterTriggerFiringDepth > 0);
+ Assert(!afterTriggers.firing_batch_callbacks);
oldcxt = MemoryContextSwitchTo(TopTransactionContext);
item = palloc(sizeof(AfterTriggerCallbackItem));
item->callback = callback;
item->arg = arg;
- afterTriggers.batch_callbacks =
- lappend(afterTriggers.batch_callbacks, item);
+ if (afterTriggers.query_depth >= 0)
+ {
+ AfterTriggersQueryData *qs =
+ &afterTriggers.query_stack[afterTriggers.query_depth];
+ qs->batch_callbacks = lappend(qs->batch_callbacks, item);
+ }
+ else
+ afterTriggers.batch_callbacks =
+ lappend(afterTriggers.batch_callbacks, item);
MemoryContextSwitchTo(oldcxt);
}
/*
* FireAfterTriggerBatchCallbacks
- * Invoke and clear all registered batch callbacks.
+ * Invoke all callbacks in the given list.
*
- * Only fires at the outermost query level (query_depth == 0) or from
- * top-level operations (query_depth == -1, e.g. AfterTriggerFireDeferred
- * at COMMIT). Nested queries from SPI inside AFTER triggers run at
- * depth > 0 and must not tear down resources the outer batch still needs.
+ * Memory cleanup of the list and its items is handled by the caller
+ * (AfterTriggerFreeQuery for query-level callbacks, AfterTriggerEndXact
+ * for top-level deferred callbacks).
*/
static void
-FireAfterTriggerBatchCallbacks(void)
+FireAfterTriggerBatchCallbacks(List *callbacks)
{
ListCell *lc;
- if (afterTriggers.query_depth > 0)
- return;
-
Assert(afterTriggerFiringDepth > 0);
- foreach(lc, afterTriggers.batch_callbacks)
+ afterTriggers.firing_batch_callbacks = true;
+ foreach(lc, callbacks)
{
AfterTriggerCallbackItem *item = lfirst(lc);
item->callback(item->arg);
}
-
- list_free_deep(afterTriggers.batch_callbacks);
- afterTriggers.batch_callbacks = NIL;
+ afterTriggers.firing_batch_callbacks = false;
}
/*
--
2.47.3
[application/octet-stream] v4-0003-Add-test-module-for-RI-fast-path-FK-checks-under-.patch (17.5K, 4-v4-0003-Add-test-module-for-RI-fast-path-FK-checks-under-.patch)
download | inline diff:
From 0cfd3e2ab44bcd5fcddad3173e37de62bfd1a842 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Thu, 9 Apr 2026 14:44:51 +0900
Subject: [PATCH v4 3/3] Add test module for RI fast-path FK checks under
C-level SPI
Add test_spi_resowner, a test module providing a SQL-callable C function
that executes SQL via SPI with a dedicated short-lived resource owner.
This reproduces the crash scenario fixed by the previous commit that
cannot be triggered from PL/pgSQL, since PL/pgSQL's SPI connection spans
the entire function call and its resource owner outlives the batch
callback.
The critical test case calls spi_exec_sql() from inside an AFTER trigger,
where the FK checks fire under a nested SPI context while the outer
trigger-firing loop is active. The dedicated resource owner ensures it is
released before the outer batch callback fires, reproducing the resource
owner mismatch that previously caused a crash. Additional test cases
exercise multiple FK constraints, FK violations, and PL/pgSQL calling the
C SPI function, matching the PostGIS toTopoGeom() call pattern reported
by Evan Montgomery-Recht.
Reported-by: Evan Montgomery-Recht <[email protected]>
Author: Evan Montgomery-Recht <[email protected]>
Co-authored-by: Amit Langote <[email protected]>
Discussion: https://postgr.es/m/CAEg7pwcKf01FmDqFAf-Hzu_pYnMYScY_Otid-pe9uw3BJ6gq9g@mail.gmail.com
---
src/test/modules/Makefile | 1 +
src/test/modules/meson.build | 1 +
src/test/modules/test_spi_resowner/Makefile | 23 ++++
.../expected/ri_fastpath.out | 116 ++++++++++++++++++
.../modules/test_spi_resowner/meson.build | 31 +++++
.../test_spi_resowner/sql/ri_fastpath.sql | 105 ++++++++++++++++
.../test_spi_resowner--1.0.sql | 9 ++
.../test_spi_resowner/test_spi_resowner.c | 70 +++++++++++
.../test_spi_resowner.control | 4 +
9 files changed, 360 insertions(+)
create mode 100644 src/test/modules/test_spi_resowner/Makefile
create mode 100644 src/test/modules/test_spi_resowner/expected/ri_fastpath.out
create mode 100644 src/test/modules/test_spi_resowner/meson.build
create mode 100644 src/test/modules/test_spi_resowner/sql/ri_fastpath.sql
create mode 100644 src/test/modules/test_spi_resowner/test_spi_resowner--1.0.sql
create mode 100644 src/test/modules/test_spi_resowner/test_spi_resowner.c
create mode 100644 src/test/modules/test_spi_resowner/test_spi_resowner.control
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 0a74ab5c86f..016b328c8c5 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -52,6 +52,7 @@ SUBDIRS = \
test_shmem \
test_shm_mq \
test_slru \
+ test_spi_resowner \
test_tidstore \
unsafe_tests \
worker_spi \
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 4bca42bb370..3ca454064d0 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -53,6 +53,7 @@ subdir('test_saslprep')
subdir('test_shmem')
subdir('test_shm_mq')
subdir('test_slru')
+subdir('test_spi_resowner')
subdir('test_tidstore')
subdir('typcache')
subdir('unsafe_tests')
diff --git a/src/test/modules/test_spi_resowner/Makefile b/src/test/modules/test_spi_resowner/Makefile
new file mode 100644
index 00000000000..5a69e3a3c42
--- /dev/null
+++ b/src/test/modules/test_spi_resowner/Makefile
@@ -0,0 +1,23 @@
+# src/test/modules/test_spi_resowner/Makefile
+
+MODULE_big = test_spi_resowner
+OBJS = \
+ $(WIN32RES) \
+ test_spi_resowner.o
+PGFILEDESC = "test_spi_resowner - SQL-callable C SPI function under a dedicated ResourceOwner"
+
+EXTENSION = test_spi_resowner
+DATA = test_spi_resowner--1.0.sql
+
+REGRESS = ri_fastpath
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_spi_resowner
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_spi_resowner/expected/ri_fastpath.out b/src/test/modules/test_spi_resowner/expected/ri_fastpath.out
new file mode 100644
index 00000000000..03984ca892e
--- /dev/null
+++ b/src/test/modules/test_spi_resowner/expected/ri_fastpath.out
@@ -0,0 +1,116 @@
+--
+-- Test RI fast-path FK check under C-level SPI.
+--
+-- The RI fast-path caches PK relation references in ri_FastPathGetEntry()
+-- under the current resource owner. When FK triggers fire inside a
+-- C-level SPI context that creates a dedicated short-lived resource owner,
+-- those references must be released before the inner resource owner is
+-- released. The fix ensures batch callbacks fire at the same firing depth
+-- at which they were registered, while the corresponding resource owner
+-- is still alive. Without this, ri_FastPathTeardown would crash with
+-- Assert(rel->rd_refcnt > 0) in index_close.
+--
+-- Simple PL/pgSQL does not trigger this because its SPI connection spans
+-- the entire function call, so its resource owner outlives the batch
+-- callback. The critical test case requires a C function that creates a
+-- dedicated short-lived resource owner around its SPI call.
+--
+CREATE EXTENSION test_spi_resowner;
+CREATE TABLE ri_fp_pk1 (id serial PRIMARY KEY);
+CREATE TABLE ri_fp_pk2 (id serial PRIMARY KEY);
+CREATE TABLE ri_fp_pk3 (id serial PRIMARY KEY);
+INSERT INTO ri_fp_pk1 VALUES (1);
+INSERT INTO ri_fp_pk2 VALUES (1);
+INSERT INTO ri_fp_pk3 VALUES (1);
+CREATE TABLE ri_fp_fk (
+ id serial PRIMARY KEY,
+ a int REFERENCES ri_fp_pk1(id),
+ b int REFERENCES ri_fp_pk2(id),
+ c int REFERENCES ri_fp_pk3(id),
+ d int REFERENCES ri_fp_pk1(id),
+ e int REFERENCES ri_fp_pk2(id),
+ f int REFERENCES ri_fp_pk3(id)
+);
+-- C-level SPI INSERT: the critical test case.
+SELECT spi_exec_sql(
+ 'INSERT INTO ri_fp_fk (a, b, c, d, e, f) VALUES (1, 1, 1, 1, 1, 1)');
+ spi_exec_sql
+--------------
+
+(1 row)
+
+-- Additional C-level SPI INSERTs to exercise batch reuse across calls.
+-- Use different column orderings to ensure each is a distinct statement.
+SELECT spi_exec_sql(
+ 'INSERT INTO ri_fp_fk (f, e, d, c, b, a) VALUES (1, 1, 1, 1, 1, 1)');
+ spi_exec_sql
+--------------
+
+(1 row)
+
+SELECT spi_exec_sql(
+ 'INSERT INTO ri_fp_fk (a, c, e, b, d, f) VALUES (1, 1, 1, 1, 1, 1)');
+ spi_exec_sql
+--------------
+
+(1 row)
+
+-- C-level SPI with FK violation: should error
+SELECT spi_exec_sql(
+ 'INSERT INTO ri_fp_fk (a, b, c, d, e, f) VALUES (999, 1, 1, 1, 1, 1)');
+ERROR: insert or update on table "ri_fp_fk" violates foreign key constraint "ri_fp_fk_a_fkey"
+DETAIL: Key (a)=(999) is not present in table "ri_fp_pk1".
+CONTEXT: SQL statement "INSERT INTO ri_fp_fk (a, b, c, d, e, f) VALUES (999, 1, 1, 1, 1, 1)"
+-- Nested: PL/pgSQL calling C SPI (mimics PostGIS toTopoGeom pattern)
+CREATE FUNCTION plpgsql_calls_c_spi() RETURNS void AS $$
+DECLARE
+ ins_stmt text := 'INSERT INTO ri_fp_fk (a, b, c, d, e, f) VALUES (1, 1, 1, 1, 1, 1)';
+BEGIN
+ PERFORM spi_exec_sql(ins_stmt);
+END;
+$$ LANGUAGE plpgsql;
+SELECT plpgsql_calls_c_spi();
+ plpgsql_calls_c_spi
+---------------------
+
+(1 row)
+
+-- AFTER trigger that uses C-level SPI to insert into an FK-referencing table.
+-- The FK batch callback is registered at the inner SPI's query level and
+-- must fire before the inner resource owner is released.
+CREATE TABLE ri_fp_outer (id int PRIMARY KEY);
+CREATE TABLE ri_fp_inner (id int REFERENCES ri_fp_pk1(id));
+CREATE FUNCTION outer_trigger_spi_ok() RETURNS trigger AS $$
+BEGIN
+ PERFORM spi_exec_sql('INSERT INTO ri_fp_inner VALUES (1)');
+ RETURN NEW;
+END $$ LANGUAGE plpgsql;
+CREATE TRIGGER outer_tg AFTER INSERT ON ri_fp_outer
+ FOR EACH ROW EXECUTE FUNCTION outer_trigger_spi_ok();
+-- Fires outer_tg, whose PL/pgSQL body calls spi_exec_sql(). The C function
+-- creates a dedicated resource owner that is released after the FK batch
+-- callback fires.
+INSERT INTO ri_fp_outer VALUES (1);
+CREATE FUNCTION outer_trigger_spi_fail() RETURNS trigger AS $$
+BEGIN
+ PERFORM spi_exec_sql('INSERT INTO ri_fp_inner VALUES (3)');
+ RETURN NEW;
+END $$ LANGUAGE plpgsql;
+DROP TRIGGER outer_tg ON ri_fp_outer;
+DROP FUNCTION outer_trigger_spi_ok();
+CREATE TRIGGER outer_tg AFTER INSERT ON ri_fp_outer
+ FOR EACH ROW EXECUTE FUNCTION outer_trigger_spi_fail();
+-- Like above but the inner insert fails.
+INSERT INTO ri_fp_outer VALUES (2);
+ERROR: insert or update on table "ri_fp_inner" violates foreign key constraint "ri_fp_inner_id_fkey"
+DETAIL: Key (id)=(3) is not present in table "ri_fp_pk1".
+CONTEXT: SQL statement "INSERT INTO ri_fp_inner VALUES (3)"
+SQL statement "SELECT spi_exec_sql('INSERT INTO ri_fp_inner VALUES (3)')"
+PL/pgSQL function outer_trigger_spi_fail() line 3 at PERFORM
+DROP TRIGGER outer_tg ON ri_fp_outer;
+DROP FUNCTION outer_trigger_spi_fail();
+DROP TABLE ri_fp_inner, ri_fp_outer;
+-- Cleanup
+DROP TABLE ri_fp_fk;
+DROP TABLE ri_fp_pk3, ri_fp_pk2, ri_fp_pk1;
+DROP EXTENSION test_spi_resowner;
diff --git a/src/test/modules/test_spi_resowner/meson.build b/src/test/modules/test_spi_resowner/meson.build
new file mode 100644
index 00000000000..fbb027e05c7
--- /dev/null
+++ b/src/test/modules/test_spi_resowner/meson.build
@@ -0,0 +1,31 @@
+test_spi_resowner_sources = files(
+ 'test_spi_resowner.c',
+)
+
+if host_system == 'windows'
+ test_spi_resowner_sources += rc_lib_gen.process(win32ver_rc, extra_args: [
+ '--NAME', 'test_spi_resowner',
+ '--FILEDESC', 'test_spi_resowner - SQL-callable C SPI function under a dedicated ResourceOwner',])
+endif
+
+test_spi_resowner = shared_module('test_spi_resowner',
+ test_spi_resowner_sources,
+ kwargs: pg_test_mod_args,
+)
+test_install_libs += test_spi_resowner
+
+test_install_data += files(
+ 'test_spi_resowner.control',
+ 'test_spi_resowner--1.0.sql',
+)
+
+tests += {
+ 'name': 'test_spi_resowner',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'regress': {
+ 'sql': [
+ 'ri_fastpath',
+ ],
+ },
+}
diff --git a/src/test/modules/test_spi_resowner/sql/ri_fastpath.sql b/src/test/modules/test_spi_resowner/sql/ri_fastpath.sql
new file mode 100644
index 00000000000..11a561a06ac
--- /dev/null
+++ b/src/test/modules/test_spi_resowner/sql/ri_fastpath.sql
@@ -0,0 +1,105 @@
+--
+-- Test RI fast-path FK check under C-level SPI.
+--
+-- The RI fast-path caches PK relation references in ri_FastPathGetEntry()
+-- under the current resource owner. When FK triggers fire inside a
+-- C-level SPI context that creates a dedicated short-lived resource owner,
+-- those references must be released before the inner resource owner is
+-- released. The fix ensures batch callbacks fire at the same firing depth
+-- at which they were registered, while the corresponding resource owner
+-- is still alive. Without this, ri_FastPathTeardown would crash with
+-- Assert(rel->rd_refcnt > 0) in index_close.
+--
+-- Simple PL/pgSQL does not trigger this because its SPI connection spans
+-- the entire function call, so its resource owner outlives the batch
+-- callback. The critical test case requires a C function that creates a
+-- dedicated short-lived resource owner around its SPI call.
+--
+CREATE EXTENSION test_spi_resowner;
+
+CREATE TABLE ri_fp_pk1 (id serial PRIMARY KEY);
+CREATE TABLE ri_fp_pk2 (id serial PRIMARY KEY);
+CREATE TABLE ri_fp_pk3 (id serial PRIMARY KEY);
+INSERT INTO ri_fp_pk1 VALUES (1);
+INSERT INTO ri_fp_pk2 VALUES (1);
+INSERT INTO ri_fp_pk3 VALUES (1);
+
+CREATE TABLE ri_fp_fk (
+ id serial PRIMARY KEY,
+ a int REFERENCES ri_fp_pk1(id),
+ b int REFERENCES ri_fp_pk2(id),
+ c int REFERENCES ri_fp_pk3(id),
+ d int REFERENCES ri_fp_pk1(id),
+ e int REFERENCES ri_fp_pk2(id),
+ f int REFERENCES ri_fp_pk3(id)
+);
+
+-- C-level SPI INSERT: the critical test case.
+SELECT spi_exec_sql(
+ 'INSERT INTO ri_fp_fk (a, b, c, d, e, f) VALUES (1, 1, 1, 1, 1, 1)');
+
+-- Additional C-level SPI INSERTs to exercise batch reuse across calls.
+-- Use different column orderings to ensure each is a distinct statement.
+SELECT spi_exec_sql(
+ 'INSERT INTO ri_fp_fk (f, e, d, c, b, a) VALUES (1, 1, 1, 1, 1, 1)');
+SELECT spi_exec_sql(
+ 'INSERT INTO ri_fp_fk (a, c, e, b, d, f) VALUES (1, 1, 1, 1, 1, 1)');
+
+-- C-level SPI with FK violation: should error
+SELECT spi_exec_sql(
+ 'INSERT INTO ri_fp_fk (a, b, c, d, e, f) VALUES (999, 1, 1, 1, 1, 1)');
+
+-- Nested: PL/pgSQL calling C SPI (mimics PostGIS toTopoGeom pattern)
+CREATE FUNCTION plpgsql_calls_c_spi() RETURNS void AS $$
+DECLARE
+ ins_stmt text := 'INSERT INTO ri_fp_fk (a, b, c, d, e, f) VALUES (1, 1, 1, 1, 1, 1)';
+BEGIN
+ PERFORM spi_exec_sql(ins_stmt);
+END;
+$$ LANGUAGE plpgsql;
+
+SELECT plpgsql_calls_c_spi();
+
+-- AFTER trigger that uses C-level SPI to insert into an FK-referencing table.
+-- The FK batch callback is registered at the inner SPI's query level and
+-- must fire before the inner resource owner is released.
+CREATE TABLE ri_fp_outer (id int PRIMARY KEY);
+CREATE TABLE ri_fp_inner (id int REFERENCES ri_fp_pk1(id));
+
+CREATE FUNCTION outer_trigger_spi_ok() RETURNS trigger AS $$
+BEGIN
+ PERFORM spi_exec_sql('INSERT INTO ri_fp_inner VALUES (1)');
+ RETURN NEW;
+END $$ LANGUAGE plpgsql;
+
+CREATE TRIGGER outer_tg AFTER INSERT ON ri_fp_outer
+ FOR EACH ROW EXECUTE FUNCTION outer_trigger_spi_ok();
+
+-- Fires outer_tg, whose PL/pgSQL body calls spi_exec_sql(). The C function
+-- creates a dedicated resource owner that is released after the FK batch
+-- callback fires.
+INSERT INTO ri_fp_outer VALUES (1);
+
+CREATE FUNCTION outer_trigger_spi_fail() RETURNS trigger AS $$
+BEGIN
+ PERFORM spi_exec_sql('INSERT INTO ri_fp_inner VALUES (3)');
+ RETURN NEW;
+END $$ LANGUAGE plpgsql;
+
+DROP TRIGGER outer_tg ON ri_fp_outer;
+DROP FUNCTION outer_trigger_spi_ok();
+
+CREATE TRIGGER outer_tg AFTER INSERT ON ri_fp_outer
+ FOR EACH ROW EXECUTE FUNCTION outer_trigger_spi_fail();
+
+-- Like above but the inner insert fails.
+INSERT INTO ri_fp_outer VALUES (2);
+
+DROP TRIGGER outer_tg ON ri_fp_outer;
+DROP FUNCTION outer_trigger_spi_fail();
+DROP TABLE ri_fp_inner, ri_fp_outer;
+
+-- Cleanup
+DROP TABLE ri_fp_fk;
+DROP TABLE ri_fp_pk3, ri_fp_pk2, ri_fp_pk1;
+DROP EXTENSION test_spi_resowner;
diff --git a/src/test/modules/test_spi_resowner/test_spi_resowner--1.0.sql b/src/test/modules/test_spi_resowner/test_spi_resowner--1.0.sql
new file mode 100644
index 00000000000..29ef70ee0dc
--- /dev/null
+++ b/src/test/modules/test_spi_resowner/test_spi_resowner--1.0.sql
@@ -0,0 +1,9 @@
+/* src/test/modules/test_spi_resowner/test_spi_resowner--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_spi_resowner" to load this file. \quit
+
+CREATE FUNCTION spi_exec_sql(query text)
+RETURNS void
+AS 'MODULE_PATHNAME', 'spi_exec_sql'
+LANGUAGE C STRICT;
diff --git a/src/test/modules/test_spi_resowner/test_spi_resowner.c b/src/test/modules/test_spi_resowner/test_spi_resowner.c
new file mode 100644
index 00000000000..0306139b5c0
--- /dev/null
+++ b/src/test/modules/test_spi_resowner/test_spi_resowner.c
@@ -0,0 +1,70 @@
+/*-------------------------------------------------------------------------
+ *
+ * test_spi_resowner.c
+ * SQL-callable C function that uses SPI to execute a query.
+ *
+ * Useful for testing code paths that only trigger under C-level
+ * SPI (not PL/pgSQL), such as resource owner interactions with
+ * RI fast-path FK checks.
+ *
+ * Copyright (c) 2026, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/test/modules/test_spi_resowner/test_spi_resowner.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "executor/spi.h"
+#include "utils/builtins.h"
+
+PG_MODULE_MAGIC;
+
+PG_FUNCTION_INFO_V1(spi_exec_sql);
+
+/*
+ * spi_exec_sql(query text) - execute a SQL query via SPI.
+ *
+ * Opens a fresh SPI connection, executes the query, and closes the
+ * connection. Creates a dedicated child resource owner around the
+ * SPI_execute call and releases it before returning, ensuring that
+ * any resources registered under it (such as relation references
+ * opened by RI fast-path FK checks) are released before the outer
+ * trigger-firing batch callback fires. This reproduces the resource
+ * owner mismatch that occurs with C-language extensions like PostGIS
+ * topology functions, which cannot be triggered from PL/pgSQL since
+ * PL/pgSQL's SPI connection spans the entire function call.
+ */
+Datum
+spi_exec_sql(PG_FUNCTION_ARGS)
+{
+ const char *query = text_to_cstring(PG_GETARG_TEXT_PP(0));
+ int ret;
+ ResourceOwner save = CurrentResourceOwner;
+ ResourceOwner childowner = ResourceOwnerCreate(save, "test_spi inner");
+
+ SPI_connect();
+
+ CurrentResourceOwner = childowner;
+ ret = SPI_execute(query, false, 0);
+
+ if (ret < 0)
+ elog(ERROR, "SPI_execute failed: error code %d", ret);
+
+ SPI_finish();
+
+ CurrentResourceOwner = save;
+ ResourceOwnerRelease(childowner,
+ RESOURCE_RELEASE_BEFORE_LOCKS,
+ true, false);
+ ResourceOwnerRelease(childowner,
+ RESOURCE_RELEASE_LOCKS,
+ true, false);
+ ResourceOwnerRelease(childowner,
+ RESOURCE_RELEASE_AFTER_LOCKS,
+ true, false);
+ ResourceOwnerDelete(childowner);
+
+ PG_RETURN_VOID();
+}
diff --git a/src/test/modules/test_spi_resowner/test_spi_resowner.control b/src/test/modules/test_spi_resowner/test_spi_resowner.control
new file mode 100644
index 00000000000..2120ae9442f
--- /dev/null
+++ b/src/test/modules/test_spi_resowner/test_spi_resowner.control
@@ -0,0 +1,4 @@
+comment = 'Test SQL-callable C function that uses SPI using dedicated ResourceOwner'
+default_version = '1.0'
+module_pathname = '$libdir/test_spi_resowner'
+relocatable = true
--
2.47.3
view thread (63+ 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]
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3
In-Reply-To: <CA+HiwqFWLR01NjK5Y+MKiyaqg64ThVS7UYKK3ZBVNHvtm=3-ug@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