postgres.git / summary / log / commit / refs
commit d2a710c7e9e08e101694fe6c1f6789f18f9d3dcf
Author: Amit Langote <amitlan@postgresql.org>
Date: Thu Aug 20 08:12:53 2026 +0000
Track RI fast-path FK-check batches per firing cycle
Commit 34a30786293 fixed an RI fast-path crash under nested C-level SPI
by keeping batch-callback lists per after-trigger query depth. That fix
was incomplete: the RI fast path still tracked callback registration
with one global flag. Once an outer firing cycle had registered its
callback, the flag suppressed registration for a nested cycle, leaving
the nested batch to be handled by the outer callback, too late and with
the wrong snapshot, potentially after the ResourceOwner holding its
relations had gone away.
Nor is per-depth callback registration sufficient while the cache is
keyed only by constraint OID. If nested firing checks the same
constraint, it reuses the outer entry, combining rows that must be
checked in separate firing cycles.
Key the cache by both constraint OID and query depth. Register a callback
for each depth that creates an entry, and make ri_FastPathEndBatch() flush
and release only entries belonging to the ending depth. Add
AfterTriggerCurrentQueryDepth() so ri_triggers.c can obtain the current
depth; depth -1 represents deferred firing.
Add regression coverage for nested firing through a cursor portal, whose
resources must not outlive the nested cycle, nested firing of the same
constraint at different query depths, and deferred firing at query depth
-1.
Reported-by: Noah Misch <noah@leadboat.com>
Reported-by: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/20260705222115.be.noahmisch@microsoft.com
Discussion: https://postgr.es/m/CAH2-Wz=D533JbF_ak_Pc8kP0FKse-ju8DnMxtjvY==yHsP4xgw@mail.gmail.com
Backpatch-through: 19
src/backend/commands/trigger.c | 14 ++++
src/backend/utils/adt/ri_triggers.c | 106 +++++++++++++++++++++++-------
src/include/commands/trigger.h | 1 +
src/test/regress/expected/foreign_key.out | 92 ++++++++++++++++++++++++++
src/test/regress/sql/foreign_key.sql | 78 ++++++++++++++++++++++
src/tools/pgindent/typedefs.list | 1 +
6 files changed, 269 insertions(+), 23 deletions(-)
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index f1914aac4c6..911045b9b9d 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -6951,3 +6951,17 @@ AfterTriggerIsActive(void)
{
return afterTriggers.firing_depth > 0;
}
+
+/*
+ * AfterTriggerCurrentQueryDepth
+ * Return the current after-trigger query nesting depth.
+ *
+ * Lets a batch-callback registrant (e.g. the RI fast path) associate cached
+ * state with the firing cycle that created it, so a nested cycle's callback
+ * acts only on its own entries. Returns -1 outside any query level.
+ */
+int
+AfterTriggerCurrentQueryDepth(void)
+{
+ return afterTriggers.query_depth;
+}
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index f76eb37731e..5a209673b61 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -231,13 +231,27 @@ typedef struct RI_CompareHashEntry
*/
#define RI_FASTPATH_BATCH_SIZE 64
+/*
+ * RI_FastPathKey
+ * Hash key for an RI_FastPathEntry.
+ *
+ * A constraint can be checked in nested trigger-firing cycles. Each cycle
+ * must have a separate entry so that its rows are checked with that cycle's
+ * snapshot and its resources are released by that cycle's callback.
+ */
+typedef struct RI_FastPathKey
+{
+ Oid conoid; /* pg_constraint OID */
+ int query_depth; /* after-trigger query depth */
+} RI_FastPathKey;
+
/*
* RI_FastPathEntry
- * Per-constraint cache of resources needed by ri_FastPathBatchFlush().
+ * Per-constraint, per-firing-cycle cache of resources needed by
+ * ri_FastPathBatchFlush().
*
- * One entry per constraint, keyed by pg_constraint OID. Created lazily
- * by ri_FastPathGetEntry() on first use within a trigger-firing batch
- * and torn down by ri_FastPathTeardown() at batch end.
+ * Created lazily by ri_FastPathGetEntry() on first use within a
+ * trigger-firing batch and torn down by ri_FastPathTeardown() at batch end.
*
* FK tuples are buffered in batch[] across trigger invocations and
* flushed when the buffer fills or the batch ends.
@@ -251,7 +265,7 @@ typedef struct RI_CompareHashEntry
*/
typedef struct RI_FastPathEntry
{
- Oid conoid; /* hash key: pg_constraint OID */
+ RI_FastPathKey key; /* hash key */
Oid fk_relid; /* for ri_FastPathEndBatch() */
Relation pk_rel;
Relation idx_rel;
@@ -284,7 +298,6 @@ static HTAB *ri_compare_cache = NULL;
static dclist_head ri_constraint_cache_valid_list;
static HTAB *ri_fastpath_cache = NULL;
-static bool ri_fastpath_callback_registered = false;
static bool ri_fastpath_flushing = false;
/*
@@ -382,7 +395,7 @@ pg_noreturn static void ri_ReportViolation(const RI_ConstraintInfo *riinfo,
static RI_FastPathEntry *ri_FastPathGetEntry(const RI_ConstraintInfo *riinfo,
Relation fk_rel);
static void ri_FastPathEndBatch(void *arg);
-static void ri_FastPathTeardown(void);
+static void ri_FastPathTeardown(int depth);
/*
@@ -4302,6 +4315,7 @@ ri_FastPathEndBatch(void *arg)
{
HASH_SEQ_STATUS status;
RI_FastPathEntry *entry;
+ int my_depth = (int) (intptr_t) arg;
if (ri_fastpath_cache == NULL)
return;
@@ -4326,10 +4340,13 @@ ri_FastPathEndBatch(void *arg)
hash_seq_init(&status, ri_fastpath_cache);
while ((entry = hash_seq_search(&status)) != NULL)
{
- if (entry->batch_count > 0)
+ /* Flush only entries created in the cycle now ending. */
+ if (entry->key.query_depth == my_depth && entry->batch_count > 0)
{
Relation fk_rel = table_open(entry->fk_relid, AccessShareLock);
- RI_ConstraintInfo *riinfo = ri_LoadConstraintInfo(entry->conoid);
+ RI_ConstraintInfo *riinfo;
+
+ riinfo = ri_LoadConstraintInfo(entry->key.conoid);
ri_FastPathBatchFlush(entry, fk_rel, riinfo);
table_close(fk_rel, NoLock);
@@ -4342,17 +4359,26 @@ ri_FastPathEndBatch(void *arg)
}
PG_END_TRY();
- ri_FastPathTeardown();
+ /*
+ * Release this cycle's entries and remove them from the cache; leave
+ * outer cycles' entries for their own callbacks. Destroy the cache once
+ * empty.
+ */
+ ri_FastPathTeardown(my_depth);
}
/*
* ri_FastPathTeardown
- * Tear down all cached fast-path state.
+ * Release and remove the cached entries of one firing cycle, and drop
+ * the cache once it holds no more entries.
*
- * Called from ri_FastPathEndBatch() after flushing any remaining rows.
+ * Called from ri_FastPathEndBatch() with the depth of the cycle that is
+ * ending: it releases only that cycle's entries, leaving an outer cycle's
+ * still-live entries for their own callbacks. The cache (and its static
+ * pointer) go away once the last entry is removed.
*/
static void
-ri_FastPathTeardown(void)
+ri_FastPathTeardown(int depth)
{
HASH_SEQ_STATUS status;
RI_FastPathEntry *entry;
@@ -4363,6 +4389,8 @@ ri_FastPathTeardown(void)
hash_seq_init(&status, ri_fastpath_cache);
while ((entry = hash_seq_search(&status)) != NULL)
{
+ if (entry->key.query_depth != depth)
+ continue;
if (entry->idx_rel)
index_close(entry->idx_rel, NoLock);
if (entry->pk_rel)
@@ -4373,11 +4401,15 @@ ri_FastPathTeardown(void)
ExecDropSingleTupleTableSlot(entry->fk_slot);
if (entry->flush_cxt)
MemoryContextDelete(entry->flush_cxt);
+ hash_search(ri_fastpath_cache, &entry->key, HASH_REMOVE, NULL);
}
- hash_destroy(ri_fastpath_cache);
- ri_fastpath_cache = NULL;
- ri_fastpath_callback_registered = false;
+ if (hash_get_num_entries(ri_fastpath_cache) == 0)
+ {
+ hash_destroy(ri_fastpath_cache);
+ ri_fastpath_cache = NULL;
+ ri_fastpath_flushing = false;
+ }
}
/*
@@ -4423,7 +4455,6 @@ AtEOXact_RI(bool isCommit)
* memory-context reset; here we only drop the references to it.
*/
ri_fastpath_cache = NULL;
- ri_fastpath_callback_registered = false;
/*
* Also clear the in-flush flag. ri_FastPathEndBatch() already clears it
@@ -4462,15 +4493,20 @@ AtEOXact_RI(bool isCommit)
static RI_FastPathEntry *
ri_FastPathGetEntry(const RI_ConstraintInfo *riinfo, Relation fk_rel)
{
+ RI_FastPathKey key;
RI_FastPathEntry *entry;
bool found;
+ int cur_depth = AfterTriggerCurrentQueryDepth();
+
+ key.conoid = riinfo->constraint_id;
+ key.query_depth = cur_depth;
/* Create hash table on first use in this batch */
if (ri_fastpath_cache == NULL)
{
HASHCTL ctl;
- ctl.keysize = sizeof(Oid);
+ ctl.keysize = sizeof(RI_FastPathKey);
ctl.entrysize = sizeof(RI_FastPathEntry);
ctl.hcxt = TopTransactionContext;
ri_fastpath_cache = hash_create("RI fast-path cache",
@@ -4479,7 +4515,7 @@ ri_FastPathGetEntry(const RI_ConstraintInfo *riinfo, Relation fk_rel)
HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
}
- entry = hash_search(ri_fastpath_cache, &riinfo->constraint_id,
+ entry = hash_search(ri_fastpath_cache, &key,
HASH_ENTER, &found);
if (!found)
@@ -4536,11 +4572,35 @@ ri_FastPathGetEntry(const RI_ConstraintInfo *riinfo, Relation fk_rel)
ALLOCSET_SMALL_SIZES);
MemoryContextSwitchTo(oldcxt);
- /* Ensure cleanup at end of this trigger-firing batch */
- if (!ri_fastpath_callback_registered)
+ /*
+ * Register an end-of-batch callback once per firing cycle, passing
+ * the query depth so the callback flushes only entries belonging to
+ * that cycle.
+ */
{
- RegisterAfterTriggerBatchCallback(ri_FastPathEndBatch, NULL);
- ri_fastpath_callback_registered = true;
+ bool depth_registered = false;
+ HASH_SEQ_STATUS reg_status;
+ RI_FastPathEntry *other;
+
+ /*
+ * An existing entry at this depth means its callback is already
+ * registered. Ignore the just-created entry, which is already in
+ * the hash.
+ */
+ hash_seq_init(®_status, ri_fastpath_cache);
+ while ((other = hash_seq_search(®_status)) != NULL)
+ {
+ if (other != entry && other->key.query_depth == cur_depth)
+ {
+ depth_registered = true;
+ hash_seq_term(®_status);
+ break;
+ }
+ }
+
+ if (!depth_registered)
+ RegisterAfterTriggerBatchCallback(ri_FastPathEndBatch,
+ (void *) (intptr_t) cur_depth);
}
entry->flushing = false;
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index 0c3d485abf4..1f268f87957 100644
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -309,6 +309,7 @@ typedef void (*AfterTriggerBatchCallback) (void *arg);
extern void RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback,
void *arg);
extern bool AfterTriggerIsActive(void);
+extern int AfterTriggerCurrentQueryDepth(void);
extern void AtEOXact_RI(bool isCommit);
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 01343c58e11..126c251bbda 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -3951,3 +3951,95 @@ ROLLBACK;
RESET ROLE;
DROP TABLE fpav_fk, fpav_pk, fpav_cv_fk, fpav_cv_pk;
DROP ROLE regress_fpav_role;
+-- An AFTER trigger runs a query of its own, and that query inserts into a
+-- second table with a fast-path foreign key. The entry the nested INSERT
+-- creates belongs to the cursor's portal, which is gone by the time the
+-- entry is torn down at the end of the outer statement. Every key stored
+-- below is present in its referenced table, so the INSERT must just succeed.
+CREATE TABLE fp_customer (id int PRIMARY KEY);
+INSERT INTO fp_customer VALUES (1);
+CREATE TABLE fp_product (id int PRIMARY KEY);
+INSERT INTO fp_product SELECT generate_series(1, 4);
+CREATE TABLE fp_kit_component (kit_product_id int, component_product_id int);
+INSERT INTO fp_kit_component VALUES (1, 2), (1, 3), (1, 4);
+CREATE TABLE fp_order (id int, customer_id int REFERENCES fp_customer,
+ product_id int);
+CREATE TABLE fp_order_item (order_id int, product_id int
+ REFERENCES fp_product);
+CREATE FUNCTION fp_add_order_item(order_id int, product_id int) RETURNS int
+ LANGUAGE plpgsql AS $$
+BEGIN
+ INSERT INTO fp_order_item VALUES (order_id, product_id);
+ RETURN product_id;
+END$$;
+CREATE FUNCTION fp_expand_kit() RETURNS trigger LANGUAGE plpgsql AS $$
+DECLARE
+ component_id int;
+ ncomponents int := 0;
+BEGIN
+ FOR component_id IN
+ SELECT fp_add_order_item(NEW.id, component_product_id)
+ FROM fp_kit_component WHERE kit_product_id = NEW.product_id
+ LOOP
+ ncomponents := ncomponents + 1;
+ END LOOP;
+ RAISE NOTICE 'order % expanded into % order items', NEW.id, ncomponents;
+ RETURN NULL;
+END$$;
+CREATE TRIGGER fp_expand_kit_trg AFTER INSERT ON fp_order
+ FOR EACH ROW EXECUTE FUNCTION fp_expand_kit();
+INSERT INTO fp_order VALUES (1, 1, 1);
+NOTICE: order 1 expanded into 3 order items
+SELECT count(*) FROM fp_order_item;
+ count
+-------
+ 3
+(1 row)
+
+DROP TABLE fp_order, fp_order_item, fp_kit_component, fp_product, fp_customer;
+DROP FUNCTION fp_expand_kit(), fp_add_order_item(int, int);
+-- Nested firing of the same constraint must use an entry for its own query
+-- depth. The RAISE is reached if the nested violation remains buffered for
+-- the outer cycle's callback.
+CREATE TABLE fp_depth_pk (id int PRIMARY KEY);
+INSERT INTO fp_depth_pk VALUES (1);
+CREATE TABLE fp_depth_fk (a int REFERENCES fp_depth_pk);
+CREATE FUNCTION fp_depth_reentry() RETURNS trigger LANGUAGE plpgsql AS $$
+BEGIN
+ IF NEW.a = 1 THEN
+ INSERT INTO fp_depth_fk VALUES (999);
+ RAISE EXCEPTION 'nested FK check was not flushed';
+ END IF;
+ RETURN NEW;
+END$$;
+-- Sort after the RI trigger, so the outer row has already been batched.
+CREATE TRIGGER zz_fp_depth_reentry AFTER INSERT ON fp_depth_fk
+ FOR EACH ROW EXECUTE FUNCTION fp_depth_reentry();
+INSERT INTO fp_depth_fk VALUES (1);
+ERROR: insert or update on table "fp_depth_fk" violates foreign key constraint "fp_depth_fk_a_fkey"
+DETAIL: Key (a)=(999) is not present in table "fp_depth_pk".
+CONTEXT: SQL statement "INSERT INTO fp_depth_fk VALUES (999)"
+PL/pgSQL function fp_depth_reentry() line 4 at SQL statement
+SELECT * FROM fp_depth_fk;
+ a
+---
+(0 rows)
+
+DROP TABLE fp_depth_fk, fp_depth_pk;
+DROP FUNCTION fp_depth_reentry();
+-- Deferred FK check fires at commit (query depth -1); its batch must still get
+-- a callback registered and flushed.
+CREATE TABLE fp_deferred_pk (id int PRIMARY KEY);
+CREATE TABLE fp_deferred_fk (a int REFERENCES fp_deferred_pk (id)
+ DEFERRABLE INITIALLY DEFERRED);
+BEGIN;
+INSERT INTO fp_deferred_fk VALUES (1);
+INSERT INTO fp_deferred_pk VALUES (1);
+COMMIT;
+SELECT count(*) AS deferred_rows FROM fp_deferred_fk; -- 1, check passed at commit
+ deferred_rows
+---------------
+ 1
+(1 row)
+
+DROP TABLE fp_deferred_fk, fp_deferred_pk;
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 987cea61ba2..7319de6a280 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -2897,3 +2897,81 @@ ROLLBACK;
RESET ROLE;
DROP TABLE fpav_fk, fpav_pk, fpav_cv_fk, fpav_cv_pk;
DROP ROLE regress_fpav_role;
+
+-- An AFTER trigger runs a query of its own, and that query inserts into a
+-- second table with a fast-path foreign key. The entry the nested INSERT
+-- creates belongs to the cursor's portal, which is gone by the time the
+-- entry is torn down at the end of the outer statement. Every key stored
+-- below is present in its referenced table, so the INSERT must just succeed.
+CREATE TABLE fp_customer (id int PRIMARY KEY);
+INSERT INTO fp_customer VALUES (1);
+CREATE TABLE fp_product (id int PRIMARY KEY);
+INSERT INTO fp_product SELECT generate_series(1, 4);
+CREATE TABLE fp_kit_component (kit_product_id int, component_product_id int);
+INSERT INTO fp_kit_component VALUES (1, 2), (1, 3), (1, 4);
+CREATE TABLE fp_order (id int, customer_id int REFERENCES fp_customer,
+ product_id int);
+CREATE TABLE fp_order_item (order_id int, product_id int
+ REFERENCES fp_product);
+CREATE FUNCTION fp_add_order_item(order_id int, product_id int) RETURNS int
+ LANGUAGE plpgsql AS $$
+BEGIN
+ INSERT INTO fp_order_item VALUES (order_id, product_id);
+ RETURN product_id;
+END$$;
+CREATE FUNCTION fp_expand_kit() RETURNS trigger LANGUAGE plpgsql AS $$
+DECLARE
+ component_id int;
+ ncomponents int := 0;
+BEGIN
+ FOR component_id IN
+ SELECT fp_add_order_item(NEW.id, component_product_id)
+ FROM fp_kit_component WHERE kit_product_id = NEW.product_id
+ LOOP
+ ncomponents := ncomponents + 1;
+ END LOOP;
+ RAISE NOTICE 'order % expanded into % order items', NEW.id, ncomponents;
+ RETURN NULL;
+END$$;
+CREATE TRIGGER fp_expand_kit_trg AFTER INSERT ON fp_order
+ FOR EACH ROW EXECUTE FUNCTION fp_expand_kit();
+INSERT INTO fp_order VALUES (1, 1, 1);
+SELECT count(*) FROM fp_order_item;
+DROP TABLE fp_order, fp_order_item, fp_kit_component, fp_product, fp_customer;
+DROP FUNCTION fp_expand_kit(), fp_add_order_item(int, int);
+
+-- Nested firing of the same constraint must use an entry for its own query
+-- depth. The RAISE is reached if the nested violation remains buffered for
+-- the outer cycle's callback.
+CREATE TABLE fp_depth_pk (id int PRIMARY KEY);
+INSERT INTO fp_depth_pk VALUES (1);
+CREATE TABLE fp_depth_fk (a int REFERENCES fp_depth_pk);
+CREATE FUNCTION fp_depth_reentry() RETURNS trigger LANGUAGE plpgsql AS $$
+BEGIN
+ IF NEW.a = 1 THEN
+ INSERT INTO fp_depth_fk VALUES (999);
+ RAISE EXCEPTION 'nested FK check was not flushed';
+ END IF;
+ RETURN NEW;
+END$$;
+-- Sort after the RI trigger, so the outer row has already been batched.
+CREATE TRIGGER zz_fp_depth_reentry AFTER INSERT ON fp_depth_fk
+ FOR EACH ROW EXECUTE FUNCTION fp_depth_reentry();
+
+INSERT INTO fp_depth_fk VALUES (1);
+SELECT * FROM fp_depth_fk;
+
+DROP TABLE fp_depth_fk, fp_depth_pk;
+DROP FUNCTION fp_depth_reentry();
+
+-- Deferred FK check fires at commit (query depth -1); its batch must still get
+-- a callback registered and flushed.
+CREATE TABLE fp_deferred_pk (id int PRIMARY KEY);
+CREATE TABLE fp_deferred_fk (a int REFERENCES fp_deferred_pk (id)
+ DEFERRABLE INITIALLY DEFERRED);
+BEGIN;
+INSERT INTO fp_deferred_fk VALUES (1);
+INSERT INTO fp_deferred_pk VALUES (1);
+COMMIT;
+SELECT count(*) AS deferred_rows FROM fp_deferred_fk; -- 1, check passed at commit
+DROP TABLE fp_deferred_fk, fp_deferred_pk;
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index f416ec0f475..23d4e6c0651 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2521,6 +2521,7 @@ RI_CompareHashEntry
RI_CompareKey
RI_ConstraintInfo
RI_FastPathEntry
+RI_FastPathKey
RI_QueryHashEntry
RI_QueryKey
RTEKind
[parent: 28f9c5b5777b]