From 1c615e4026b41798d032f06d5968490514cf6ceb Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Wed, 2 Sep 2026 21:58:22 +0900 Subject: [PATCH 1/3] Take RI fast-path snapshot after locking referenced relation ri_FastPathCheck() acquired its scan snapshot before opening the referenced relation. If it then waited for the relation lock in READ COMMITTED mode, a referenced row committed during the wait would not be visible to the old snapshot. The check could consequently report a foreign key violation even though the referenced row existed. The batched path does not have this problem, because it opens the relations before acquiring the snapshot used to check the batch. Consequently, batching currently masks the problem for ordinary DML. Fix the per-row path before removing batching and making that path handle those checks. Take the snapshot after opening the referenced relation and reloading the constraint information. This also agrees with the SPI path, which selects its snapshot after executor startup has acquired the required locks. Add isolation-test coverage for the visibility of a referenced row committed after the referencing transaction has executed an earlier command. A later command can see such a row in READ COMMITTED, but not in REPEATABLE READ or SERIALIZABLE. Discussion: https://postgr.es/m/CA+HiwqEhm+_=bs=2wavAJz-UqC+1KebD31++mapJQQGweE8iQQ@mail.gmail.com Backpatch-through: 19 --- src/backend/utils/adt/ri_triggers.c | 14 +++++- src/test/isolation/expected/fk-snapshot-2.out | 44 +++++++++++++++++++ src/test/isolation/specs/fk-snapshot-2.spec | 15 +++++++ 3 files changed, 72 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 439376a6cc2..b3073a0323b 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -2882,7 +2882,6 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo, * ri_PerformCheck(). */ CommandCounterIncrement(); - snapshot = RegisterSnapshot(GetTransactionSnapshot()); INJECTION_POINT("ri-before-pk-lock", NULL); @@ -2893,6 +2892,19 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo, idx_rel = index_open(riinfo->conindid, AccessShareLock); + /* + * Only now take the snapshot the scan will use. Acquiring it before + * table_open() would let an unbounded amount of time pass while we wait + * for the lock, during which another transaction can commit the very row + * we are about to look for. The scan would not see it and the check + * would report a violation for a key that exists. + * + * The SPI path does not have this problem: for this check it passes + * InvalidSnapshot, so SPI takes the snapshot when the plan is executed, + * after the executor has taken its locks. + */ + snapshot = RegisterSnapshot(GetTransactionSnapshot()); + slot = table_slot_create(pk_rel, NULL); GetUserIdAndSecContext(&saved_userid, &saved_sec_context); diff --git a/src/test/isolation/expected/fk-snapshot-2.out b/src/test/isolation/expected/fk-snapshot-2.out index 7333643e9ac..376460a00e5 100644 --- a/src/test/isolation/expected/fk-snapshot-2.out +++ b/src/test/isolation/expected/fk-snapshot-2.out @@ -59,3 +59,47 @@ step s1c: COMMIT; step s2ins: <... completed> ERROR: could not serialize access due to concurrent delete step s2c: COMMIT; + +starting permutation: s1rr s2rr s2sel s1ins s1c s2ins2 s2c +step s1rr: BEGIN ISOLATION LEVEL REPEATABLE READ; +step s2rr: BEGIN ISOLATION LEVEL REPEATABLE READ; +step s2sel: SELECT count(*) FROM parent; +count +----- + 1 +(1 row) + +step s1ins: INSERT INTO parent VALUES (2); +step s1c: COMMIT; +step s2ins2: INSERT INTO child VALUES (2, 2); +ERROR: insert or update on table "child" violates foreign key constraint "child_parent_id_fkey" +step s2c: COMMIT; + +starting permutation: s1ser s2ser s2sel s1ins s1c s2ins2 s2c +step s1ser: BEGIN ISOLATION LEVEL SERIALIZABLE; +step s2ser: BEGIN ISOLATION LEVEL SERIALIZABLE; +step s2sel: SELECT count(*) FROM parent; +count +----- + 1 +(1 row) + +step s1ins: INSERT INTO parent VALUES (2); +step s1c: COMMIT; +step s2ins2: INSERT INTO child VALUES (2, 2); +ERROR: insert or update on table "child" violates foreign key constraint "child_parent_id_fkey" +step s2c: COMMIT; + +starting permutation: s1rc s2rc s2sel s1ins s1c s2ins2 s2c +step s1rc: BEGIN ISOLATION LEVEL READ COMMITTED; +step s2rc: BEGIN ISOLATION LEVEL READ COMMITTED; +step s2sel: SELECT count(*) FROM parent; +count +----- + 1 +(1 row) + +step s1ins: INSERT INTO parent VALUES (2); +step s1c: COMMIT; +step s2ins2: INSERT INTO child VALUES (2, 2); +step s2c: COMMIT; diff --git a/src/test/isolation/specs/fk-snapshot-2.spec b/src/test/isolation/specs/fk-snapshot-2.spec index 94cd151aab9..5b0144c0f70 100644 --- a/src/test/isolation/specs/fk-snapshot-2.spec +++ b/src/test/isolation/specs/fk-snapshot-2.spec @@ -21,6 +21,7 @@ step s1rr { BEGIN ISOLATION LEVEL REPEATABLE READ; } step s1ser { BEGIN ISOLATION LEVEL SERIALIZABLE; } step s1del { DELETE FROM parent WHERE parent_id = 1; } step s1c { COMMIT; } +step s1ins { INSERT INTO parent VALUES (2); } session s2 step s2rc { BEGIN ISOLATION LEVEL READ COMMITTED; } @@ -28,6 +29,8 @@ step s2rr { BEGIN ISOLATION LEVEL REPEATABLE READ; } step s2ser { BEGIN ISOLATION LEVEL SERIALIZABLE; } step s2ins { INSERT INTO child VALUES (1, 1); } step s2c { COMMIT; } +step s2sel { SELECT count(*) FROM parent; } +step s2ins2 { INSERT INTO child VALUES (2, 2); } # Violates referential integrity unless we use a crosscheck snapshot, # which is up-to-date compared with the transaction's snapshot. @@ -48,3 +51,15 @@ permutation s1ser s2ser s2ins s1del s2c s1c # We raise a concurrent update error # which is good enough: permutation s1ser s2ser s1del s2ins s1c s2c + +# A parent row committed by another transaction after this one took its +# snapshot. RI_FKey_check passes detectNewRows = false, so the check runs +# under the transaction snapshot rather than a current one, and the row is +# correctly not visible: referencing it is a violation. Only the parent-side +# checks need to see rows committed since the snapshot. +permutation s1rr s2rr s2sel s1ins s1c s2ins2 s2c +permutation s1ser s2ser s2sel s1ins s1c s2ins2 s2c + +# The same order in READ COMMITTED, where the check's snapshot is a fresh one +# and the parent row is visible. +permutation s1rc s2rc s2sel s1ins s1c s2ins2 s2c From 324f846845e14a79271b60271d30250f983633b6 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Wed, 2 Sep 2026 22:45:00 +0900 Subject: [PATCH 2/3] Remove batching from RI fast-path checks Commit b7b27eb41a5 added batching to the direct-index fast path for foreign key checks introduced by 2da86c1ef9b. Instead of probing the referenced index once per row, it accumulated referencing rows and checked them in groups, using SK_SEARCHARRAY for single-column foreign keys. The batching requires state to survive across trigger invocations and to be flushed at the end of each trigger-firing cycle. Follow-up work has had to define how that state interacts with nested trigger firing, subtransactions, deferred constraints, and SET CONSTRAINTS. In particular, SET CONSTRAINTS ... IMMEDIATE invoked from a trigger can re-enter the after-trigger machinery while an outer batch remains active. Failure to handle one of those cases can leave a buffered check unperformed, allowing a transaction to commit a permanent foreign key violation without reporting an error. With PostgreSQL 19 close to release, there is not enough time to gain confidence that all relevant trigger and transaction states have been covered. Remove the batching and its after-trigger callback infrastructure. This also removes the per-batch RI cache and the associated subtransaction cleanup. Restore AfterTriggerFireDeferred() to its form before batching was added. Retain the tests added with the batching commit and its follow-up fixes, because they continue to exercise the underlying RI cases through the per-row path. This preserves regression coverage for those behaviors and keeps test coverage aligned with master, simplifying future backpatching of test cases. Keep the underlying per-row fast path. It performs each check synchronously, retains no state across trigger invocations, and requires no changes to the trigger or subtransaction machinery. Also retain the fast-path metadata invalidation handling and the fixes made to the per-row probe, including support for domain-typed referencing columns, restriction to btree referenced indexes, concurrent index replacement, metadata invalidation, and nullable referenced keys. This removal applies only to REL_19_STABLE. The batched implementation is retained in master for v20 development. Discussion: https://postgr.es/m/ --- src/backend/access/transam/xact.c | 2 - src/backend/commands/trigger.c | 206 +----- src/backend/utils/adt/ri_triggers.c | 930 ++-------------------------- src/include/commands/trigger.h | 24 - 4 files changed, 40 insertions(+), 1122 deletions(-) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 9e2d507c8a9..3a89149016f 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -5245,7 +5245,6 @@ CommitSubTransaction(void) s->parent->subTransactionId); AtEOSubXact_HashTables(true, s->nestingLevel); AtEOSubXact_PgStat(true, s->nestingLevel); - AtEOSubXact_RI(true, s->subTransactionId, s->parent->subTransactionId); AtSubCommit_Snapshot(s->nestingLevel); /* @@ -5420,7 +5419,6 @@ AbortSubTransaction(void) s->parent->subTransactionId); AtEOSubXact_HashTables(false, s->nestingLevel); AtEOSubXact_PgStat(false, s->nestingLevel); - AtEOSubXact_RI(false, s->subTransactionId, s->parent->subTransactionId); AtSubAbort_Snapshot(s->nestingLevel); } diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 911045b9b9d..2555cbb015d 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -3905,18 +3905,6 @@ typedef struct AfterTriggersData /* per-subtransaction-level data: */ AfterTriggersTransData *trans_stack; /* array of structs shown below */ int maxtransdepth; /* allocated len of above array */ - - List *batch_callbacks; /* List of AfterTriggerCallbackItem; for - * deferred constraints */ - bool firing_batch_callbacks; /* true when in - * FireAfterTriggerBatchCallbacks() */ - - /* - * 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 @@ -3924,7 +3912,6 @@ 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 @@ -3933,8 +3920,6 @@ struct AfterTriggersTransData SetConstraintState state; /* saved S C state, or NULL if not yet saved */ AfterTriggerEventList events; /* saved list pointer */ int query_depth; /* saved query_depth */ - int firing_depth; /* saved firing_depth */ - bool firing_batch_callbacks; /* saved firing_batch_callbacks */ CommandId firing_counter; /* saved firing_counter */ }; @@ -3956,13 +3941,6 @@ struct AfterTriggersTableData TupleTableSlot *storeslot; /* for converting to tuplestore's format */ }; -/* Entry in afterTriggers.batch_callbacks */ -typedef struct AfterTriggerCallbackItem -{ - AfterTriggerBatchCallback callback; - void *arg; -} AfterTriggerCallbackItem; - static AfterTriggersData afterTriggers; static void AfterTriggerExecute(EState *estate, @@ -3998,7 +3976,6 @@ static SetConstraintState SetConstraintStateAddItem(SetConstraintState state, Oid tgoid, bool tgisdeferred); static void cancel_prior_stmt_triggers(Oid relid, CmdType cmdType, int tgevent); -static void FireAfterTriggerBatchCallbacks(List *callbacks); /* * Get the FDW tuplestore for the current trigger query level, creating it @@ -5124,9 +5101,6 @@ 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; /* * Verify that there is no leftover state remaining. If these assertions @@ -5211,7 +5185,6 @@ AfterTriggerEndQuery(EState *estate) */ qs = &afterTriggers.query_stack[afterTriggers.query_depth]; - afterTriggers.firing_depth++; for (;;) { if (afterTriggerMarkEvents(&qs->events, &afterTriggers.events, true)) @@ -5249,23 +5222,10 @@ AfterTriggerEndQuery(EState *estate) break; } - /* - * Fire batch callbacks before releasing query-level storage and before - * decrementing query_depth. Callbacks may do real work (index probes, - * error reporting). - * - * Recompute qs first: the loop above refreshes it after each - * afterTriggerInvokeEvents() call (see comment there), but the "all - * fired" break exits without doing so, leaving qs potentially stale here. - */ - qs = &afterTriggers.query_stack[afterTriggers.query_depth]; - FireAfterTriggerBatchCallbacks(qs->batch_callbacks); - /* Release query-level-local storage, including tuplestores if any */ AfterTriggerFreeQuery(&afterTriggers.query_stack[afterTriggers.query_depth]); afterTriggers.query_depth--; - afterTriggers.firing_depth--; } @@ -5322,9 +5282,6 @@ AfterTriggerFreeQuery(AfterTriggersQueryData *qs) */ qs->tables = NIL; list_free_deep(tables); - - list_free_deep(qs->batch_callbacks); - qs->batch_callbacks = NIL; } @@ -5364,34 +5321,17 @@ AfterTriggerFireDeferred(void) * Run all the remaining triggers. Loop until they are all gone, in case * some trigger queues more for us to do. */ - afterTriggers.firing_depth++; while (afterTriggerMarkEvents(events, NULL, false)) { CommandId firing_id = afterTriggers.firing_counter++; - (void) afterTriggerInvokeEvents(events, firing_id, NULL, true); - - /* - * Flush any fast-path FK-check batches accumulated by the triggers - * just fired. A batch callback runs user-supplied cast or equality - * functions, whose DML can queue further deferred trigger events. - * Flush inside the loop so afterTriggerMarkEvents() sees any such - * events on the next iteration and fires them; flushing after the - * loop would leave them unfired, silently skipping e.g. a deferred FK - * check and letting a violating row commit. (The former "all fired" - * break is therefore gone: the loop now terminates only when - * afterTriggerMarkEvents() finds nothing left, including events - * queued by the flush.) - */ - FireAfterTriggerBatchCallbacks(afterTriggers.batch_callbacks); + if (afterTriggerInvokeEvents(events, firing_id, NULL, true)) + break; /* all fired */ } - afterTriggers.firing_depth--; - /* - * 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. + * We don't bother freeing the event list, since it will go away anyway + * (and more efficiently than via pfree) in AfterTriggerEndXact. */ if (snap_pushed) @@ -5453,12 +5393,6 @@ AfterTriggerEndXact(bool isCommit) /* No more afterTriggers manipulation until next transaction starts. */ afterTriggers.query_depth = -1; - - afterTriggers.firing_depth = 0; - - list_free_deep(afterTriggers.batch_callbacks); - afterTriggers.batch_callbacks = NIL; - afterTriggers.firing_batch_callbacks = false; } /* @@ -5506,9 +5440,6 @@ AfterTriggerBeginSubXact(void) afterTriggers.trans_stack[my_level].state = NULL; afterTriggers.trans_stack[my_level].events = afterTriggers.events; afterTriggers.trans_stack[my_level].query_depth = afterTriggers.query_depth; - afterTriggers.trans_stack[my_level].firing_depth = afterTriggers.firing_depth; - afterTriggers.trans_stack[my_level].firing_batch_callbacks = - afterTriggers.firing_batch_callbacks; afterTriggers.trans_stack[my_level].firing_counter = afterTriggers.firing_counter; } @@ -5608,29 +5539,6 @@ AfterTriggerEndSubXact(bool isCommit) } } } - - /* - * Restore firing_depth and firing_batch_callbacks to their values at - * subtransaction start. The matching decrement of firing_depth in - * AfterTriggerEndQuery()/AfterTriggerFireDeferred(), and the clearing of - * firing_batch_callbacks in FireAfterTriggerBatchCallbacks(), run after - * their loops and are not protected by PG_FINALLY. A trigger or batch - * callback error caught by this subtransaction can therefore leave either - * one set; restoring the saved values unwinds only this subtransaction's - * firing. - * - * Restoring (rather than zeroing/clearing) matters because a - * subtransaction can begin and end while an outer query's triggers are - * firing -- for instance a batch callback whose user-supplied cast or - * equality function runs DML in a BEGIN ... EXCEPTION block. There - * firing_depth is positive and firing_batch_callbacks is true; forcing - * them to 0/false would corrupt the outer firing - * (FireAfterTriggerBatchCallbacks() asserts firing_depth > 0, and - * clearing the guard would defeat its re-entrancy check). - */ - afterTriggers.firing_depth = afterTriggers.trans_stack[my_level].firing_depth; - afterTriggers.firing_batch_callbacks = - afterTriggers.trans_stack[my_level].firing_batch_callbacks; } /* @@ -5785,7 +5693,6 @@ AfterTriggerEnlargeQueryState(void) qs->events.tailfree = NULL; qs->fdw_tuplestore = NULL; qs->tables = NIL; - qs->batch_callbacks = NIL; ++init_depth; } @@ -6135,7 +6042,6 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt) AfterTriggerEventList *events = &afterTriggers.events; bool snapshot_set = false; - afterTriggers.firing_depth++; while (afterTriggerMarkEvents(events, NULL, true)) { CommandId firing_id = afterTriggers.firing_counter++; @@ -6165,14 +6071,6 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt) break; /* all fired */ } - /* - * 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; - if (snapshot_set) PopActiveSnapshot(); } @@ -6869,99 +6767,3 @@ check_modified_virtual_generated(TupleDesc tupdesc, HeapTuple tuple) return tuple; } - -/* - * RegisterAfterTriggerBatchCallback - * Register a function to be called when the current trigger-firing - * batch completes. - * - * Must be called from within a trigger function's execution context - * (i.e., while afterTriggers state is active). - * - * The callback list is cleared after invocation, so the caller must - * re-register for each new batch if needed. - */ -void -RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback, - void *arg) -{ - AfterTriggerCallbackItem *item; - MemoryContext oldcxt; - - /* - * Allocate in TopTransactionContext so the item survives for the duration - * of the batch, which may span multiple trigger invocations. - * - * Must be called while afterTriggers is active; callbacks registered - * outside a trigger-firing context would never fire. - */ - Assert(afterTriggers.firing_depth > 0); - Assert(!afterTriggers.firing_batch_callbacks); - oldcxt = MemoryContextSwitchTo(TopTransactionContext); - item = palloc(sizeof(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); - MemoryContextSwitchTo(oldcxt); -} - -/* - * FireAfterTriggerBatchCallbacks - * Invoke all callbacks in the given list. - * - * 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(List *callbacks) -{ - ListCell *lc; - - Assert(afterTriggers.firing_depth > 0); - afterTriggers.firing_batch_callbacks = true; - foreach(lc, callbacks) - { - AfterTriggerCallbackItem *item = lfirst(lc); - - item->callback(item->arg); - } - afterTriggers.firing_batch_callbacks = false; -} - -/* - * AfterTriggerIsActive - * Returns true if we're inside the after-trigger framework where - * registered batch callbacks will actually be invoked. - * - * This is false during validateForeignKeyConstraint(), which calls - * RI trigger functions directly outside the after-trigger framework. - */ -bool -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 b3073a0323b..0cb3f66c3a2 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -219,84 +219,6 @@ typedef struct RI_CompareHashEntry FmgrInfo cast_func_finfo; /* in case we must coerce input */ } RI_CompareHashEntry; -/* - * Maximum number of FK rows buffered before flushing. - * - * Larger batches amortize per-flush overhead and let the SK_SEARCHARRAY - * path walk more leaf pages in a single sorted traversal. But each - * buffered row is a materialized HeapTuple in flush_cxt, and the matched[] - * scan in ri_FastPathFlushArray() is O(batch_size) per index match. - * Benchmarking showed little difference between 16 and 64, with 256 - * consistently slower. 64 is a reasonable default. - */ -#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, per-firing-cycle cache of resources needed by - * ri_FastPathBatchFlush(). - * - * 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. - * - * RI_FastPathEntry is not subject to cache invalidation. The cached - * relations are held open with locks for the transaction duration, preventing - * relcache invalidation. The entry itself is torn down at batch end by - * ri_FastPathEndBatch(); on abort, ResourceOwner releases the cached - * relations and AtEOXact_RI() NULLs the static cache pointer to prevent - * any subsequent access. - */ -typedef struct RI_FastPathEntry -{ - RI_FastPathKey key; /* hash key */ - Oid fk_relid; /* for ri_FastPathEndBatch() */ - Relation pk_rel; - Relation idx_rel; - TupleTableSlot *pk_slot; - TupleTableSlot *fk_slot; - MemoryContext flush_cxt; /* short-lived context for per-flush work */ - - /* - * TODO: batch[] is HeapTuple[] because the AFTER trigger machinery - * currently passes tuples as HeapTuples. Once trigger infrastructure is - * slotified, this should use a slot array or whatever batched tuple - * storage abstraction exists at that point to be TAM-agnostic. - */ - HeapTuple batch[RI_FASTPATH_BATCH_SIZE]; - int batch_count; - - /* - * true while this entry's batch is being flushed; guards against - * re-entrant ri_FastPathBatchAdd from user code run during the flush. - */ - bool flushing; - - /* - * Subtransaction whose resource owner opened this entry's relations. - * AtEOSubXact_RI() drops only entries matching an aborting subxact, so a - * subxact abort during outer-level trigger firing leaves the outer batch - * intact. - */ - SubTransactionId subid; -} RI_FastPathEntry; - /* * Local data */ @@ -305,9 +227,6 @@ static HTAB *ri_query_cache = NULL; static HTAB *ri_compare_cache = NULL; static dclist_head ri_constraint_cache_valid_list; -static HTAB *ri_fastpath_cache = NULL; -static bool ri_fastpath_flushing = false; - /* * FastPathMeta objects detached from their cache entry by invalidation, but * possibly still referenced by an RI check further up the stack. Released @@ -365,18 +284,6 @@ static bool ri_PerformCheck(const RI_ConstraintInfo *riinfo, bool detectNewRows, int expect_OK); static void ri_FastPathCheck(RI_ConstraintInfo *riinfo, Relation fk_rel, TupleTableSlot *newslot); -static void ri_FastPathBatchAdd(RI_ConstraintInfo *riinfo, - Relation fk_rel, TupleTableSlot *newslot); -static void ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel, - RI_ConstraintInfo *riinfo); -static int ri_FastPathFlushArray(RI_FastPathEntry *fpentry, TupleTableSlot *fk_slot, - const RI_ConstraintInfo *riinfo, - FastPathMeta *fpmeta, Relation fk_rel, - Snapshot snapshot, IndexScanDesc scandesc); -static int ri_FastPathFlushLoop(RI_FastPathEntry *fpentry, TupleTableSlot *fk_slot, - const RI_ConstraintInfo *riinfo, - FastPathMeta *fpmeta, Relation fk_rel, - Snapshot snapshot, IndexScanDesc scandesc); static bool ri_FastPathProbeOne(Relation pk_rel, Relation idx_rel, IndexScanDesc scandesc, TupleTableSlot *slot, Snapshot snapshot, const RI_ConstraintInfo *riinfo, @@ -400,10 +307,6 @@ pg_noreturn static void ri_ReportViolation(const RI_ConstraintInfo *riinfo, Relation pk_rel, Relation fk_rel, TupleTableSlot *violatorslot, TupleDesc tupdesc, int queryno, bool is_restrict, bool partgone); -static RI_FastPathEntry *ri_FastPathGetEntry(const RI_ConstraintInfo *riinfo, - Relation fk_rel); -static void ri_FastPathEndBatch(void *arg); -static void ri_FastPathTeardown(int depth); /* @@ -514,32 +417,12 @@ RI_FKey_check(TriggerData *trigdata) * lock. This is semantically equivalent to the SPI path below but avoids * the per-row executor overhead. * - * ri_FastPathBatchAdd() and ri_FastPathCheck() report the violation - * themselves if no matching PK row is found, so they only return on - * success. + * ri_FastPathCheck() reports the violation itself if no matching PK row + * is found, so it only returns on success. */ if (ri_fastpath_is_applicable(riinfo)) { - if (AfterTriggerIsActive() && !ri_fastpath_flushing) - { - /* Batched path: buffer and probe in groups */ - ri_FastPathBatchAdd(riinfo, fk_rel, newslot); - } - else - { - /* - * Per-row path, used when batching is not applicable: - * - * - ALTER TABLE validation, where no after-trigger firing is - * active; - * - * - a re-entrant check from user cast/operator code running - * during a batch flush, since adding a cache entry while - * ri_FastPathEndBatch is iterating the cache could leave it - * unflushed. - */ - ri_FastPathCheck(riinfo, fk_rel, newslot); - } + ri_FastPathCheck(riinfo, fk_rel, newslot); return PointerGetDatum(NULL); } @@ -2887,7 +2770,7 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo, pk_rel = table_open(riinfo->pk_relid, RowShareLock); - /* Re-read the constraint under that lock; see ri_FastPathGetEntry(). */ + /* Re-read the constraint under that lock. */ riinfo = ri_LoadConstraintInfo(riinfo->constraint_id); idx_rel = index_open(riinfo->conindid, AccessShareLock); @@ -2951,401 +2834,6 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo, table_close(pk_rel, NoLock); } -/* - * ri_FastPathBatchAdd - * Buffer a FK row for batched probing. - * - * Adds the row to the batch buffer. When the buffer is full, flushes all - * buffered rows by probing the PK index. Any violation is reported - * immediately during the flush via ri_ReportViolation (which does not return). - * - * Uses the per-batch cache (RI_FastPathEntry) to avoid per-row relation - * open/close, slot creation, etc. - * - * The batch is also flushed at end of trigger-firing cycle via - * ri_FastPathEndBatch(). - */ -static void -ri_FastPathBatchAdd(RI_ConstraintInfo *riinfo, - Relation fk_rel, TupleTableSlot *newslot) -{ - RI_FastPathEntry *fpentry = ri_FastPathGetEntry(riinfo, fk_rel); - - /* - * If this entry is already being flushed, a cast function or an operator - * invoked during the flush has re-entered with DML on the same FK. Fall - * back to the per-row path rather than touching the batch array, which is - * mid-flush. - */ - if (unlikely(fpentry->flushing)) - { - ri_FastPathCheck(riinfo, fk_rel, newslot); - return; - } - - /* - * A batch is filled and flushed within a single trigger-firing cycle, so - * every row added to an entry comes from the subtransaction that created - * it. AtEOSubXact_RI() relies on this to identify an aborting - * subtransaction's entries by the subid stamped at entry creation. - */ - Assert(fpentry->subid == GetCurrentSubTransactionId()); - - /* - * Buffer the row. A full batch is flushed below and re-entry is handled - * above, so there is always room here; the bounds check just guards the - * array write. - */ - if (fpentry->batch_count < RI_FASTPATH_BATCH_SIZE) - { - MemoryContext oldcxt = MemoryContextSwitchTo(fpentry->flush_cxt); - - fpentry->batch[fpentry->batch_count] = - ExecCopySlotHeapTuple(newslot); - fpentry->batch_count++; - MemoryContextSwitchTo(oldcxt); - } - else - elog(ERROR, "RI fast-path batch unexpectedly full"); - - /* Flush as soon as the batch is full. */ - if (fpentry->batch_count == RI_FASTPATH_BATCH_SIZE) - ri_FastPathBatchFlush(fpentry, fk_rel, riinfo); -} - -/* - * ri_FastPathBatchFlush - * Flush all buffered FK rows by probing the PK index. - * - * Dispatches to ri_FastPathFlushArray() for single-column FKs - * (using SK_SEARCHARRAY) or ri_FastPathFlushLoop() for multi-column - * FKs (per-row probing). Violations are reported immediately via - * ri_ReportViolation(), which does not return. - */ -static void -ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel, - RI_ConstraintInfo *riinfo) -{ - Relation pk_rel = fpentry->pk_rel; - Relation idx_rel = fpentry->idx_rel; - TupleTableSlot *fk_slot = fpentry->fk_slot; - Snapshot snapshot; - IndexScanDesc scandesc; - Oid saved_userid; - int saved_sec_context; - MemoryContext oldcxt; - FastPathMeta *fpmeta; - int violation_index; - - if (fpentry->batch_count == 0) - return; - - /* - * CCI and security context switch are done once for the entire batch. - * Per-row CCI is unnecessary because by the time a flush runs, all AFTER - * triggers for the buffered rows have already fired (trigger invocations - * strictly alternate per row), so a single CCI advances past all their - * effects. Per-row security context switch is unnecessary because each - * row's probe runs entirely as the PK table owner, same as the SPI path - * -- the only difference is that the SPI path sets and restores the - * context per row whereas we do it once around the whole batch. - */ - CommandCounterIncrement(); - snapshot = RegisterSnapshot(GetTransactionSnapshot()); - - /* - * build_index_scankeys() may palloc cast results for cross-type FKs. Use - * the entry's short-lived flush context so these don't accumulate across - * batches. - */ - oldcxt = MemoryContextSwitchTo(fpentry->flush_cxt); - - GetUserIdAndSecContext(&saved_userid, &saved_sec_context); - SetUserIdAndSecContext(RelationGetForm(pk_rel)->relowner, - saved_sec_context | - SECURITY_LOCAL_USERID_CHANGE | - SECURITY_NOFORCE_RLS); - - /* - * Check that the current user has permission to access pk_rel. Done here - * rather than at entry creation so that permission changes between - * flushes are respected, matching the per-row behavior of the SPI path, - * albeit checked once per flush rather than once per row, like in - * ri_FastPathCheck(). - */ - ri_CheckPermissions(pk_rel); - - /* - * Begin the scan under the switched user id, so that any access method - * code invoked by index_beginscan() runs as the PK relation's owner. For - * btree this has no functional consequence, but it keeps the ordering - * correct for out-of-tree access methods. - */ - scandesc = index_beginscan(pk_rel, idx_rel, snapshot, NULL, - riinfo->nkeys, 0, SO_NONE); - - if (riinfo->fpmeta == NULL) - { - /* Reload to ensure it's valid. */ - riinfo = ri_LoadConstraintInfo(riinfo->constraint_id); - ri_populate_fastpath_metadata(riinfo, fk_rel, idx_rel); - } - Assert(riinfo->fpmeta); - - /* - * Take our own reference to the metadata for the duration of the flush. - * The probe below runs user-defined cast and equality functions, which - * can accept invalidation messages; InvalidateConstraintCacheCallBack() - * then clears riinfo->fpmeta, so re-reading it partway through the batch - * would find NULL. The object itself stays valid until AtEOXact_RI(). - */ - fpmeta = riinfo->fpmeta; - - /* - * The probe runs user-defined cast and equality functions. Set the - * flushing flag around it so a re-entrant ri_FastPathBatchAdd on this - * entry takes the per-row path, and clear it even on error so the entry - * is reusable if the error is caught by a savepoint. - */ - Assert(!fpentry->flushing); - fpentry->flushing = true; - PG_TRY(); - { - /* Skip array overhead for single-row batches. */ - if (riinfo->nkeys == 1 && fpentry->batch_count > 1) - violation_index = ri_FastPathFlushArray(fpentry, fk_slot, riinfo, - fpmeta, fk_rel, snapshot, - scandesc); - else - violation_index = ri_FastPathFlushLoop(fpentry, fk_slot, riinfo, - fpmeta, fk_rel, snapshot, - scandesc); - } - PG_FINALLY(); - { - fpentry->flushing = false; - fpentry->batch_count = 0; - } - PG_END_TRY(); - - SetUserIdAndSecContext(saved_userid, saved_sec_context); - UnregisterSnapshot(snapshot); - index_endscan(scandesc); - - if (violation_index >= 0) - { - ExecStoreHeapTuple(fpentry->batch[violation_index], fk_slot, false); - ri_ReportViolation(riinfo, pk_rel, fk_rel, - fk_slot, NULL, - RI_PLAN_CHECK_LOOKUPPK, false, false); - } - - MemoryContextReset(fpentry->flush_cxt); - MemoryContextSwitchTo(oldcxt); -} - -/* - * ri_FastPathFlushLoop - * Multi-column fallback: probe the index once per buffered row. - * - * Used for composite foreign keys where SK_SEARCHARRAY does not - * apply, and also for single-row batches of single-column FKs where - * the array overhead is not worth it. - * - * Returns the index of the first violating row in the batch array, or -1 if - * all rows are valid. - */ -static int -ri_FastPathFlushLoop(RI_FastPathEntry *fpentry, TupleTableSlot *fk_slot, - const RI_ConstraintInfo *riinfo, FastPathMeta *fpmeta, - Relation fk_rel, Snapshot snapshot, - IndexScanDesc scandesc) -{ - Relation pk_rel = fpentry->pk_rel; - Relation idx_rel = fpentry->idx_rel; - TupleTableSlot *pk_slot = fpentry->pk_slot; - Datum pk_vals[INDEX_MAX_KEYS]; - char pk_nulls[INDEX_MAX_KEYS]; - ScanKeyData skey[INDEX_MAX_KEYS]; - bool found = true; - - for (int i = 0; i < fpentry->batch_count; i++) - { - ExecStoreHeapTuple(fpentry->batch[i], fk_slot, false); - ri_ExtractValues(fk_rel, fk_slot, riinfo, false, pk_vals, pk_nulls); - build_index_scankeys(riinfo, fpmeta, idx_rel, pk_vals, pk_nulls, skey); - - found = ri_FastPathProbeOne(pk_rel, idx_rel, scandesc, pk_slot, - snapshot, riinfo, skey, riinfo->nkeys); - - /* Report first unmatched row */ - if (!found) - return i; - } - - /* All pass. */ - return -1; -} - -/* - * ri_FastPathFlushArray - * Single-column fast path using SK_SEARCHARRAY. - * - * Builds an array of FK values and does one index scan with - * SK_SEARCHARRAY. The index AM sorts and deduplicates the array - * internally, then walks matching leaf pages in order. Each - * matched PK tuple is locked and rechecked as before; a matched[] - * bitmap tracks which batch items were satisfied. - * - * Returns the index of the first violating row in the batch array, or -1 if - * all rows are valid. - */ -static int -ri_FastPathFlushArray(RI_FastPathEntry *fpentry, TupleTableSlot *fk_slot, - const RI_ConstraintInfo *riinfo, FastPathMeta *fpmeta, - Relation fk_rel, Snapshot snapshot, - IndexScanDesc scandesc) -{ - Relation pk_rel = fpentry->pk_rel; - Relation idx_rel = fpentry->idx_rel; - TupleTableSlot *pk_slot = fpentry->pk_slot; - Datum search_vals[RI_FASTPATH_BATCH_SIZE]; - bool matched[RI_FASTPATH_BATCH_SIZE]; - int nvals = fpentry->batch_count; - Datum pk_vals[INDEX_MAX_KEYS]; - char pk_nulls[INDEX_MAX_KEYS]; - ScanKeyData skey[1]; - FmgrInfo *cast_func_finfo; - FmgrInfo *eq_opr_finfo; - Oid elem_type; - int16 elem_len; - bool elem_byval; - char elem_align; - ArrayType *arr; - - Assert(fpmeta); - - memset(matched, 0, nvals * sizeof(bool)); - - /* - * Extract FK values, casting to the operator's expected input type if - * needed (e.g. int8 FK -> int4 for int48eq). - */ - cast_func_finfo = &fpmeta->cast_func_finfo[0]; - eq_opr_finfo = &fpmeta->eq_opr_finfo[0]; - for (int i = 0; i < nvals; i++) - { - ExecStoreHeapTuple(fpentry->batch[i], fk_slot, false); - ri_ExtractValues(fk_rel, fk_slot, riinfo, false, pk_vals, pk_nulls); - - /* Cast if needed (e.g. int8 FK -> numeric PK) */ - if (OidIsValid(cast_func_finfo->fn_oid)) - search_vals[i] = FunctionCall3(cast_func_finfo, - pk_vals[0], - Int32GetDatum(-1), - BoolGetDatum(false)); - else - search_vals[i] = pk_vals[0]; - } - - /* - * Array element type must match the operator's right-hand input type, - * which is what the index comparison expects on the search side. - * ri_populate_fastpath_metadata() stores exactly this via - * get_op_opfamily_properties(), which returns the operator's right-hand - * type as the subtype for cross-type operators (e.g. int8 for int48eq) - * and the common type for same-type operators. - */ - elem_type = fpmeta->subtypes[0]; - Assert(OidIsValid(elem_type)); - get_typlenbyvalalign(elem_type, &elem_len, &elem_byval, &elem_align); - - arr = construct_array(search_vals, nvals, - elem_type, elem_len, elem_byval, elem_align); - - /* - * Build scan key with SK_SEARCHARRAY. The index AM code will internally - * sort and deduplicate, then walk leaf pages in order. - * - * ri_fastpath_is_applicable() restricts the fast path to btree indexes, - * which support SK_SEARCHARRAY. - * - * This path handles single-column FKs only, so index_attnos[0] == 1. - */ - Assert(idx_rel->rd_indam->amsearcharray); - Assert(fpmeta->index_attnos[0] == 1); - ScanKeyEntryInitialize(&skey[0], - SK_SEARCHARRAY, - fpmeta->index_attnos[0], - fpmeta->strats[0], - fpmeta->subtypes[0], - idx_rel->rd_indcollation[fpmeta->index_attnos[0] - 1], - fpmeta->regops[0], - PointerGetDatum(arr)); - - index_rescan(scandesc, skey, 1, NULL, 0); - - /* - * Walk all matches. The index AM returns them in index order. For each - * match, find which batch item(s) it satisfies. - */ - while (index_getnext_slot(scandesc, ForwardScanDirection, pk_slot)) - { - Datum found_val; - bool found_null; - - /* - * No key recheck is needed here, so we have no use for - * concurrently_updated. Unlike ri_FastPathProbeOne(), which takes - * the index scan's word for it that the tuple matches, this path - * compares the key against every buffered FK value below, and it does - * so using found_val, which is read out of the version we actually - * locked. A concurrent key update is therefore caught by that - * comparison: the batch item that led us to this tuple is left - * unmatched and reported as a violation. - */ - if (!ri_LockPKTuple(pk_rel, pk_slot, snapshot, NULL)) - continue; - - /* - * Extract the PK value from the matched and locked tuple. - * - * A foreign key may reference a nullable unique column, not just a - * NOT NULL primary key. If ri_LockPKTuple() chased an update chain - * to a version whose referenced key is now NULL, that version cannot - * equal any buffered (non-null) FK value, so skip it. This mirrors - * the SPI path, where the requalifying "pkatt = $n" yields NULL and - * the row is not returned. - */ - found_val = slot_getattr(pk_slot, riinfo->pk_attnums[0], &found_null); - if (found_null) - continue; - - /* - * Linear scan to mark all batch items matching this PK value. - * O(batch_size) per match, O(batch_size^2) worst case -- fine for the - * current batch size of 64. - */ - for (int i = 0; i < nvals; i++) - { - if (!matched[i] && - DatumGetBool(FunctionCall2Coll(eq_opr_finfo, - idx_rel->rd_indcollation[0], - found_val, - search_vals[i]))) - matched[i] = true; - } - } - - /* Report first unmatched row */ - for (int i = 0; i < nvals; i++) - if (!matched[i]) - return i; - - /* All pass. */ - return -1; -} - /* * ri_FastPathProbeOne * Probe the PK index for one set of scan keys, lock the matching @@ -3696,6 +3184,38 @@ ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo, MemoryContextSwitchTo(oldcxt); } +/* + * AtEOXact_RI + * End-of-transaction cleanup for referential integrity. + * + * Currently this only releases fast-path metadata detached during the + * transaction. InvalidateConstraintCacheCallBack() cannot free a + * FastPathMeta when it detaches one, because an RI check further up the + * stack may still hold a pointer into it. It queues them on + * ri_fpmeta_dead_list instead, and we release them here, where no such + * reference can exist. isCommit is accepted for consistency with the + * other AtEOXact_* routines but is not used: the release is the same on + * the commit and the abort path. + * + * There is no AtEOSubXact_RI() counterpart. Nothing here is scoped to a + * subtransaction: a detached FastPathMeta stays reachable from the dead + * list whichever subtransaction detached it, and a check holding a pointer + * into one may be running at an outer level, so releasing at subtransaction + * end would be unsafe as well as unnecessary. + */ +void +AtEOXact_RI(bool isCommit) +{ + while (ri_fpmeta_dead_list != NULL) + { + FastPathMeta *dead = ri_fpmeta_dead_list; + + ri_fpmeta_dead_list = dead->next_dead; + MemoryContextDelete(dead->scratch_cxt); + pfree(dead); + } +} + /* * Extract fields from a tuple into Datum/nulls arrays */ @@ -4321,381 +3841,3 @@ RI_FKey_trigger_type(Oid tgfoid) return RI_TRIGGER_NONE; } - -/* - * ri_FastPathEndBatch - * Flush remaining rows and tear down cached state. - * - * Registered as an AfterTriggerBatchCallback. Note: the flush can - * do real work (CCI, security context switch, index probes) and can - * throw ERROR on a constraint violation. If that happens, - * ri_FastPathTeardown never runs; ResourceOwner releases the cached - * relations and AtEOXact_RI() resets the static state on the abort path. - */ -static void -ri_FastPathEndBatch(void *arg) -{ - HASH_SEQ_STATUS status; - RI_FastPathEntry *entry; - int my_depth = (int) (intptr_t) arg; - - if (ri_fastpath_cache == NULL) - return; - - /* - * Set a flag for the duration of the scan so that any FK check triggered - * by user cast or operator code during a flush takes the per-row path - * instead of adding a new entry to the cache we are iterating. A new - * entry could land in an already-scanned bucket and then be torn down - * unflushed below. - * - * The flush can throw ERROR (a reported constraint violation, or an error - * from the user code it runs). In that case ri_FastPathTeardown below is - * skipped; the ResourceOwner and the transaction-end callback handle - * resource cleanup on the abort path. The PG_FINALLY only resets the - * flag and deliberately does not attempt teardown. - */ - Assert(!ri_fastpath_flushing); - ri_fastpath_flushing = true; - PG_TRY(); - { - hash_seq_init(&status, ri_fastpath_cache); - 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) - { - Relation fk_rel = table_open(entry->fk_relid, AccessShareLock); - RI_ConstraintInfo *riinfo; - - riinfo = ri_LoadConstraintInfo(entry->key.conoid); - - ri_FastPathBatchFlush(entry, fk_rel, riinfo); - table_close(fk_rel, NoLock); - } - } - } - PG_FINALLY(); - { - ri_fastpath_flushing = false; - } - PG_END_TRY(); - - /* - * 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 - * 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 - * 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) -{ - HASH_SEQ_STATUS status; - RI_FastPathEntry *entry; - - if (ri_fastpath_cache == NULL) - return; - - 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) - table_close(entry->pk_rel, NoLock); - if (entry->pk_slot) - ExecDropSingleTupleTableSlot(entry->pk_slot); - if (entry->fk_slot) - ExecDropSingleTupleTableSlot(entry->fk_slot); - if (entry->flush_cxt) - MemoryContextDelete(entry->flush_cxt); - hash_search(ri_fastpath_cache, &entry->key, HASH_REMOVE, NULL); - } - - if (hash_get_num_entries(ri_fastpath_cache) == 0) - { - hash_destroy(ri_fastpath_cache); - ri_fastpath_cache = NULL; - ri_fastpath_flushing = false; - } -} - -/* - * AtEOXact_RI - * Reset fast-path batching state at end of transaction. - * - * Called from CommitTransaction() and PrepareTransaction() with isCommit - * true, and from AbortTransaction() with isCommit false. - * - * By the time we get here on a clean commit or prepare, the fast-path cache - * has already been flushed and torn down by ri_FastPathEndBatch() (an - * AfterTriggerBatchCallback fired from AfterTriggerFireDeferred(), well before - * this point), so the static pointers are already clear and the reset below is - * a no-op. A surviving cache at commit means a trigger batch was never - * flushed, which would have silently skipped FK checks, so we complain. - * - * On abort, ri_FastPathEndBatch()/ri_FastPathTeardown() may not have run (a - * flush can error out partway): the ResourceOwner releases the cached - * relations and the TopTransactionContext reset frees the cache memory, but - * the process-local static pointers below would dangle into the next - * transaction. This resets them so they don't. - * - * The reset touches only backend-local static state (no relations, locks, - * buffers or catalog access), so it has no ordering dependency on the - * surrounding ResourceOwnerRelease() / AtEOXact_* steps. - */ -void -AtEOXact_RI(bool isCommit) -{ - /* - * The cache must be empty on a clean commit or prepare; a survivor means - * a trigger batch went unflushed. Assert for assert-enabled builds and, - * since the transaction is already committed by now and FK checks may - * have been skipped, also warn in production builds. - */ - Assert(ri_fastpath_cache == NULL || !isCommit); - if (isCommit && ri_fastpath_cache != NULL) - elog(WARNING, "RI fast-path cache not flushed at end of transaction"); - - /* - * Clear the static pointers/flags. The cache memory lives in - * TopTransactionContext and is freed by the end-of-transaction - * memory-context reset; here we only drop the references to it. - */ - ri_fastpath_cache = NULL; - - /* - * Also clear the in-flush flag. ri_FastPathEndBatch() already clears it - * via PG_FINALLY, so this is just defensive: it keeps a stale flag from - * surviving into the next transaction should any future path leave it - * set. - */ - ri_fastpath_flushing = false; - - /* - * Release fast-path metadata detached during this transaction by - * InvalidateConstraintCacheCallBack(). We are past every RI check that - * could still hold a pointer into one of these, so freeing here is safe - * on both the commit and the abort path. - */ - while (ri_fpmeta_dead_list != NULL) - { - FastPathMeta *dead = ri_fpmeta_dead_list; - - ri_fpmeta_dead_list = dead->next_dead; - MemoryContextDelete(dead->scratch_cxt); - pfree(dead); - } -} - -/* - * AtEOSubXact_RI - * Reset fast-path batching state at subtransaction end. - * - * Called from CommitSubTransaction() with isCommit true and from - * AbortSubTransaction() with isCommit false, in both cases after the - * subtransaction's ResourceOwnerRelease(). - * - * Fast-path cache entries are normally flushed and removed at the end of - * their trigger-firing cycle, and the cache is destroyed when its last entry - * is removed. Thus, at a normal subtransaction boundary this is a no-op. - * - * The exception is a batch flush that errors out partway and is caught by this - * subtransaction (e.g. a PL/pgSQL EXCEPTION block): ri_FastPathEndBatch()'s - * teardown was skipped, so the cache still contains entries whose relations - * were opened under this subtransaction's resource owner. That owner has - * just released those relations, making the entries stale. Remove those - * entries so a later firing cycle cannot reuse them. Entries belonging to - * outer subtransactions remain valid and are preserved. - * - * The remaining slot storage and per-entry flush contexts are reclaimed when - * TopTransactionContext is reset at top-level transaction end. - */ -void -AtEOSubXact_RI(bool isCommit, SubTransactionId mySubid, - SubTransactionId parentSubid) -{ - HASH_SEQ_STATUS status; - RI_FastPathEntry *entry; - long remaining; - - if (ri_fastpath_cache == NULL) - return; - - /* Process only entries belonging to the ending subtransaction. */ - hash_seq_init(&status, ri_fastpath_cache); - while ((entry = hash_seq_search(&status)) != NULL) - { - 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); - } - - /* If that emptied the cache, drop it so the next batch starts clean. */ - remaining = hash_get_num_entries(ri_fastpath_cache); - if (remaining == 0) - { - hash_destroy(ri_fastpath_cache); - ri_fastpath_cache = NULL; - ri_fastpath_flushing = false; - } -} - -/* - * ri_FastPathGetEntry - * Look up or create a per-batch cache entry for the given constraint. - * - * On first call for a constraint within a batch: opens pk_rel and the index, - * allocates slots for both FK row and the looked up PK row, and registers the - * cleanup callback. - * - * On subsequent calls: returns the existing entry. - */ -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(RI_FastPathKey); - ctl.entrysize = sizeof(RI_FastPathEntry); - ctl.hcxt = TopTransactionContext; - ri_fastpath_cache = hash_create("RI fast-path cache", - 16, - &ctl, - HASH_ELEM | HASH_BLOBS | HASH_CONTEXT); - } - - entry = hash_search(ri_fastpath_cache, &key, - HASH_ENTER, &found); - - if (!found) - { - MemoryContext oldcxt; - - /* - * Zero out non-key fields so ri_FastPathTeardown is safe if we error - * out during partial initialization below. - */ - memset(((char *) entry) + offsetof(RI_FastPathEntry, pk_rel), 0, - sizeof(RI_FastPathEntry) - offsetof(RI_FastPathEntry, pk_rel)); - - oldcxt = MemoryContextSwitchTo(TopTransactionContext); - - entry->fk_relid = RelationGetRelid(fk_rel); - - /* - * Open PK table and its unique index. - * - * RowShareLock on pk_rel matches what the SPI path's SELECT ... FOR - * KEY SHARE would acquire as a relation-level lock. AccessShareLock - * on the index is standard for index scans. - * - * We don't release these locks until end of transaction, matching SPI - * behavior. - */ - - INJECTION_POINT("ri-before-pk-lock", NULL); - - entry->pk_rel = table_open(riinfo->pk_relid, RowShareLock); - - /* - * conindid may have been read before we took that lock, and REINDEX - * CONCURRENTLY moves a constraint to a new index. Re-read it now: - * LockRelationOid() processes invalidation messages after acquiring - * the lock, so we either see the new index, or an old one that cannot - * be marked dead or dropped until this transaction ends. - */ - riinfo = ri_LoadConstraintInfo(riinfo->constraint_id); - - entry->idx_rel = index_open(riinfo->conindid, AccessShareLock); - entry->pk_slot = table_slot_create(entry->pk_rel, NULL); - - /* - * Must be TTSOpsHeapTuple because ExecStoreHeapTuple() is used to - * load entries from batch[] into this slot for value extraction. - */ - entry->fk_slot = MakeSingleTupleTableSlot(RelationGetDescr(fk_rel), - &TTSOpsHeapTuple); - - entry->flush_cxt = AllocSetContextCreate(TopTransactionContext, - "RI fast path flush temporary context", - ALLOCSET_SMALL_SIZES); - MemoryContextSwitchTo(oldcxt); - - /* - * Register an end-of-batch callback once per firing cycle, passing - * the query depth so the callback flushes only entries belonging to - * that cycle. - */ - { - 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; - entry->batch_count = 0; - entry->subid = GetCurrentSubTransactionId(); - } - - return entry; -} diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index fecdb785f35..d5cb16597f4 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -289,30 +289,6 @@ extern void RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, extern int RI_FKey_trigger_type(Oid tgfoid); -/* - * Callback type for end-of-trigger-batch callbacks. - * - * Currently used by ri_triggers.c to flush fast-path FK batches and - * clean up associated resources. - * - * Registered via RegisterAfterTriggerBatchCallback(). Invoked when - * the current trigger-firing batch completes: - * - AfterTriggerEndQuery() (immediate constraints) - * - AfterTriggerFireDeferred() (deferred constraints at COMMIT) - * - AfterTriggerSetState() (SET CONSTRAINTS IMMEDIATE) - * - * The callback list is cleared after each batch. Callers must - * re-register if they need to be called again in a subsequent batch. - */ -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); -extern void AtEOSubXact_RI(bool isCommit, SubTransactionId mySubid, - SubTransactionId parentSubid); #endif /* TRIGGER_H */ From e7f519bd02e503d5f1894bc449145c2580d479d2 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Thu, 3 Sep 2026 15:28:39 +0900 Subject: [PATCH 3/3] fixups per review --- doc/src/sgml/release-19.sgml | 6 ------ src/backend/utils/adt/ri_triggers.c | 20 ++++++++++---------- src/tools/pgindent/typedefs.list | 4 ---- 3 files changed, 10 insertions(+), 20 deletions(-) diff --git a/doc/src/sgml/release-19.sgml b/doc/src/sgml/release-19.sgml index ba85e9140ec..a4d0db5e0d4 100644 --- a/doc/src/sgml/release-19.sgml +++ b/doc/src/sgml/release-19.sgml @@ -704,12 +704,6 @@ This information is used by the optimizer in planning memory usage. diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 0cb3f66c3a2..cfcf32311b5 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -2526,10 +2526,11 @@ InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid, /* * Detach any fast-path metadata so that the next check * repopulates it, but do not free it here. ri_FastPathCheck() - * and the flush routines copy riinfo->fpmeta into a local (and - * take FmgrInfo pointers into it) and then run index scans, tuple - * locking, and user-supplied cast and equality functions, all of - * which can accept invalidation messages and reach this callback. + * copies riinfo->fpmeta into a local (and takes FmgrInfo pointers + * into it) and then run index scans, tuple locking, and + * user-supplied cast and equality functions, all of which can + * accept invalidation messages and reach this callback. + * * Freeing now would leave those callers reading freed memory. * Queue it instead; AtEOXact_RI() releases it once no RI check * can be running. @@ -2971,12 +2972,11 @@ ri_fastpath_is_applicable(const RI_ConstraintInfo *riinfo) return false; /* - * The fast path probes the referenced index directly and, for - * single-column keys, uses SK_SEARCHARRAY. A foreign key's referenced - * index need not be a primary key; transformFkeyCheckAttrs() accepts any - * unique index, so an out-of-tree amcanunique access method could reach - * here. Restrict the fast path to btree, which is what the direct probe - * and SK_SEARCHARRAY assume; other access methods fall back to SPI. + * The fast path probes the referenced index directly. A foreign key's + * referenced index need not be a primary key; transformFkeyCheckAttrs() + * accepts any unique index, so an out-of-tree amcanunique access method + * could reach here. Restrict the fast path to btree; other access + * methods fall back to SPI. */ if (!riinfo->pk_index_is_btree) return false; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index bb31ca52c0f..c2ff9acd8ef 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -30,8 +30,6 @@ AddForeignUpdateTargets_function AddrInfo AffixNode AffixNodeData -AfterTriggerBatchCallback -AfterTriggerCallbackItem AfterTriggerEvent AfterTriggerEventChunk AfterTriggerEventData @@ -2518,8 +2516,6 @@ RIX RI_CompareHashEntry RI_CompareKey RI_ConstraintInfo -RI_FastPathEntry -RI_FastPathKey RI_QueryHashEntry RI_QueryKey RTEKind