public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Langote <[email protected]>
To: Evan Montgomery-Recht <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3
Date: Wed, 8 Apr 2026 23:26:21 +0900
Message-ID: <CA+HiwqFt4NGTNk7BinOsHHM48E9zGAa852vCfGoSe1bbL=JNFQ@mail.gmail.com> (raw)
In-Reply-To: <CA+HiwqFdkJGBU9QmYkm6056SuhunGk7yFCuVP=2vBejJa+qhGg@mail.gmail.com>
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>
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.
--
Thanks, Amit Langote
Attachments:
[application/octet-stream] v3-0004-Store-batch-callbacks-at-the-appropriate-level-ra.patch (7.8K, 2-v3-0004-Store-batch-callbacks-at-the-appropriate-level-ra.patch)
download | inline diff:
From 14eb87d068e46939c325c2f070c66f4dfb4f064a Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Wed, 8 Apr 2026 23:22:50 +0900
Subject: [PATCH v3 4/4] Store batch callbacks at the appropriate level rather
than depth-matching
Instead of tagging each AfterTriggerCallbackItem with a firing depth
and matching at invocation time, store callbacks directly 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 (deferred firing at COMMIT).
FireAfterTriggerBatchCallbacks() is simplified to just iterate and
invoke the passed list. Memory cleanup is handled by the caller:
AfterTriggerFreeQuery() for query-level callbacks and
AfterTriggerEndXact() for the top-level list.
This eliminates the firing_depth field from AfterTriggerCallbackItem
and the depth-matched iteration logic, replacing it with natural
list-level scoping. The firing_depth counter in AfterTriggersData
is retained solely for AfterTriggerIsActive().
---
src/backend/commands/trigger.c | 90 ++++++++++++----------------------
1 file changed, 32 insertions(+), 58 deletions(-)
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index bbc2405cc4a..993a00aec8c 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -3902,8 +3902,8 @@ typedef struct AfterTriggersData
*/
int firing_depth;
- List *batch_callbacks; /* List of AfterTriggerCallbackItem,
- * possibly from multiple firing depths */
+ List *batch_callbacks; /* List of AfterTriggerCallbackItem;
+ * for deferred constraints */
} AfterTriggersData;
struct AfterTriggersQueryData
@@ -3911,6 +3911,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
@@ -3944,8 +3945,6 @@ struct AfterTriggersTableData
typedef struct AfterTriggerCallbackItem
{
AfterTriggerBatchCallback callback;
- int firing_depth; /* afterTriggerFiringDepth when registered;
- * callback fires only at this depth */
void *arg;
} AfterTriggerCallbackItem;
@@ -3984,7 +3983,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
@@ -5237,17 +5236,15 @@ 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);
+ afterTriggers.firing_depth--;
/* Release query-level-local storage, including tuplestores if any */
AfterTriggerFreeQuery(&afterTriggers.query_stack[afterTriggers.query_depth]);
afterTriggers.query_depth--;
- afterTriggers.firing_depth--;
}
@@ -5304,6 +5301,9 @@ AfterTriggerFreeQuery(AfterTriggersQueryData *qs)
*/
qs->tables = NIL;
list_free_deep(tables);
+
+ list_free_deep(qs->batch_callbacks);
+ qs->batch_callbacks = NIL;
}
@@ -5353,8 +5353,7 @@ AfterTriggerFireDeferred(void)
}
/* Flush any fast-path batches accumulated by the triggers just fired. */
- FireAfterTriggerBatchCallbacks();
-
+ FireAfterTriggerBatchCallbacks(afterTriggers.batch_callbacks);
afterTriggers.firing_depth--;
/*
@@ -5424,14 +5423,8 @@ AfterTriggerEndXact(bool isCommit)
afterTriggers.firing_depth = 0;
- Assert(afterTriggers.batch_callbacks == NIL || !isCommit);
-
- /* On abort, discard any pending callbacks without firing them. */
- if (!isCommit)
- {
- list_free_deep(afterTriggers.batch_callbacks);
- afterTriggers.batch_callbacks = NIL;
- }
+ list_free_deep(afterTriggers.batch_callbacks);
+ afterTriggers.batch_callbacks = NIL;
}
/*
@@ -5732,6 +5725,7 @@ AfterTriggerEnlargeQueryState(void)
qs->events.tailfree = NULL;
qs->fdw_tuplestore = NULL;
qs->tables = NIL;
+ qs->batch_callbacks = NIL;
++init_depth;
}
@@ -6114,7 +6108,9 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt)
/*
* Flush any fast-path batches accumulated by the triggers just fired.
*/
- FireAfterTriggerBatchCallbacks();
+ FireAfterTriggerBatchCallbacks(afterTriggers.batch_callbacks);
+ list_free_deep(afterTriggers.batch_callbacks);
+ afterTriggers.batch_callbacks = NIL;
afterTriggers.firing_depth--;
if (snapshot_set)
@@ -6843,60 +6839,38 @@ RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback,
oldcxt = MemoryContextSwitchTo(TopTransactionContext);
item = palloc(sizeof(AfterTriggerCallbackItem));
item->callback = callback;
- item->firing_depth = afterTriggers.firing_depth;
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 callbacks registered at the current firing depth.
- *
- * Each callback is tagged with the afterTriggerFiringDepth at registration
- * time. Only callbacks matching the current depth are invoked; the rest
- * are retained for when their own depth fires. This ensures that nested
- * trigger-firing contexts (e.g., SPI calls inside AFTER triggers) only
- * fire the callbacks they registered, leaving outer-level callbacks intact
- * until their firing depth is reached.
+ * Invoke all callbacks in the given list.
*
- * The list is updated before any callbacks are invoked so that if a
- * callback throws an ERROR the list is already in a consistent state.
+ * 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)
{
- List *remaining = NIL;
- List *to_fire = NIL;
ListCell *lc;
- /* remaining and to_fire lists must survive until callbacks complete */
- MemoryContext oldcxt = MemoryContextSwitchTo(TopTransactionContext);
-
- foreach(lc, afterTriggers.batch_callbacks)
- {
- AfterTriggerCallbackItem *item = lfirst(lc);
-
- if (item->firing_depth == afterTriggers.firing_depth)
- to_fire = lappend(to_fire, item);
- else
- remaining = lappend(remaining, item);
- }
-
- list_free(afterTriggers.batch_callbacks);
- afterTriggers.batch_callbacks = remaining;
- MemoryContextSwitchTo(oldcxt);
-
- /* Now fire; if one throws, the list is already clean */
- foreach(lc, to_fire)
+ foreach(lc, callbacks)
{
AfterTriggerCallbackItem *item = lfirst(lc);
item->callback(item->arg);
- pfree(item);
}
- list_free(to_fire);
}
/*
--
2.47.3
[application/octet-stream] v3-0002-Fix-RI-fast-path-crash-under-nested-C-level-SPI.patch (5.6K, 3-v3-0002-Fix-RI-fast-path-crash-under-nested-C-level-SPI.patch)
download | inline diff:
From 312fad1c36e064ab9e7dc1780575e8c07f300751 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Wed, 8 Apr 2026 18:17:40 +0900
Subject: [PATCH v3 2/4] 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 tagging each AfterTriggerCallbackItem with the
afterTriggerFiringDepth (added in 5c54c3ed1b9) at registration time
and firing only callbacks whose depth matches the current depth. This
replaces the query_depth > 0 suppression guard. Callbacks now fire at
the same firing depth at which they were registered, while the
resource owner that was active during registration is still alive,
eliminating the mismatch.
While at it, ensure callbacks are properly accounted for at all
transaction boundaries, as cleanup of b7b27eb41a5c: assert on commit
that no callbacks remain unfired, and discard any remaining callbacks
on transaction abort. Also restructure FireAfterTriggerBatchCallbacks()
to update afterTriggers.batch_callbacks before invoking any callbacks,
so that if a callback throws an ERROR the list is already in a
consistent state.
Note that ri_PerformCheck() uses fire_triggers=false, which skips
AfterTriggerBeginQuery/EndQuery and thus never increments
afterTriggerFiringDepth; events queued there fire at the outer
query's depth and are unaffected by this change.
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/backend/commands/trigger.c | 54 +++++++++++++++++++++++++++-------
1 file changed, 43 insertions(+), 11 deletions(-)
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index c41005ba44e..f59537fe86e 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -3935,6 +3935,8 @@ struct AfterTriggersTableData
typedef struct AfterTriggerCallbackItem
{
AfterTriggerBatchCallback callback;
+ int firing_depth; /* afterTriggerFiringDepth when registered;
+ * callback fires only at this depth */
void *arg;
} AfterTriggerCallbackItem;
@@ -5419,6 +5421,15 @@ AfterTriggerEndXact(bool isCommit)
afterTriggers.query_depth = -1;
afterTriggerFiringDepth = 0;
+
+ Assert(afterTriggers.batch_callbacks == NIL || !isCommit);
+
+ /* On abort, discard any pending callbacks without firing them. */
+ if (!isCommit)
+ {
+ list_free_deep(afterTriggers.batch_callbacks);
+ afterTriggers.batch_callbacks = NIL;
+ }
}
/*
@@ -6830,6 +6841,7 @@ RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback,
oldcxt = MemoryContextSwitchTo(TopTransactionContext);
item = palloc(sizeof(AfterTriggerCallbackItem));
item->callback = callback;
+ item->firing_depth = afterTriggerFiringDepth;
item->arg = arg;
afterTriggers.batch_callbacks =
lappend(afterTriggers.batch_callbacks, item);
@@ -6838,31 +6850,51 @@ RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback,
/*
* FireAfterTriggerBatchCallbacks
- * Invoke and clear all registered batch callbacks.
+ * Invoke callbacks registered at the current firing depth.
+ *
+ * Each callback is tagged with the afterTriggerFiringDepth at registration
+ * time. Only callbacks matching the current depth are invoked; the rest
+ * are retained for when their own depth fires. This ensures that nested
+ * trigger-firing contexts (e.g., SPI calls inside AFTER triggers) only
+ * fire the callbacks they registered, leaving outer-level callbacks intact
+ * until their firing depth is reached.
*
- * 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.
+ * The list is updated before any callbacks are invoked so that if a
+ * callback throws an ERROR the list is already in a consistent state.
*/
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);
- Assert(afterTriggerFiringDepth > 0);
foreach(lc, afterTriggers.batch_callbacks)
{
AfterTriggerCallbackItem *item = lfirst(lc);
- item->callback(item->arg);
+ if (item->firing_depth == afterTriggerFiringDepth)
+ to_fire = lappend(to_fire, item);
+ else
+ remaining = lappend(remaining, item);
}
- list_free_deep(afterTriggers.batch_callbacks);
- afterTriggers.batch_callbacks = NIL;
+ list_free(afterTriggers.batch_callbacks);
+ afterTriggers.batch_callbacks = remaining;
+ MemoryContextSwitchTo(oldcxt);
+
+ /* Now fire; if one throws, the list is already clean */
+ foreach(lc, to_fire)
+ {
+ AfterTriggerCallbackItem *item = lfirst(lc);
+
+ item->callback(item->arg);
+ pfree(item);
+ }
+ list_free(to_fire);
}
/*
--
2.47.3
[application/octet-stream] v3-0003-Move-afterTriggerFiringDepth-into-AfterTriggersDa.patch (5.6K, 4-v3-0003-Move-afterTriggerFiringDepth-into-AfterTriggersDa.patch)
download | inline diff:
From 4ed90a5b74c77c5e4a2f5b0a602a06a372aec795 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Wed, 8 Apr 2026 23:22:41 +0900
Subject: [PATCH v3 3/4] Move afterTriggerFiringDepth into AfterTriggersData
The static variable afterTriggerFiringDepth 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.
---
src/backend/commands/trigger.c | 42 ++++++++++++++++++----------------
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index f59537fe86e..bbc2405cc4a 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -3894,7 +3894,16 @@ typedef struct AfterTriggersData
AfterTriggersTransData *trans_stack; /* array of structs shown below */
int maxtransdepth; /* allocated len of above array */
- List *batch_callbacks; /* List of AfterTriggerCallbackItem */
+ /*
+ * Incremented around the trigger-firing loops in AfterTriggerEndQuery,
+ * AfterTriggerFireDeferred, and AfterTriggerSetState. Used by
+ * AfterTriggerIsActive() and to tag batch callbacks with the depth at
+ * which they should fire.
+ */
+ int firing_depth;
+
+ List *batch_callbacks; /* List of AfterTriggerCallbackItem,
+ * possibly from multiple firing depths */
} AfterTriggersData;
struct AfterTriggersQueryData
@@ -3942,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,
@@ -5108,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;
/*
@@ -5122,7 +5125,6 @@ AfterTriggerBeginXact(void)
Assert(afterTriggers.events.head == NULL);
Assert(afterTriggers.trans_stack == NULL);
Assert(afterTriggers.maxtransdepth == 0);
- Assert(afterTriggerFiringDepth == 0);
}
@@ -5194,7 +5196,7 @@ AfterTriggerEndQuery(EState *estate)
*/
qs = &afterTriggers.query_stack[afterTriggers.query_depth];
- afterTriggerFiringDepth++;
+ afterTriggers.firing_depth++;
for (;;)
{
if (afterTriggerMarkEvents(&qs->events, &afterTriggers.events, true))
@@ -5245,7 +5247,7 @@ AfterTriggerEndQuery(EState *estate)
AfterTriggerFreeQuery(&afterTriggers.query_stack[afterTriggers.query_depth]);
afterTriggers.query_depth--;
- afterTriggerFiringDepth--;
+ afterTriggers.firing_depth--;
}
@@ -5341,7 +5343,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++;
@@ -5353,7 +5355,7 @@ AfterTriggerFireDeferred(void)
/* Flush any fast-path batches accumulated by the triggers just fired. */
FireAfterTriggerBatchCallbacks();
- afterTriggerFiringDepth--;
+ afterTriggers.firing_depth--;
/*
* We don't bother freeing the event list, since it will go away anyway
@@ -5420,7 +5422,7 @@ AfterTriggerEndXact(bool isCommit)
/* No more afterTriggers manipulation until next transaction starts. */
afterTriggers.query_depth = -1;
- afterTriggerFiringDepth = 0;
+ afterTriggers.firing_depth = 0;
Assert(afterTriggers.batch_callbacks == NIL || !isCommit);
@@ -6079,7 +6081,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++;
@@ -6113,7 +6115,7 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt)
* Flush any fast-path batches accumulated by the triggers just fired.
*/
FireAfterTriggerBatchCallbacks();
- afterTriggerFiringDepth--;
+ afterTriggers.firing_depth--;
if (snapshot_set)
PopActiveSnapshot();
@@ -6837,11 +6839,11 @@ 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);
oldcxt = MemoryContextSwitchTo(TopTransactionContext);
item = palloc(sizeof(AfterTriggerCallbackItem));
item->callback = callback;
- item->firing_depth = afterTriggerFiringDepth;
+ item->firing_depth = afterTriggers.firing_depth;
item->arg = arg;
afterTriggers.batch_callbacks =
lappend(afterTriggers.batch_callbacks, item);
@@ -6876,7 +6878,7 @@ FireAfterTriggerBatchCallbacks(void)
{
AfterTriggerCallbackItem *item = lfirst(lc);
- if (item->firing_depth == afterTriggerFiringDepth)
+ if (item->firing_depth == afterTriggers.firing_depth)
to_fire = lappend(to_fire, item);
else
remaining = lappend(remaining, item);
@@ -6908,5 +6910,5 @@ FireAfterTriggerBatchCallbacks(void)
bool
AfterTriggerIsActive(void)
{
- return afterTriggerFiringDepth > 0;
+ return afterTriggers.firing_depth > 0;
}
--
2.47.3
[application/octet-stream] v3-0001-Modified-test-suite-from-Evan-s-patch.patch (17.1K, 5-v3-0001-Modified-test-suite-from-Evan-s-patch.patch)
download | inline diff:
From 2da74146aafbd9d505e9e0d9038138bc46f0cd08 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Wed, 8 Apr 2026 13:53:08 +0900
Subject: [PATCH v3 1/4] Modified test suite from Evan's patch
The C function in the original test module was using the same resource
owner as the parent SELECT, so the crash could not be reproduced.
Added a dedicated resource owner around the SPI call to ensure the inner
resource owner is released before the outer trigger-firing batch callback
fires, which is necessary to trigger the crash this test is meant to
cover.
---
src/test/modules/Makefile | 1 +
src/test/modules/meson.build | 1 +
src/test/modules/test_spi_resowner/Makefile | 23 ++++
.../expected/ri_fastpath.out | 118 ++++++++++++++++++
.../modules/test_spi_resowner/meson.build | 31 +++++
.../test_spi_resowner/sql/ri_fastpath.sql | 107 ++++++++++++++++
.../test_spi_resowner--1.0.sql | 9 ++
.../test_spi_resowner/test_spi_resowner.c | 70 +++++++++++
.../test_spi_resowner.control | 4 +
9 files changed, 364 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..ad6b0f7c9b3
--- /dev/null
+++ b/src/test/modules/test_spi_resowner/expected/ri_fastpath.out
@@ -0,0 +1,118 @@
+--
+-- 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.
+-- Without the fix this crashes the server.
+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, not crash
+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 firing depth and
+-- must fire before the inner resource owner is released. This exercises
+-- the depth-matched callback firing introduced to fix that crash.
+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; the FK batch callback fires at the
+-- inner SPI's firing depth before that resource owner is released.
+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..4517b2437c4
--- /dev/null
+++ b/src/test/modules/test_spi_resowner/sql/ri_fastpath.sql
@@ -0,0 +1,107 @@
+--
+-- 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.
+-- Without the fix this crashes the server.
+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, not crash
+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 firing depth and
+-- must fire before the inner resource owner is released. This exercises
+-- the depth-matched callback firing introduced to fix that crash.
+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; the FK batch callback fires at the
+-- inner SPI's firing depth before that resource owner is released.
+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]
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3
In-Reply-To: <CA+HiwqFt4NGTNk7BinOsHHM48E9zGAa852vCfGoSe1bbL=JNFQ@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