From 8c55f488a256af90cbfa014eecd12347af4e7c1e Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Thu, 27 Aug 2026 21:14:52 +0900 Subject: [PATCH v1] Fix RI fast-path batching in a nested SET CONSTRAINTS cycle SET CONSTRAINTS ... IMMEDIATE makes the named constraints immediate and fires the events already queued for them. It does that without opening a query level, so its firing cycle runs at the same after-trigger query depth as the cycle containing it. Commit 6fc2a48 used that query depth as part of the RI fast-path cache key and to decide whether the cycle had already registered an end-of-batch callback, so a SET CONSTRAINTS issued from an AFTER trigger found the enclosing cycle's entry and registered no callback of its own. Callbacks are also registered into the current query level's list when query_depth >= 0, while AfterTriggerSetState() fired afterTriggers.batch_callbacks, the list used only outside a query level. The nested cycle's batch was therefore never flushed. SET CONSTRAINTS reported success for a constraint it had not checked, and the entry outlived the subtransaction whose resource owner had opened its relations. When that subtransaction committed, as it does when a PL/pgSQL EXCEPTION block's body completes, those relation and tuple-descriptor references were force-released, and the enclosing cycle's callback, which selected entries by query depth, then flushed through them. Key fast-path entries by firing depth, which does bracket a firing cycle, and replace AfterTriggerCurrentQueryDepth() with AfterTriggerCurrentFiringDepth(). The former had no remaining callers; it was added by 6fc2a48 for this purpose and nothing else used it. Give the nested cycle its own callback list in AfterTriggerSetState(), restored in PG_FINALLY because a PL/pgSQL EXCEPTION block can catch an error from the cycle and continue. Also remove, rather than reassign to the parent, a fast-path entry found by AtEOSubXact_RI() on the commit path; subtransaction commit force-releases those references rather than transferring them, so such an entry is stale on commit as well as on abort. Reported-by: Ayush Tiwari (offlist) Discussion: https://postgr.es/m/CA+HiwqE2MRym5fGfxz58AdzxEzXyDuE4SEpM3eoH3HCii=Wh=A@mail.gmail.com Backpatch-through: 19 --- src/backend/commands/trigger.c | 138 ++++++++++++++------- src/backend/utils/adt/ri_triggers.c | 76 +++++++----- src/include/commands/trigger.h | 2 +- src/test/regress/expected/foreign_key.out | 141 ++++++++++++++++++++++ src/test/regress/sql/foreign_key.sql | 113 +++++++++++++++++ 5 files changed, 393 insertions(+), 77 deletions(-) diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 79ffd2cada6..3d4d851cbe4 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -4024,6 +4024,7 @@ static SetConstraintState SetConstraintStateAddItem(SetConstraintState state, static void cancel_prior_stmt_triggers(Oid relid, CmdType cmdType, int tgevent); static void FireAfterTriggerBatchCallbacks(List *callbacks); +static List **afterTriggerBatchCallbackList(int query_depth); /* * Get the FDW tuplestore for the current trigger query level, creating it @@ -6157,44 +6158,77 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt) { AfterTriggerEventList *events = &afterTriggers.events; bool snapshot_set = false; + int cb_depth = afterTriggers.query_depth; + List *save_batch_callbacks; + + /* + * This starts a trigger-firing cycle nested inside whatever cycle is + * already running, but unlike AfterTriggerEndQuery() it opens no query + * level. Batch callbacks registered by the triggers fired below would + * therefore land in the same list as the enclosing cycle's, which + * fires that list at its own end. Swap in an empty list so this cycle + * fires and frees only its own callbacks: firing the enclosing cycle's + * here would flush its batch early, under a resource owner that can go + * away before the batch is used again, and leave it with nothing to + * flush when it does end. + * + * The restore must happen even if a trigger throws, because the error + * may be caught by a PL/pgSQL EXCEPTION block and the enclosing cycle + * then carries on and must still find its callbacks. + */ + save_batch_callbacks = *afterTriggerBatchCallbackList(cb_depth); + *afterTriggerBatchCallbackList(cb_depth) = NIL; afterTriggers.firing_depth++; - while (afterTriggerMarkEvents(events, NULL, true)) - { - CommandId firing_id = afterTriggers.firing_counter++; - /* - * Make sure a snapshot has been established in case trigger - * functions need one. Note that we avoid setting a snapshot if - * we don't find at least one trigger that has to be fired now. - * This is so that BEGIN; SET CONSTRAINTS ...; SET TRANSACTION - * ISOLATION LEVEL SERIALIZABLE; ... works properly. (If we are - * at the start of a transaction it's not possible for any trigger - * events to be queued yet.) - */ - if (!snapshot_set) + PG_TRY(); + { + while (afterTriggerMarkEvents(events, NULL, true)) { - PushActiveSnapshot(GetTransactionSnapshot()); - snapshot_set = true; + CommandId firing_id = afterTriggers.firing_counter++; + + /* + * Make sure a snapshot has been established in case trigger + * functions need one. Note that we avoid setting a snapshot if + * we don't find at least one trigger that has to be fired now. + * This is so that BEGIN; SET CONSTRAINTS ...; SET TRANSACTION + * ISOLATION LEVEL SERIALIZABLE; ... works properly. (If we are + * at the start of a transaction it's not possible for any trigger + * events to be queued yet.) + */ + if (!snapshot_set) + { + PushActiveSnapshot(GetTransactionSnapshot()); + snapshot_set = true; + } + + /* + * We can delete fired events if we are at top transaction level, + * but we'd better not if inside a subtransaction, since the + * subtransaction could later get rolled back. + */ + if (afterTriggerInvokeEvents(events, firing_id, NULL, + !IsSubTransaction())) + break; /* all fired */ } /* - * We can delete fired events if we are at top transaction level, - * but we'd better not if inside a subtransaction, since the - * subtransaction could later get rolled back. + * Flush any fast-path batches accumulated by the triggers just + * fired. SET CONSTRAINTS ... IMMEDIATE means "check these now", so + * nothing may be left buffered here: a pending batch would report + * success for a constraint that was never actually checked. */ - if (afterTriggerInvokeEvents(events, firing_id, NULL, - !IsSubTransaction())) - break; /* all fired */ + FireAfterTriggerBatchCallbacks(*afterTriggerBatchCallbackList(cb_depth)); } + PG_FINALLY(); + { + List **cb_list = afterTriggerBatchCallbackList(cb_depth); - /* - * Flush any fast-path batches accumulated by the triggers just fired. - */ - FireAfterTriggerBatchCallbacks(afterTriggers.batch_callbacks); - afterTriggers.firing_depth--; - list_free_deep(afterTriggers.batch_callbacks); - afterTriggers.batch_callbacks = NIL; + list_free_deep(*cb_list); + *cb_list = save_batch_callbacks; + afterTriggers.firing_depth--; + } + PG_END_TRY(); if (snapshot_set) PopActiveSnapshot(); @@ -6910,6 +6944,7 @@ RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback, { AfterTriggerCallbackItem *item; MemoryContext oldcxt; + List **cb_list; /* * Allocate in TopTransactionContext so the item survives for the duration @@ -6924,19 +6959,34 @@ RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback, item = palloc_object(AfterTriggerCallbackItem); item->callback = callback; item->arg = arg; - 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); + cb_list = afterTriggerBatchCallbackList(afterTriggers.query_depth); + *cb_list = lappend(*cb_list, item); MemoryContextSwitchTo(oldcxt); } +/* + * afterTriggerBatchCallbackList + * Return the list batch callbacks registered at the given query depth + * belong to. + * + * Callbacks registered inside a query level are owned by that level and fired + * by AfterTriggerEndQuery(); those registered outside any query level (a + * deferred-trigger firing at transaction end) are owned by afterTriggers + * itself. + * + * Callers must re-derive the pointer rather than hold it across trigger + * firing: query_stack is repalloc'd when a nested AfterTriggerBeginQuery() + * grows it. + */ +static List ** +afterTriggerBatchCallbackList(int query_depth) +{ + if (query_depth >= 0) + return &afterTriggers.query_stack[query_depth].batch_callbacks; + + return &afterTriggers.batch_callbacks; +} + /* * FireAfterTriggerBatchCallbacks * Invoke all callbacks in the given list. @@ -6976,15 +7026,17 @@ AfterTriggerIsActive(void) } /* - * AfterTriggerCurrentQueryDepth - * Return the current after-trigger query nesting depth. + * AfterTriggerCurrentFiringDepth + * Return the current trigger-firing cycle 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. + * acts only on its own entries. The query depth cannot serve here: + * AfterTriggerSetState() fires queued events without opening a query level, + * so its cycle runs at the same query depth as the cycle containing it. */ int -AfterTriggerCurrentQueryDepth(void) +AfterTriggerCurrentFiringDepth(void) { - return afterTriggers.query_depth; + return afterTriggers.firing_depth; } diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index d4545618634..022e674d5d9 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -238,11 +238,15 @@ typedef struct RI_CompareHashEntry * 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. + * + * The firing depth, not the query depth, identifies the cycle: SET CONSTRAINTS + * ... IMMEDIATE fires queued events without opening a query level, so its cycle + * runs at the same query depth as the cycle that contains it. */ typedef struct RI_FastPathKey { Oid conoid; /* pg_constraint OID */ - int query_depth; /* after-trigger query depth */ + int firing_depth; /* after-trigger firing cycle depth */ } RI_FastPathKey; /* @@ -403,7 +407,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(int depth); +static void ri_FastPathTeardown(int firing_depth); /* @@ -4325,7 +4329,7 @@ ri_FastPathEndBatch(void *arg) { HASH_SEQ_STATUS status; RI_FastPathEntry *entry; - int my_depth = (int) (intptr_t) arg; + int my_firing_depth = (int) (intptr_t) arg; if (ri_fastpath_cache == NULL) return; @@ -4351,7 +4355,7 @@ ri_FastPathEndBatch(void *arg) while ((entry = hash_seq_search(&status)) != NULL) { /* Flush only entries created in the cycle now ending. */ - if (entry->key.query_depth == my_depth && entry->batch_count > 0) + if (entry->key.firing_depth == my_firing_depth && entry->batch_count > 0) { Relation fk_rel = table_open(entry->fk_relid, AccessShareLock); RI_ConstraintInfo *riinfo; @@ -4374,7 +4378,7 @@ ri_FastPathEndBatch(void *arg) * outer cycles' entries for their own callbacks. Destroy the cache once * empty. */ - ri_FastPathTeardown(my_depth); + ri_FastPathTeardown(my_firing_depth); } /* @@ -4382,13 +4386,13 @@ ri_FastPathEndBatch(void *arg) * Release and remove the cached entries of one firing cycle, and drop * the cache once it holds no more entries. * - * Called from ri_FastPathEndBatch() with the depth of the cycle that is + * Called from ri_FastPathEndBatch() with the firing 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(int depth) +ri_FastPathTeardown(int firing_depth) { HASH_SEQ_STATUS status; RI_FastPathEntry *entry; @@ -4399,7 +4403,7 @@ ri_FastPathTeardown(int depth) hash_seq_init(&status, ri_fastpath_cache); while ((entry = hash_seq_search(&status)) != NULL) { - if (entry->key.query_depth != depth) + if (entry->key.firing_depth != firing_depth) continue; if (entry->idx_rel) index_close(entry->idx_rel, NoLock); @@ -4510,6 +4514,11 @@ AtEOXact_RI(bool isCommit) * entries so a later firing cycle cannot reuse them. Entries belonging to * outer subtransactions remain valid and are preserved. * + * parentSubid is unused: an entry found here is stale on the commit path too, + * since subtransaction commit force-releases the relation and tuple-descriptor + * references rather than transferring them to the parent. The slots survive, + * but point at tuple descriptors that no longer do. + * * The remaining slot storage and per-entry flush contexts are reclaimed when * TopTransactionContext is reset at top-level transaction end. */ @@ -4531,20 +4540,21 @@ AtEOSubXact_RI(bool isCommit, SubTransactionId mySubid, if (entry->subid != mySubid) continue; - if (isCommit) - { - /* - * A committing subxact's entry should already have been flushed - * and torn down at its statement's end (ri_FastPathEndBatch()), - * so we don't expect to find one here. If we do, reassign it to - * the parent so it's still cleaned up rather than left under a - * subxact id that no longer exists. - */ - Assert(false); - entry->subid = parentSubid; - } - else - hash_search(ri_fastpath_cache, &entry->key, HASH_REMOVE, NULL); + /* + * A committing subxact's entry should already have been flushed and + * torn down at the end of the firing cycle that created it + * (ri_FastPathEndBatch()), so we don't expect to find one here. + */ + Assert(!isCommit); + + /* + * Remove it in either case. Subtransaction commit does not hand this + * entry's relation and tuple-descriptor references to the parent's + * resource owner; it force-releases them just as abort does. Keeping + * the entry -- reassigned to the parent or otherwise -- would leave a + * later firing cycle free to flush through released references. + */ + hash_search(ri_fastpath_cache, &entry->key, HASH_REMOVE, NULL); } /* If that emptied the cache, drop it so the next batch starts clean. */ @@ -4573,10 +4583,10 @@ ri_FastPathGetEntry(const RI_ConstraintInfo *riinfo, Relation fk_rel) RI_FastPathKey key; RI_FastPathEntry *entry; bool found; - int cur_depth = AfterTriggerCurrentQueryDepth(); + int cur_firing_depth = AfterTriggerCurrentFiringDepth(); key.conoid = riinfo->constraint_id; - key.query_depth = cur_depth; + key.firing_depth = cur_firing_depth; /* Create hash table on first use in this batch */ if (ri_fastpath_cache == NULL) @@ -4651,33 +4661,33 @@ ri_FastPathGetEntry(const RI_ConstraintInfo *riinfo, Relation fk_rel) /* * Register an end-of-batch callback once per firing cycle, passing - * the query depth so the callback flushes only entries belonging to + * the firing depth so the callback flushes only entries belonging to * that cycle. */ { - bool depth_registered = false; + bool firing_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. + * An existing entry at this firing 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) + if (other != entry && other->key.firing_depth == cur_firing_depth) { - depth_registered = true; + firing_depth_registered = true; hash_seq_term(®_status); break; } } - if (!depth_registered) + if (!firing_depth_registered) RegisterAfterTriggerBatchCallback(ri_FastPathEndBatch, - (void *) (intptr_t) cur_depth); + (void *) (intptr_t) cur_firing_depth); } entry->flushing = false; diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index fecdb785f35..c9905908328 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -309,7 +309,7 @@ typedef void (*AfterTriggerBatchCallback) (void *arg); extern void RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback, void *arg); extern bool AfterTriggerIsActive(void); -extern int AfterTriggerCurrentQueryDepth(void); +extern int AfterTriggerCurrentFiringDepth(void); extern void AtEOXact_RI(bool isCommit); extern void AtEOSubXact_RI(bool isCommit, SubTransactionId mySubid, diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index ac044eb40fa..e0846465960 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -4021,6 +4021,147 @@ SELECT * FROM fp_same_fk; DROP TRIGGER fp_reentry_same_constraint_trg ON fp_same_fk; DROP FUNCTION fp_reentry_same_constraint(); DROP TABLE fp_same_fk, fp_same_pk; +-- SET CONSTRAINTS fires queued events without opening a query level, so a +-- SET CONSTRAINTS ... IMMEDIATE issued from an AFTER trigger runs a firing +-- cycle nested at the enclosing cycle's query depth. Each cycle must flush +-- its own fast-path checks before it ends, and must leave the other cycle's +-- checks alone. +CREATE TABLE fp_sc_pk (id int PRIMARY KEY); +INSERT INTO fp_sc_pk VALUES (1); +CREATE TABLE fp_sc_fk ( + a int REFERENCES fp_sc_pk (id), + b int CONSTRAINT fp_sc_deferred REFERENCES fp_sc_pk (id) + DEFERRABLE INITIALLY DEFERRED); +CREATE FUNCTION fp_sc_check_now() RETURNS trigger LANGUAGE plpgsql AS $$ +BEGIN + BEGIN + SET CONSTRAINTS fp_sc_deferred IMMEDIATE; + EXCEPTION WHEN foreign_key_violation THEN + RAISE NOTICE 'caught by SET CONSTRAINTS'; + END; + RETURN NEW; +END$$; +-- The trigger name must sort after the RI trigger for column a, so that a's +-- check is already buffered in the enclosing cycle's batch when this fires. +CREATE TRIGGER zz_fp_sc_check_now AFTER INSERT ON fp_sc_fk + FOR EACH ROW EXECUTE FUNCTION fp_sc_check_now(); +-- Deferred constraint violated: the nested SET CONSTRAINTS must raise, the +-- EXCEPTION block must catch it, and rolling the subtransaction back must +-- restore the deferred state so the violation surfaces again at commit. +BEGIN; +INSERT INTO fp_sc_fk VALUES (1, 999); +NOTICE: caught by SET CONSTRAINTS +COMMIT; +ERROR: insert or update on table "fp_sc_fk" violates foreign key constraint "fp_sc_deferred" +DETAIL: Key (b)=(999) is not present in table "fp_sc_pk". +SELECT count(*) FROM fp_sc_fk; + count +------- + 0 +(1 row) + +-- Nothing violated: the nested cycle flushes, its subtransaction commits, +-- and the enclosing cycle's own check still runs when it ends. +BEGIN; +INSERT INTO fp_sc_fk VALUES (1, 1); +COMMIT; +SELECT count(*) FROM fp_sc_fk; + count +------- + 1 +(1 row) + +-- A key the enclosing cycle is responsible for is missing: its check must +-- still run once the nested cycle has finished. +BEGIN; +INSERT INTO fp_sc_fk VALUES (999, 1); +ERROR: insert or update on table "fp_sc_fk" violates foreign key constraint "fp_sc_fk_a_fkey" +DETAIL: Key (a)=(999) is not present in table "fp_sc_pk". +COMMIT; +SELECT count(*) FROM fp_sc_fk; + count +------- + 1 +(1 row) + +DROP TRIGGER zz_fp_sc_check_now ON fp_sc_fk; +-- The same, from a trigger deferred to commit. That firing cycle runs +-- outside any query level, so its batch callbacks are owned by the +-- after-trigger state itself rather than by a query level. +CREATE CONSTRAINT TRIGGER zz_fp_sc_at_commit AFTER INSERT ON fp_sc_fk + DEFERRABLE INITIALLY DEFERRED + FOR EACH ROW EXECUTE FUNCTION fp_sc_check_now(); +BEGIN; +INSERT INTO fp_sc_fk VALUES (1, 1); +COMMIT; +SELECT count(*) FROM fp_sc_fk; + count +------- + 2 +(1 row) + +DROP TRIGGER zz_fp_sc_at_commit ON fp_sc_fk; +DROP FUNCTION fp_sc_check_now(); +DROP TABLE fp_sc_fk, fp_sc_pk; +-- SET CONSTRAINTS from a cast function invoked during a fast-path flush. +-- The nested cycle runs inside FireAfterTriggerBatchCallbacks(), at the +-- flushing cycle's query depth. The flush guard sends its FK checks down the +-- per-row path, so it creates no entries and fires an empty callback list; the +-- flush it interrupted must still complete. +CREATE TABLE fp_sc_cast_pk (id int PRIMARY KEY); +INSERT INTO fp_sc_cast_pk VALUES (1); +CREATE TABLE fp_sc_cast_defpk (id int PRIMARY KEY); +INSERT INTO fp_sc_cast_defpk VALUES (1); +CREATE TABLE fp_sc_cast_deffk (a int CONSTRAINT fp_sc_cast_def + REFERENCES fp_sc_cast_defpk (id) DEFERRABLE INITIALLY DEFERRED); +CREATE TYPE fp_sc_vch AS (v int); +CREATE FUNCTION fp_sc_vcast(fp_sc_vch) RETURNS int LANGUAGE plpgsql AS $$ +BEGIN + BEGIN + SET CONSTRAINTS fp_sc_cast_def IMMEDIATE; + EXCEPTION WHEN foreign_key_violation THEN + NULL; + END; + RETURN $1.v; +END$$; +CREATE CAST (fp_sc_vch AS int) WITH FUNCTION fp_sc_vcast(fp_sc_vch) AS IMPLICIT; +CREATE TABLE fp_sc_cast_fk (a fp_sc_vch REFERENCES fp_sc_cast_pk (id)); +-- Fill exactly one batch so the flush fires the cast. Nothing is violated, so +-- the cast's subtransaction commits and the constraint stays immediate: the +-- next violating row must be rejected right away rather than at commit. That +-- is what shows the cast's SET CONSTRAINTS actually took effect. +BEGIN; +INSERT INTO fp_sc_cast_deffk VALUES (1); +INSERT INTO fp_sc_cast_fk SELECT row(1)::fp_sc_vch FROM generate_series(1, 64); +SELECT count(*) FROM fp_sc_cast_fk; + count +------- + 64 +(1 row) + +INSERT INTO fp_sc_cast_deffk VALUES (999); +ERROR: insert or update on table "fp_sc_cast_deffk" violates foreign key constraint "fp_sc_cast_def" +DETAIL: Key (a)=(999) is not present in table "fp_sc_cast_defpk". +ROLLBACK; +-- Now the deferred row is already violating when the cast runs. Its +-- SET CONSTRAINTS raises, the cast's subtransaction rolls back, and that +-- restores the deferred state, so the violation surfaces at commit instead. +BEGIN; +INSERT INTO fp_sc_cast_deffk VALUES (999); +INSERT INTO fp_sc_cast_fk SELECT row(1)::fp_sc_vch FROM generate_series(1, 64); +COMMIT; +ERROR: insert or update on table "fp_sc_cast_deffk" violates foreign key constraint "fp_sc_cast_def" +DETAIL: Key (a)=(999) is not present in table "fp_sc_cast_defpk". +SELECT count(*) FROM fp_sc_cast_fk; + count +------- + 0 +(1 row) + +DROP TABLE fp_sc_cast_fk, fp_sc_cast_deffk, fp_sc_cast_pk, fp_sc_cast_defpk; +DROP CAST (fp_sc_vch AS int); +DROP FUNCTION fp_sc_vcast(fp_sc_vch); +DROP TYPE fp_sc_vch; -- 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 diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index a93e81b42bc..e38706b1124 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -2955,6 +2955,119 @@ DROP TRIGGER fp_reentry_same_constraint_trg ON fp_same_fk; DROP FUNCTION fp_reentry_same_constraint(); DROP TABLE fp_same_fk, fp_same_pk; +-- SET CONSTRAINTS fires queued events without opening a query level, so a +-- SET CONSTRAINTS ... IMMEDIATE issued from an AFTER trigger runs a firing +-- cycle nested at the enclosing cycle's query depth. Each cycle must flush +-- its own fast-path checks before it ends, and must leave the other cycle's +-- checks alone. +CREATE TABLE fp_sc_pk (id int PRIMARY KEY); +INSERT INTO fp_sc_pk VALUES (1); +CREATE TABLE fp_sc_fk ( + a int REFERENCES fp_sc_pk (id), + b int CONSTRAINT fp_sc_deferred REFERENCES fp_sc_pk (id) + DEFERRABLE INITIALLY DEFERRED); +CREATE FUNCTION fp_sc_check_now() RETURNS trigger LANGUAGE plpgsql AS $$ +BEGIN + BEGIN + SET CONSTRAINTS fp_sc_deferred IMMEDIATE; + EXCEPTION WHEN foreign_key_violation THEN + RAISE NOTICE 'caught by SET CONSTRAINTS'; + END; + RETURN NEW; +END$$; +-- The trigger name must sort after the RI trigger for column a, so that a's +-- check is already buffered in the enclosing cycle's batch when this fires. +CREATE TRIGGER zz_fp_sc_check_now AFTER INSERT ON fp_sc_fk + FOR EACH ROW EXECUTE FUNCTION fp_sc_check_now(); + +-- Deferred constraint violated: the nested SET CONSTRAINTS must raise, the +-- EXCEPTION block must catch it, and rolling the subtransaction back must +-- restore the deferred state so the violation surfaces again at commit. +BEGIN; +INSERT INTO fp_sc_fk VALUES (1, 999); +COMMIT; +SELECT count(*) FROM fp_sc_fk; + +-- Nothing violated: the nested cycle flushes, its subtransaction commits, +-- and the enclosing cycle's own check still runs when it ends. +BEGIN; +INSERT INTO fp_sc_fk VALUES (1, 1); +COMMIT; +SELECT count(*) FROM fp_sc_fk; + +-- A key the enclosing cycle is responsible for is missing: its check must +-- still run once the nested cycle has finished. +BEGIN; +INSERT INTO fp_sc_fk VALUES (999, 1); +COMMIT; +SELECT count(*) FROM fp_sc_fk; + +DROP TRIGGER zz_fp_sc_check_now ON fp_sc_fk; + +-- The same, from a trigger deferred to commit. That firing cycle runs +-- outside any query level, so its batch callbacks are owned by the +-- after-trigger state itself rather than by a query level. +CREATE CONSTRAINT TRIGGER zz_fp_sc_at_commit AFTER INSERT ON fp_sc_fk + DEFERRABLE INITIALLY DEFERRED + FOR EACH ROW EXECUTE FUNCTION fp_sc_check_now(); +BEGIN; +INSERT INTO fp_sc_fk VALUES (1, 1); +COMMIT; +SELECT count(*) FROM fp_sc_fk; + +DROP TRIGGER zz_fp_sc_at_commit ON fp_sc_fk; +DROP FUNCTION fp_sc_check_now(); +DROP TABLE fp_sc_fk, fp_sc_pk; + +-- SET CONSTRAINTS from a cast function invoked during a fast-path flush. +-- The nested cycle runs inside FireAfterTriggerBatchCallbacks(), at the +-- flushing cycle's query depth. The flush guard sends its FK checks down the +-- per-row path, so it creates no entries and fires an empty callback list; the +-- flush it interrupted must still complete. +CREATE TABLE fp_sc_cast_pk (id int PRIMARY KEY); +INSERT INTO fp_sc_cast_pk VALUES (1); +CREATE TABLE fp_sc_cast_defpk (id int PRIMARY KEY); +INSERT INTO fp_sc_cast_defpk VALUES (1); +CREATE TABLE fp_sc_cast_deffk (a int CONSTRAINT fp_sc_cast_def + REFERENCES fp_sc_cast_defpk (id) DEFERRABLE INITIALLY DEFERRED); +CREATE TYPE fp_sc_vch AS (v int); +CREATE FUNCTION fp_sc_vcast(fp_sc_vch) RETURNS int LANGUAGE plpgsql AS $$ +BEGIN + BEGIN + SET CONSTRAINTS fp_sc_cast_def IMMEDIATE; + EXCEPTION WHEN foreign_key_violation THEN + NULL; + END; + RETURN $1.v; +END$$; +CREATE CAST (fp_sc_vch AS int) WITH FUNCTION fp_sc_vcast(fp_sc_vch) AS IMPLICIT; +CREATE TABLE fp_sc_cast_fk (a fp_sc_vch REFERENCES fp_sc_cast_pk (id)); + +-- Fill exactly one batch so the flush fires the cast. Nothing is violated, so +-- the cast's subtransaction commits and the constraint stays immediate: the +-- next violating row must be rejected right away rather than at commit. That +-- is what shows the cast's SET CONSTRAINTS actually took effect. +BEGIN; +INSERT INTO fp_sc_cast_deffk VALUES (1); +INSERT INTO fp_sc_cast_fk SELECT row(1)::fp_sc_vch FROM generate_series(1, 64); +SELECT count(*) FROM fp_sc_cast_fk; +INSERT INTO fp_sc_cast_deffk VALUES (999); +ROLLBACK; + +-- Now the deferred row is already violating when the cast runs. Its +-- SET CONSTRAINTS raises, the cast's subtransaction rolls back, and that +-- restores the deferred state, so the violation surfaces at commit instead. +BEGIN; +INSERT INTO fp_sc_cast_deffk VALUES (999); +INSERT INTO fp_sc_cast_fk SELECT row(1)::fp_sc_vch FROM generate_series(1, 64); +COMMIT; +SELECT count(*) FROM fp_sc_cast_fk; + +DROP TABLE fp_sc_cast_fk, fp_sc_cast_deffk, fp_sc_cast_pk, fp_sc_cast_defpk; +DROP CAST (fp_sc_vch AS int); +DROP FUNCTION fp_sc_vcast(fp_sc_vch); +DROP TYPE fp_sc_vch; + -- 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 -- 2.47.3