From 747de1f3675fdb71a86eb2688299f0cb5f174431 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Thu, 3 Sep 2026 19:48:14 +0900 Subject: [PATCH v1 2/2] 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. Remove tests that exercise only the batching implementation and its callback and cache lifetime machinery. Retain tests that continue to exercise the underlying per-row fast path, including validation, scan-key construction, deferred checks, and metadata invalidation. 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/ --- .git-blame-ignore-revs | 3 - doc/src/sgml/release-19.sgml | 4 - src/backend/access/transam/xact.c | 2 - src/backend/commands/trigger.c | 206 +--- src/backend/utils/adt/ri_triggers.c | 940 +----------------- src/include/commands/trigger.h | 24 - .../expected/fk-crosstype-recheck.out | 37 - src/test/isolation/isolation_schedule | 1 - .../isolation/specs/fk-crosstype-recheck.spec | 54 - src/test/regress/expected/foreign_key.out | 347 +------ src/test/regress/expected/triggers.out | 24 - src/test/regress/sql/foreign_key.sql | 305 +----- src/test/regress/sql/triggers.sql | 23 - src/tools/pgindent/typedefs.list | 4 - 14 files changed, 122 insertions(+), 1852 deletions(-) delete mode 100644 src/test/isolation/expected/fk-crosstype-recheck.out delete mode 100644 src/test/isolation/specs/fk-crosstype-recheck.spec diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index a602b47f7c9..b99ba0983c2 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -17,9 +17,6 @@ 3d2f2eb1664e5c823b66a063d2de9a9787970d42 # 2026-08-15 23:16:15 +0900 # pgindent fix for commit 7b7c4a8dcc9 -52d87b42d9bef6a2ca66572fc39e5c1b0f61f5dd # 2026-08-07 17:38:13 +0900 -# Fix indentation issue introduced by commit 291a4bd2ca - b1aeda3ec939c2867e5c3eb4ee7e1bae4768503e # 2026-07-30 09:26:07 +0200 # pgindent fix for 4ee0ccfd diff --git a/doc/src/sgml/release-19.sgml b/doc/src/sgml/release-19.sgml index ba85e9140ec..544bc7f0371 100644 --- a/doc/src/sgml/release-19.sgml +++ b/doc/src/sgml/release-19.sgml @@ -706,10 +706,6 @@ Author: Amit Langote 2026-03-31 [2da86c1ef] Add fast path for foreign key constraint checks Author: Amit Langote 2026-04-01 [e484b0eea] Fix two issues in fast-path FK check introduced by commi -Author: Amit Langote -2026-04-03 [b7b27eb41] Optimize fast-path FK checks with batched index probes -Author: Amit Langote -2026-04-07 [5c54c3ed1] Fix deferred FK check batching introduced by commit b7b2 --> 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 60394fb9934..66f8dbdfe88 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -23,7 +23,6 @@ #include "postgres.h" -#include "access/amapi.h" #include "access/genam.h" #include "access/htup_details.h" #include "access/skey.h" @@ -219,84 +218,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 +226,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 +283,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 +306,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 +416,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 (via ereport) 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); } @@ -2643,13 +2525,13 @@ 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. - * Freeing now would leave those callers reading freed memory. - * Queue it instead; AtEOXact_RI() releases it once no RI check - * can be running. + * copies riinfo->fpmeta into a local (and takes FmgrInfo pointers + * into it) and then runs 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. */ if (riinfo->fpmeta) { @@ -2854,8 +2736,7 @@ ri_PerformCheck(const RI_ConstraintInfo *riinfo, /* * ri_FastPathCheck - * Perform per row FK existence check via direct index probe, - * bypassing SPI. + * Perform FK existence check via direct index probe, bypassing SPI. * * If no matching PK row exists, report the violation via ri_ReportViolation(), * otherwise, the function returns normally. @@ -2887,7 +2768,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 +2832,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 @@ -3388,11 +2874,9 @@ ri_FastPathProbeOne(Relation pk_rel, Relation idx_rel, * Calls table_tuple_lock() directly with handling specific to RI checks. * Returns true if the tuple was successfully locked. * - * If concurrently_updated is not NULL, sets *concurrently_updated to true - * if the locked tuple was reached by following an update chain - * (tmfd.traversed), indicating the caller should recheck the key. Callers - * that compare the locked tuple's key against the value they were looking - * for anyway can pass NULL. + * Sets *concurrently_updated to true if the locked tuple was reached + * by following an update chain (tmfd.traversed), indicating the caller + * should recheck the key. */ static bool ri_LockPKTuple(Relation pk_rel, TupleTableSlot *slot, Snapshot snap, @@ -3402,8 +2886,7 @@ ri_LockPKTuple(Relation pk_rel, TupleTableSlot *slot, Snapshot snap, TM_Result result; int lockflags = TUPLE_LOCK_FLAG_LOCK_UPDATE_IN_PROGRESS; - if (concurrently_updated) - *concurrently_updated = false; + *concurrently_updated = false; if (!IsolationUsesXactSnapshot()) lockflags |= TUPLE_LOCK_FLAG_FIND_LAST_VERSION; @@ -3416,7 +2899,7 @@ ri_LockPKTuple(Relation pk_rel, TupleTableSlot *slot, Snapshot snap, switch (result) { case TM_Ok: - if (tmfd.traversed && concurrently_updated) + if (tmfd.traversed) *concurrently_updated = true; return true; @@ -3483,12 +2966,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; @@ -4322,176 +3804,28 @@ 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. + * 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) { - /* - * 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; @@ -4501,201 +3835,3 @@ AtEOXact_RI(bool isCommit) 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 */ diff --git a/src/test/isolation/expected/fk-crosstype-recheck.out b/src/test/isolation/expected/fk-crosstype-recheck.out deleted file mode 100644 index 875dc856bce..00000000000 --- a/src/test/isolation/expected/fk-crosstype-recheck.out +++ /dev/null @@ -1,37 +0,0 @@ -Parsed test spec with 2 sessions - -starting permutation: s1b s1away s1back s2ins s1c s2sel -step s1b: BEGIN; -step s1away: UPDATE fkct_pk SET k = '2020-06-01' WHERE payload = 'p1'; -step s1back: UPDATE fkct_pk SET k = '2020-01-01' WHERE payload = 'p1'; -step s2ins: INSERT INTO fkct_fk SELECT g, '2020-01-01'::timestamp FROM generate_series(1,2) g; -step s1c: COMMIT; -step s2ins: <... completed> -step s2sel: SELECT k FROM fkct_pk; - k ----------- -01-01-2020 -(1 row) - - -starting permutation: s1b s1away s2ins s1c s2sel -step s1b: BEGIN; -step s1away: UPDATE fkct_pk SET k = '2020-06-01' WHERE payload = 'p1'; -step s2ins: INSERT INTO fkct_fk SELECT g, '2020-01-01'::timestamp FROM generate_series(1,2) g; -step s1c: COMMIT; -step s2ins: <... completed> -ERROR: insert or update on table "fkct_fk" violates foreign key constraint "fkct_fk_t_fkey" -step s2sel: SELECT k FROM fkct_pk; - k ----------- -06-01-2020 -(1 row) - - -starting permutation: s1b s1aways s1backs s2inss s1c -step s1b: BEGIN; -step s1aways: UPDATE fkct_pk_same SET k = '2020-06-01' WHERE payload = 'p1'; -step s1backs: UPDATE fkct_pk_same SET k = '2020-01-01' WHERE payload = 'p1'; -step s2inss: INSERT INTO fkct_fk_same SELECT g, '2020-01-01'::timestamp FROM generate_series(1,2) g; -step s1c: COMMIT; -step s2inss: <... completed> diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 1fcf4e63238..0f2f8463328 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -30,7 +30,6 @@ test: detach-partition-concurrently-2 test: detach-partition-concurrently-3 test: detach-partition-concurrently-4 test: fk-contention -test: fk-crosstype-recheck test: fk-deadlock test: fk-deadlock2 test: fk-partitioned-1 diff --git a/src/test/isolation/specs/fk-crosstype-recheck.spec b/src/test/isolation/specs/fk-crosstype-recheck.spec deleted file mode 100644 index 5d479b8d46c..00000000000 --- a/src/test/isolation/specs/fk-crosstype-recheck.spec +++ /dev/null @@ -1,54 +0,0 @@ -# A foreign key may use a cross-type equality operator: a "date" primary key -# and a "timestamp" referencing column give "=(date,timestamp without time -# zone)", whose left input is the PK type and whose right input is the FK type. -# -# When the referenced row is updated while a check is locking it, the check -# has to re-check against the new version of the row. That re-check must -# still pass each value to the side of the operator that expects it. A date -# counts days and a timestamp counts microseconds, so reading one as the other -# compares two unrelated numbers. Both types are pass-by-value, so nothing -# here turns on how a value is stored, only on which side it is read from. -# -# Below the referenced key is present the whole time -- s1 moves it away and -# puts it back inside one transaction -- so the INSERT must succeed, exactly as -# it does for the same-type case in the second permutation. - -setup -{ - CREATE TABLE fkct_pk (k date PRIMARY KEY, payload text); - CREATE TABLE fkct_fk (id int, t timestamp REFERENCES fkct_pk(k)); - INSERT INTO fkct_pk VALUES ('2020-01-01', 'p1'); - - CREATE TABLE fkct_pk_same (k timestamp PRIMARY KEY, payload text); - CREATE TABLE fkct_fk_same (id int, t timestamp REFERENCES fkct_pk_same(k)); - INSERT INTO fkct_pk_same VALUES ('2020-01-01', 'p1'); -} - -teardown -{ - DROP TABLE fkct_fk, fkct_pk, fkct_fk_same, fkct_pk_same; -} - -session s1 -step s1b { BEGIN; } -step s1away { UPDATE fkct_pk SET k = '2020-06-01' WHERE payload = 'p1'; } -step s1back { UPDATE fkct_pk SET k = '2020-01-01' WHERE payload = 'p1'; } -step s1aways { UPDATE fkct_pk_same SET k = '2020-06-01' WHERE payload = 'p1'; } -step s1backs { UPDATE fkct_pk_same SET k = '2020-01-01' WHERE payload = 'p1'; } -step s1c { COMMIT; } - -# Two rows in one statement, so the checks are batched -- that is what reaches -# the re-check path under test. -session s2 -step s2ins { INSERT INTO fkct_fk SELECT g, '2020-01-01'::timestamp FROM generate_series(1,2) g; } -step s2inss { INSERT INTO fkct_fk_same SELECT g, '2020-01-01'::timestamp FROM generate_series(1,2) g; } -step s2sel { SELECT k FROM fkct_pk; } - -permutation s1b s1away s1back s2ins s1c s2sel - -# The mirror image: s1 leaves the key where it moved it, so the INSERT must -# fail. Making the re-check accept every concurrently updated tuple would -# satisfy the permutation above while breaking this one. -permutation s1b s1away s2ins s1c s2sel - -permutation s1b s1aways s1backs s2inss s1c diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index ac044eb40fa..cd718af83de 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -3557,27 +3557,45 @@ DETAIL: drop cascades to table fkpart13_t1 drop cascades to table fkpart13_t2 drop cascades to table fkpart13_t3 RESET search_path; --- Tests foreign key check fast-path no-cache path. +-- Test the fast-path ALTER TABLE validation path. RLS on the referenced +-- table makes RI_Initial_Check() decline the set-based check, so validation +-- invokes the RI trigger once per referencing row. +CREATE ROLE regress_fk_fastpath_role; CREATE TABLE fp_pk_alter (a int PRIMARY KEY); INSERT INTO fp_pk_alter SELECT generate_series(1, 100); +ALTER TABLE fp_pk_alter ENABLE ROW LEVEL SECURITY; +CREATE POLICY fp_pk_alter_all ON fp_pk_alter + FOR ALL USING (true) WITH CHECK (true); +GRANT REFERENCES, SELECT ON fp_pk_alter TO regress_fk_fastpath_role; CREATE TABLE fp_fk_alter (a int); INSERT INTO fp_fk_alter SELECT generate_series(1, 100); +ALTER TABLE fp_fk_alter OWNER TO regress_fk_fastpath_role; -- Validation path: should succeed +SET ROLE regress_fk_fastpath_role; ALTER TABLE fp_fk_alter ADD FOREIGN KEY (a) REFERENCES fp_pk_alter; INSERT INTO fp_fk_alter VALUES (101); -- should fail (constraint active) ERROR: insert or update on table "fp_fk_alter" violates foreign key constraint "fp_fk_alter_a_fkey" DETAIL: Key (a)=(101) is not present in table "fp_pk_alter". +RESET ROLE; DROP TABLE fp_fk_alter, fp_pk_alter; -- Separate test: validation catches existing violation CREATE TABLE fp_pk_alter2 (a int PRIMARY KEY); INSERT INTO fp_pk_alter2 VALUES (1); +ALTER TABLE fp_pk_alter2 ENABLE ROW LEVEL SECURITY; +CREATE POLICY fp_pk_alter2_all ON fp_pk_alter2 + FOR ALL USING (true) WITH CHECK (true); +GRANT REFERENCES, SELECT ON fp_pk_alter2 TO regress_fk_fastpath_role; CREATE TABLE fp_fk_alter2 (a int); INSERT INTO fp_fk_alter2 VALUES (1), (200); -- 200 has no PK match +ALTER TABLE fp_fk_alter2 OWNER TO regress_fk_fastpath_role; +SET ROLE regress_fk_fastpath_role; ALTER TABLE fp_fk_alter2 ADD FOREIGN KEY (a) REFERENCES fp_pk_alter2; -- should fail ERROR: insert or update on table "fp_fk_alter2" violates foreign key constraint "fp_fk_alter2_a_fkey" DETAIL: Key (a)=(200) is not present in table "fp_pk_alter2". +RESET ROLE; DROP TABLE fp_fk_alter2, fp_pk_alter2; --- Tests that the fast-path handles caching for multiple constraints +DROP ROLE regress_fk_fastpath_role; +-- Tests that the fast-path handles multiple constraints CREATE TABLE fp_pk1 (a int PRIMARY KEY); CREATE TABLE fp_pk2 (b int PRIMARY KEY); INSERT INTO fp_pk1 VALUES (1); @@ -3586,26 +3604,26 @@ CREATE TABLE fp_multi_fk ( a int REFERENCES fp_pk1, b int REFERENCES fp_pk2 ); -INSERT INTO fp_multi_fk VALUES (1, 1); -- two constraints, one batch +INSERT INTO fp_multi_fk VALUES (1, 1); -- two constraints INSERT INTO fp_multi_fk VALUES (1, 2); -- second constraint fails ERROR: insert or update on table "fp_multi_fk" violates foreign key constraint "fp_multi_fk_b_fkey" DETAIL: Key (b)=(2) is not present in table "fp_pk2". DROP TABLE fp_multi_fk, fp_pk1, fp_pk2; --- Test that fast-path cache handles deferred constraints and SET CONSTRAINTS IMMEDIATE +-- Test that fast-path handles deferred constraints and SET CONSTRAINTS IMMEDIATE CREATE TABLE fp_pk_defer (a int PRIMARY KEY); CREATE TABLE fp_fk_defer (a int REFERENCES fp_pk_defer DEFERRABLE INITIALLY DEFERRED); INSERT INTO fp_pk_defer VALUES (1), (2); BEGIN; INSERT INTO fp_fk_defer VALUES (1); INSERT INTO fp_fk_defer VALUES (2); -SET CONSTRAINTS ALL IMMEDIATE; -- fires batch callback here -INSERT INTO fp_fk_defer VALUES (3); -- should fail, also tests that cache was cleaned up +SET CONSTRAINTS ALL IMMEDIATE; -- fires deferred checks here +INSERT INTO fp_fk_defer VALUES (3); -- should fail ERROR: insert or update on table "fp_fk_defer" violates foreign key constraint "fp_fk_defer_a_fkey" DETAIL: Key (a)=(3) is not present in table "fp_pk_defer". COMMIT; DROP TABLE fp_pk_defer, fp_fk_defer; -- A deferred FK check queued while firing deferred triggers at commit must --- not be lost. The RI fast-path flush runs during the deferred firing loop +-- not be lost. The RI fast-path runs during the deferred firing loop -- and can run user cast/equality code whose DML queues a further deferred -- check; that check must still fire. fk_defer_main's deferred fast-path check -- runs a cast whose function inserts a violating row into fk_defer_t2, @@ -3626,8 +3644,7 @@ CREATE TABLE fk_defer_main_pk (id int PRIMARY KEY); INSERT INTO fk_defer_main_pk VALUES (1); CREATE TABLE fk_defer_main (a fk_defer_vch REFERENCES fk_defer_main_pk(id) DEFERRABLE INITIALLY DEFERRED); --- At COMMIT the queued fk_defer_t2 check must fire and report the violation, --- rather than being dropped (which would let the dangling row commit). +-- At COMMIT the queued fk_defer_t2 check must fire and report the violation BEGIN; INSERT INTO fk_defer_main VALUES (row(1)::fk_defer_vch); COMMIT; -- expected: ERROR on fk_defer_t2_a_fkey @@ -3643,25 +3660,6 @@ DROP TABLE fk_defer_main, fk_defer_main_pk, fk_defer_t2, fk_defer_t2_pk; DROP CAST (fk_defer_vch AS int); DROP FUNCTION fk_defer_cast(fk_defer_vch); DROP TYPE fk_defer_vch; --- Subtransaction abort: cached state must be invalidated on ROLLBACK TO -CREATE TABLE fp_pk_subxact (a int PRIMARY KEY); -CREATE TABLE fp_fk_subxact (a int REFERENCES fp_pk_subxact); -INSERT INTO fp_pk_subxact VALUES (1), (2); -BEGIN; -INSERT INTO fp_fk_subxact VALUES (1); -SAVEPOINT sp1; -INSERT INTO fp_fk_subxact VALUES (2); -ROLLBACK TO sp1; -INSERT INTO fp_fk_subxact VALUES (1); -COMMIT; -SELECT * FROM fp_fk_subxact; - a ---- - 1 - 1 -(2 rows) - -DROP TABLE fp_fk_subxact, fp_pk_subxact; -- FK check must see PK rows inserted by earlier AFTER triggers -- firing on the same statement CREATE TABLE fp_pk_cci (a int PRIMARY KEY); @@ -3682,7 +3680,7 @@ NOTICE: fp_auto_pk called NOTICE: fp_auto_pk called DROP TABLE fp_fk_cci, fp_pk_cci; DROP FUNCTION fp_auto_pk; --- Multi-column FK: exercises batched per-row probing with composite keys +-- Multi-column FK: exercises direct per-row probing with composite keys CREATE TABLE fp_pk_multi (a int, b int, PRIMARY KEY (a, b)); INSERT INTO fp_pk_multi SELECT i, i FROM generate_series(1, 100) i; CREATE TABLE fp_fk_multi (x int, a int, b int, @@ -3721,7 +3719,7 @@ INSERT INTO fp_fk_same VALUES (9, 9); -- should fail ERROR: insert or update on table "fp_fk_same" violates foreign key constraint "fp_fk_same_c2_c1_fkey" DETAIL: Key (c2, c1)=(9, 9) is not present in table "fp_pk_same". DROP TABLE fp_fk_same, fp_pk_same; --- Deferred constraint: batch flushed at COMMIT, not at statement end +-- Deferred constraint: deferred check fired at COMMIT, not at statement end CREATE TABLE fp_pk_commit (a int PRIMARY KEY); CREATE TABLE fp_fk_commit (a int REFERENCES fp_pk_commit DEFERRABLE INITIALLY DEFERRED); @@ -3759,15 +3757,7 @@ DETAIL: Key (a)=(999) is not present in table "fp_pk_dom". INSERT INTO fp_fk_dom VALUES (NULL); DROP TABLE fp_fk_dom, fp_pk_dom; DROP DOMAIN fp_int8dom; --- Duplicate FK values: when using the batched SAOP path, every --- row must be recognized as satisfied, not just the first match -CREATE TABLE fp_pk_dup (a int PRIMARY KEY); -INSERT INTO fp_pk_dup VALUES (1); -CREATE TABLE fp_fk_dup (a int REFERENCES fp_pk_dup); -INSERT INTO fp_fk_dup SELECT 1 FROM generate_series(1, 100); -DROP TABLE fp_fk_dup, fp_pk_dup; -- Re-entrant FK fast-path: DML on the same FK table from a cast function --- during a full-batch flush must not corrupt the batch array. CREATE TABLE fp_reentry_pk (id int PRIMARY KEY); INSERT INTO fp_reentry_pk VALUES (1), (2); CREATE TYPE fp_vch AS (v int); @@ -3781,78 +3771,26 @@ END$$; CREATE CAST (fp_vch AS int) WITH FUNCTION fp_vcast(fp_vch) AS IMPLICIT; CREATE TABLE fp_reentry_fk (a fp_vch REFERENCES fp_reentry_pk (id)); --- Fill exactly one batch so the flush fires; the cast re-enters with DML --- on the same FK and must take the per-row path. -INSERT INTO fp_reentry_fk SELECT row(1)::fp_vch FROM generate_series(1, 64); +-- cast re-enters with DML on the same FK +INSERT INTO fp_reentry_fk SELECT row(1)::fp_vch FROM generate_series(1, 2); SELECT a, count(*) FROM fp_reentry_fk GROUP BY a ORDER BY a; a | count -----+------- - (1) | 64 - (2) | 64 + (1) | 2 + (2) | 2 (2 rows) DROP TABLE fp_reentry_fk, fp_reentry_pk; DROP CAST (fp_vch AS int); DROP FUNCTION fp_vcast(fp_vch); DROP TYPE fp_vch; --- Flush error caught by a savepoint must leave the entry empty and reusable. -CREATE TABLE fp_reentry_pk2 (id int PRIMARY KEY); -INSERT INTO fp_reentry_pk2 VALUES (1); -CREATE TABLE fp_reentry_fk2 (a int REFERENCES fp_reentry_pk2 (id)); -DO $$ -BEGIN - -- A batch containing a violating row; the flush reports the violation. - BEGIN - INSERT INTO fp_reentry_fk2 SELECT CASE WHEN g = 32 THEN 999 ELSE 1 END - FROM generate_series(1, 64) g; - EXCEPTION WHEN foreign_key_violation THEN - RAISE NOTICE 'caught fk violation'; - END; - - -- Reuse the same FK with a full batch in the same transaction. The - -- entry must be empty after the caught violation: no stale rows from the - -- rolled-back batch (in particular no 999), and no array overflow. - INSERT INTO fp_reentry_fk2 SELECT 1 FROM generate_series(1, 64); -END$$; -NOTICE: caught fk violation -SELECT count(*), max(a) FROM fp_reentry_fk2; -- 64 rows, max 1 - count | max --------+----- - 64 | 1 -(1 row) - -DROP TABLE fp_reentry_fk2, fp_reentry_pk2; --- Subtransaction abort during after-trigger firing must not drop FK checks --- for rows buffered earlier in the same statement. Batching is confined to --- the top transaction level and the buffered batch is no longer discarded on --- subxact abort, so the violating rows are detected. -CREATE TABLE fp_subxact_pk (id int PRIMARY KEY); -INSERT INTO fp_subxact_pk SELECT g FROM generate_series(1, 10) g; -CREATE TABLE fp_subxact_fk (a int, tag text); -ALTER TABLE fp_subxact_fk ADD CONSTRAINT fp_subxact_fk_fkey - FOREIGN KEY (a) REFERENCES fp_subxact_pk (id); -CREATE FUNCTION fp_abort_subxact() RETURNS trigger LANGUAGE plpgsql AS $$ -BEGIN - IF NEW.tag = 'boom' THEN - BEGIN PERFORM 1/0; EXCEPTION WHEN division_by_zero THEN NULL; END; - END IF; - RETURN NEW; -END$$; -CREATE TRIGGER fp_subxact_trg AFTER INSERT ON fp_subxact_fk - FOR EACH ROW EXECUTE FUNCTION fp_abort_subxact(); -INSERT INTO fp_subxact_fk VALUES (999, 'bad'), (0, 'boom'), (1, 'ok'); -ERROR: insert or update on table "fp_subxact_fk" violates foreign key constraint "fp_subxact_fk_fkey" -DETAIL: Key (a)=(999) is not present in table "fp_subxact_pk". -DROP TRIGGER fp_subxact_trg ON fp_subxact_fk; -DROP FUNCTION fp_abort_subxact(); -DROP TABLE fp_subxact_fk, fp_subxact_pk; -- --- Cache invalidation arriving in the middle of a fast-path batch flush. +-- Cache invalidation arriving during a fast-path check. -- --- A cross-type foreign key runs the user's cast function once per key per --- buffered row, inside ri_FastPathFlushLoop(). A cast that performs DDL --- raises an invalidation there, which detaches the constraint's fast-path --- metadata while the flush is still using it. +-- A cross-type foreign key runs the user's cast function while building its +-- index scan keys. A cast that performs DDL raises an invalidation there, +-- detaching the constraint's fast-path metadata while the check is still +-- using it. -- CREATE TYPE fkint; CREATE FUNCTION fkint_in(cstring) RETURNS fkint @@ -3865,7 +3803,7 @@ LINE 1: CREATE FUNCTION fkint_out(fkint) RETURNS cstring ^ CREATE TYPE fkint (INPUT = fkint_in, OUTPUT = fkint_out, LIKE = int4); -- Renames the constraint the first time it is called, and so raises an --- invalidation partway through the flush. Guarded on the catalog so the +-- invalidation partway through the check. Guarded on the catalog so the -- second and later calls are no-ops. CREATE FUNCTION fkint_to_int4(fkint) RETURNS int4 AS $$ BEGIN @@ -3880,12 +3818,12 @@ END $$ LANGUAGE plpgsql; CREATE CAST (fkint AS int4) WITH FUNCTION fkint_to_int4(fkint) AS IMPLICIT; CREATE TABLE pktable_inval (a int4, b int4, PRIMARY KEY (a, b)); INSERT INTO pktable_inval VALUES (1, 1), (2, 2); --- Multi-column FK, so the flush takes the per-row loop rather than the --- array path; cross-type on column a, so the cast above is invoked. +-- Cross-type on column a, so the cast above is invoked. CREATE TABLE fktable_inval (a fkint, b int4, CONSTRAINT fktable_inval_fk FOREIGN KEY (a, b) REFERENCES pktable_inval (a, b)); --- More than one row, so the flush is still running after the invalidation. +-- The first row invalidates the metadata; the second check must repopulate +-- and use it safely. INSERT INTO fktable_inval VALUES ('1', 1), ('2', 2); -- Confirms the cast actually ran and raised the invalidation. Without this -- the insert above could pass merely by not exercising the path at all. @@ -3910,206 +3848,3 @@ DROP TYPE fkint CASCADE; NOTICE: drop cascades to 2 other objects DETAIL: drop cascades to function fkint_in(cstring) drop cascades to function fkint_out(fkint) --- Stranded firing state must not misroute ALTER TABLE ... ADD FOREIGN KEY --- validation into the batched fast path. A caught FK-check error inside a --- subtransaction leaves firing_depth set (its decrement is skipped); a --- following ALTER whose validation runs per-row (forced here by RLS on the --- referenced table, so RI_Initial_Check() bails) would then be wrongly treated --- as running inside trigger firing, batched, and never flushed (a utility --- command has no AfterTriggerEndQuery), silently validating a violating row. -CREATE ROLE regress_fpav_role; -CREATE TABLE fpav_pk (id int PRIMARY KEY); -INSERT INTO fpav_pk VALUES (1); -ALTER TABLE fpav_pk ENABLE ROW LEVEL SECURITY; -CREATE POLICY fpav_pk_all ON fpav_pk FOR ALL USING (true) WITH CHECK (true); -GRANT REFERENCES, SELECT ON fpav_pk TO regress_fpav_role; -CREATE TABLE fpav_fk (a int); -INSERT INTO fpav_fk VALUES (1), (99); -ALTER TABLE fpav_fk OWNER TO regress_fpav_role; -CREATE TABLE fpav_cv_pk (id int PRIMARY KEY); -INSERT INTO fpav_cv_pk VALUES (1); -CREATE TABLE fpav_cv_fk (a int REFERENCES fpav_cv_pk(id)); -GRANT INSERT ON fpav_cv_fk TO regress_fpav_role; -GRANT SELECT, INSERT ON fpav_cv_pk TO regress_fpav_role; -SET ROLE regress_fpav_role; -BEGIN; --- Caught FK violation: leaves firing_depth set if it is not restored. -DO $$ -BEGIN - BEGIN - INSERT INTO fpav_cv_fk VALUES (999); - EXCEPTION WHEN foreign_key_violation THEN - NULL; - END; -END$$; --- Must ERROR on the violating row (99), not silently validate it. -ALTER TABLE fpav_fk ADD CONSTRAINT fpav_fk_fkey - FOREIGN KEY (a) REFERENCES fpav_pk (id); -ERROR: insert or update on table "fpav_fk" violates foreign key constraint "fpav_fk_fkey" -DETAIL: Key (a)=(99) is not present in table "fpav_pk". -ROLLBACK; -RESET ROLE; -DROP TABLE fpav_fk, fpav_pk, fpav_cv_fk, fpav_cv_pk; -DROP ROLE regress_fpav_role; --- Re-entrant fast-path check inside a committing subtransaction. An AFTER --- trigger on one FK table runs FK DML on a second FK table inside a PL/pgSQL --- BEGIN ... EXCEPTION block, so the inner check batches in its own --- trigger-firing cycle nested in the outer check's. The inner cycle must --- register its own end-of-batch callback and flush -- otherwise its FK check --- is skipped (an orphan commits) and its relations leak. -CREATE TABLE fp_inner_pk (id int PRIMARY KEY); -INSERT INTO fp_inner_pk VALUES (1); -CREATE TABLE fp_inner_fk (a int REFERENCES fp_inner_pk (id)); -CREATE TABLE fp_outer_pk (id int PRIMARY KEY); -INSERT INTO fp_outer_pk SELECT g FROM generate_series(1, 64) g; -CREATE FUNCTION fp_reentry_subxact() RETURNS trigger LANGUAGE plpgsql AS $$ -BEGIN - IF NEW.a = 32 THEN - BEGIN - INSERT INTO fp_inner_fk VALUES (999); -- violates; must be caught - EXCEPTION WHEN foreign_key_violation THEN - NULL; - END; - END IF; - RETURN NEW; -END$$; -CREATE TABLE fp_outer_fk (a int REFERENCES fp_outer_pk (id)); -CREATE TRIGGER fp_reentry_subxact_trg AFTER INSERT ON fp_outer_fk - FOR EACH ROW EXECUTE FUNCTION fp_reentry_subxact(); -INSERT INTO fp_outer_fk SELECT g FROM generate_series(1, 64) g; -SELECT count(*) AS outer_rows FROM fp_outer_fk; -- 64, outer batch intact - outer_rows ------------- - 64 -(1 row) - -SELECT count(*) AS inner_rows FROM fp_inner_fk; -- 0, inner check caught - inner_rows ------------- - 0 -(1 row) - -DROP TRIGGER fp_reentry_subxact_trg ON fp_outer_fk; -DROP FUNCTION fp_reentry_subxact(); -DROP TABLE fp_outer_fk, fp_outer_pk, fp_inner_fk, fp_inner_pk; --- A nested trigger-firing cycle that checks the same constraint must use a --- separate cache entry. The inner violation is caught by its subtransaction, --- while the valid outer row remains buffered and is checked normally. -CREATE TABLE fp_same_pk (id int PRIMARY KEY); -INSERT INTO fp_same_pk VALUES (1); -CREATE TABLE fp_same_fk (a int REFERENCES fp_same_pk (id)); -CREATE FUNCTION fp_reentry_same_constraint() RETURNS trigger LANGUAGE plpgsql AS $$ -BEGIN - IF NEW.a = 1 THEN - BEGIN - INSERT INTO fp_same_fk VALUES (999); - EXCEPTION WHEN foreign_key_violation THEN - NULL; - END; - END IF; - RETURN NEW; -END$$; -CREATE TRIGGER fp_reentry_same_constraint_trg AFTER INSERT ON fp_same_fk - FOR EACH ROW EXECUTE FUNCTION fp_reentry_same_constraint(); -INSERT INTO fp_same_fk VALUES (1); -SELECT * FROM fp_same_fk; - a ---- - 1 -(1 row) - -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; --- 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/expected/triggers.out b/src/test/regress/expected/triggers.out index 8fcb33ac81a..511e7cfb6ce 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -3644,27 +3644,3 @@ drop table defer_trig; drop function whoami(); drop role regress_fn_owner; drop role regress_caller; --- --- Test a recursive AFTER ROW trigger that nests after-trigger query levels --- deeply enough to grow query_stack mid-fire. Outer levels then resume their --- post-loop cleanup against the relocated stack. --- -create table trigger_recursive (id int); -create function trigger_recursive_fn() returns trigger language plpgsql as $$ -begin - if new.id < 10 then - insert into trigger_recursive values (new.id + 1); - end if; - return new; -end$$; -create trigger trigger_recursive after insert on trigger_recursive - for each row execute function trigger_recursive_fn(); -insert into trigger_recursive values (1); -select count(*) from trigger_recursive; - count -------- - 10 -(1 row) - -drop table trigger_recursive; -drop function trigger_recursive_fn(); diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index a93e81b42bc..5ff9f0440b1 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -2536,25 +2536,43 @@ WITH cte AS ( DROP SCHEMA fkpart13 CASCADE; RESET search_path; --- Tests foreign key check fast-path no-cache path. +-- Test the fast-path ALTER TABLE validation path. RLS on the referenced +-- table makes RI_Initial_Check() decline the set-based check, so validation +-- invokes the RI trigger once per referencing row. +CREATE ROLE regress_fk_fastpath_role; CREATE TABLE fp_pk_alter (a int PRIMARY KEY); INSERT INTO fp_pk_alter SELECT generate_series(1, 100); +ALTER TABLE fp_pk_alter ENABLE ROW LEVEL SECURITY; +CREATE POLICY fp_pk_alter_all ON fp_pk_alter + FOR ALL USING (true) WITH CHECK (true); +GRANT REFERENCES, SELECT ON fp_pk_alter TO regress_fk_fastpath_role; CREATE TABLE fp_fk_alter (a int); INSERT INTO fp_fk_alter SELECT generate_series(1, 100); +ALTER TABLE fp_fk_alter OWNER TO regress_fk_fastpath_role; -- Validation path: should succeed +SET ROLE regress_fk_fastpath_role; ALTER TABLE fp_fk_alter ADD FOREIGN KEY (a) REFERENCES fp_pk_alter; INSERT INTO fp_fk_alter VALUES (101); -- should fail (constraint active) +RESET ROLE; DROP TABLE fp_fk_alter, fp_pk_alter; -- Separate test: validation catches existing violation CREATE TABLE fp_pk_alter2 (a int PRIMARY KEY); INSERT INTO fp_pk_alter2 VALUES (1); +ALTER TABLE fp_pk_alter2 ENABLE ROW LEVEL SECURITY; +CREATE POLICY fp_pk_alter2_all ON fp_pk_alter2 + FOR ALL USING (true) WITH CHECK (true); +GRANT REFERENCES, SELECT ON fp_pk_alter2 TO regress_fk_fastpath_role; CREATE TABLE fp_fk_alter2 (a int); INSERT INTO fp_fk_alter2 VALUES (1), (200); -- 200 has no PK match +ALTER TABLE fp_fk_alter2 OWNER TO regress_fk_fastpath_role; +SET ROLE regress_fk_fastpath_role; ALTER TABLE fp_fk_alter2 ADD FOREIGN KEY (a) REFERENCES fp_pk_alter2; -- should fail +RESET ROLE; DROP TABLE fp_fk_alter2, fp_pk_alter2; +DROP ROLE regress_fk_fastpath_role; --- Tests that the fast-path handles caching for multiple constraints +-- Tests that the fast-path handles multiple constraints CREATE TABLE fp_pk1 (a int PRIMARY KEY); CREATE TABLE fp_pk2 (b int PRIMARY KEY); INSERT INTO fp_pk1 VALUES (1); @@ -2563,11 +2581,11 @@ CREATE TABLE fp_multi_fk ( a int REFERENCES fp_pk1, b int REFERENCES fp_pk2 ); -INSERT INTO fp_multi_fk VALUES (1, 1); -- two constraints, one batch +INSERT INTO fp_multi_fk VALUES (1, 1); -- two constraints INSERT INTO fp_multi_fk VALUES (1, 2); -- second constraint fails DROP TABLE fp_multi_fk, fp_pk1, fp_pk2; --- Test that fast-path cache handles deferred constraints and SET CONSTRAINTS IMMEDIATE +-- Test that fast-path handles deferred constraints and SET CONSTRAINTS IMMEDIATE CREATE TABLE fp_pk_defer (a int PRIMARY KEY); CREATE TABLE fp_fk_defer (a int REFERENCES fp_pk_defer DEFERRABLE INITIALLY DEFERRED); INSERT INTO fp_pk_defer VALUES (1), (2); @@ -2575,13 +2593,13 @@ INSERT INTO fp_pk_defer VALUES (1), (2); BEGIN; INSERT INTO fp_fk_defer VALUES (1); INSERT INTO fp_fk_defer VALUES (2); -SET CONSTRAINTS ALL IMMEDIATE; -- fires batch callback here -INSERT INTO fp_fk_defer VALUES (3); -- should fail, also tests that cache was cleaned up +SET CONSTRAINTS ALL IMMEDIATE; -- fires deferred checks here +INSERT INTO fp_fk_defer VALUES (3); -- should fail COMMIT; DROP TABLE fp_pk_defer, fp_fk_defer; -- A deferred FK check queued while firing deferred triggers at commit must --- not be lost. The RI fast-path flush runs during the deferred firing loop +-- not be lost. The RI fast-path runs during the deferred firing loop -- and can run user cast/equality code whose DML queues a further deferred -- check; that check must still fire. fk_defer_main's deferred fast-path check -- runs a cast whose function inserts a violating row into fk_defer_t2, @@ -2602,8 +2620,7 @@ CREATE TABLE fk_defer_main_pk (id int PRIMARY KEY); INSERT INTO fk_defer_main_pk VALUES (1); CREATE TABLE fk_defer_main (a fk_defer_vch REFERENCES fk_defer_main_pk(id) DEFERRABLE INITIALLY DEFERRED); --- At COMMIT the queued fk_defer_t2 check must fire and report the violation, --- rather than being dropped (which would let the dangling row commit). +-- At COMMIT the queued fk_defer_t2 check must fire and report the violation BEGIN; INSERT INTO fk_defer_main VALUES (row(1)::fk_defer_vch); COMMIT; -- expected: ERROR on fk_defer_t2_a_fkey @@ -2618,20 +2635,6 @@ DROP CAST (fk_defer_vch AS int); DROP FUNCTION fk_defer_cast(fk_defer_vch); DROP TYPE fk_defer_vch; --- Subtransaction abort: cached state must be invalidated on ROLLBACK TO -CREATE TABLE fp_pk_subxact (a int PRIMARY KEY); -CREATE TABLE fp_fk_subxact (a int REFERENCES fp_pk_subxact); -INSERT INTO fp_pk_subxact VALUES (1), (2); -BEGIN; -INSERT INTO fp_fk_subxact VALUES (1); -SAVEPOINT sp1; -INSERT INTO fp_fk_subxact VALUES (2); -ROLLBACK TO sp1; -INSERT INTO fp_fk_subxact VALUES (1); -COMMIT; -SELECT * FROM fp_fk_subxact; -DROP TABLE fp_fk_subxact, fp_pk_subxact; - -- FK check must see PK rows inserted by earlier AFTER triggers -- firing on the same statement CREATE TABLE fp_pk_cci (a int PRIMARY KEY); @@ -2654,7 +2657,7 @@ INSERT INTO fp_fk_cci VALUES (1), (2), (3); DROP TABLE fp_fk_cci, fp_pk_cci; DROP FUNCTION fp_auto_pk; --- Multi-column FK: exercises batched per-row probing with composite keys +-- Multi-column FK: exercises direct per-row probing with composite keys CREATE TABLE fp_pk_multi (a int, b int, PRIMARY KEY (a, b)); INSERT INTO fp_pk_multi SELECT i, i FROM generate_series(1, 100) i; CREATE TABLE fp_fk_multi (x int, a int, b int, @@ -2690,7 +2693,7 @@ INSERT INTO fp_fk_same VALUES (1, 2); -- should succeed INSERT INTO fp_fk_same VALUES (9, 9); -- should fail DROP TABLE fp_fk_same, fp_pk_same; --- Deferred constraint: batch flushed at COMMIT, not at statement end +-- Deferred constraint: deferred check fired at COMMIT, not at statement end CREATE TABLE fp_pk_commit (a int PRIMARY KEY); CREATE TABLE fp_fk_commit (a int REFERENCES fp_pk_commit DEFERRABLE INITIALLY DEFERRED); @@ -2725,16 +2728,7 @@ INSERT INTO fp_fk_dom VALUES (NULL); DROP TABLE fp_fk_dom, fp_pk_dom; DROP DOMAIN fp_int8dom; --- Duplicate FK values: when using the batched SAOP path, every --- row must be recognized as satisfied, not just the first match -CREATE TABLE fp_pk_dup (a int PRIMARY KEY); -INSERT INTO fp_pk_dup VALUES (1); -CREATE TABLE fp_fk_dup (a int REFERENCES fp_pk_dup); -INSERT INTO fp_fk_dup SELECT 1 FROM generate_series(1, 100); -DROP TABLE fp_fk_dup, fp_pk_dup; - -- Re-entrant FK fast-path: DML on the same FK table from a cast function --- during a full-batch flush must not corrupt the batch array. CREATE TABLE fp_reentry_pk (id int PRIMARY KEY); INSERT INTO fp_reentry_pk VALUES (1), (2); CREATE TYPE fp_vch AS (v int); @@ -2748,67 +2742,21 @@ END$$; CREATE CAST (fp_vch AS int) WITH FUNCTION fp_vcast(fp_vch) AS IMPLICIT; CREATE TABLE fp_reentry_fk (a fp_vch REFERENCES fp_reentry_pk (id)); --- Fill exactly one batch so the flush fires; the cast re-enters with DML --- on the same FK and must take the per-row path. -INSERT INTO fp_reentry_fk SELECT row(1)::fp_vch FROM generate_series(1, 64); +-- cast re-enters with DML on the same FK +INSERT INTO fp_reentry_fk SELECT row(1)::fp_vch FROM generate_series(1, 2); SELECT a, count(*) FROM fp_reentry_fk GROUP BY a ORDER BY a; DROP TABLE fp_reentry_fk, fp_reentry_pk; DROP CAST (fp_vch AS int); DROP FUNCTION fp_vcast(fp_vch); DROP TYPE fp_vch; --- Flush error caught by a savepoint must leave the entry empty and reusable. -CREATE TABLE fp_reentry_pk2 (id int PRIMARY KEY); -INSERT INTO fp_reentry_pk2 VALUES (1); -CREATE TABLE fp_reentry_fk2 (a int REFERENCES fp_reentry_pk2 (id)); -DO $$ -BEGIN - -- A batch containing a violating row; the flush reports the violation. - BEGIN - INSERT INTO fp_reentry_fk2 SELECT CASE WHEN g = 32 THEN 999 ELSE 1 END - FROM generate_series(1, 64) g; - EXCEPTION WHEN foreign_key_violation THEN - RAISE NOTICE 'caught fk violation'; - END; - - -- Reuse the same FK with a full batch in the same transaction. The - -- entry must be empty after the caught violation: no stale rows from the - -- rolled-back batch (in particular no 999), and no array overflow. - INSERT INTO fp_reentry_fk2 SELECT 1 FROM generate_series(1, 64); -END$$; -SELECT count(*), max(a) FROM fp_reentry_fk2; -- 64 rows, max 1 -DROP TABLE fp_reentry_fk2, fp_reentry_pk2; - --- Subtransaction abort during after-trigger firing must not drop FK checks --- for rows buffered earlier in the same statement. Batching is confined to --- the top transaction level and the buffered batch is no longer discarded on --- subxact abort, so the violating rows are detected. -CREATE TABLE fp_subxact_pk (id int PRIMARY KEY); -INSERT INTO fp_subxact_pk SELECT g FROM generate_series(1, 10) g; -CREATE TABLE fp_subxact_fk (a int, tag text); -ALTER TABLE fp_subxact_fk ADD CONSTRAINT fp_subxact_fk_fkey - FOREIGN KEY (a) REFERENCES fp_subxact_pk (id); -CREATE FUNCTION fp_abort_subxact() RETURNS trigger LANGUAGE plpgsql AS $$ -BEGIN - IF NEW.tag = 'boom' THEN - BEGIN PERFORM 1/0; EXCEPTION WHEN division_by_zero THEN NULL; END; - END IF; - RETURN NEW; -END$$; -CREATE TRIGGER fp_subxact_trg AFTER INSERT ON fp_subxact_fk - FOR EACH ROW EXECUTE FUNCTION fp_abort_subxact(); -INSERT INTO fp_subxact_fk VALUES (999, 'bad'), (0, 'boom'), (1, 'ok'); -DROP TRIGGER fp_subxact_trg ON fp_subxact_fk; -DROP FUNCTION fp_abort_subxact(); -DROP TABLE fp_subxact_fk, fp_subxact_pk; - -- --- Cache invalidation arriving in the middle of a fast-path batch flush. +-- Cache invalidation arriving during a fast-path check. -- --- A cross-type foreign key runs the user's cast function once per key per --- buffered row, inside ri_FastPathFlushLoop(). A cast that performs DDL --- raises an invalidation there, which detaches the constraint's fast-path --- metadata while the flush is still using it. +-- A cross-type foreign key runs the user's cast function while building its +-- index scan keys. A cast that performs DDL raises an invalidation there, +-- detaching the constraint's fast-path metadata while the check is still +-- using it. -- CREATE TYPE fkint; CREATE FUNCTION fkint_in(cstring) RETURNS fkint @@ -2818,7 +2766,7 @@ CREATE FUNCTION fkint_out(fkint) RETURNS cstring CREATE TYPE fkint (INPUT = fkint_in, OUTPUT = fkint_out, LIKE = int4); -- Renames the constraint the first time it is called, and so raises an --- invalidation partway through the flush. Guarded on the catalog so the +-- invalidation partway through the check. Guarded on the catalog so the -- second and later calls are no-ops. CREATE FUNCTION fkint_to_int4(fkint) RETURNS int4 AS $$ BEGIN @@ -2836,13 +2784,13 @@ CREATE CAST (fkint AS int4) WITH FUNCTION fkint_to_int4(fkint) AS IMPLICIT; CREATE TABLE pktable_inval (a int4, b int4, PRIMARY KEY (a, b)); INSERT INTO pktable_inval VALUES (1, 1), (2, 2); --- Multi-column FK, so the flush takes the per-row loop rather than the --- array path; cross-type on column a, so the cast above is invoked. +-- Cross-type on column a, so the cast above is invoked. CREATE TABLE fktable_inval (a fkint, b int4, CONSTRAINT fktable_inval_fk FOREIGN KEY (a, b) REFERENCES pktable_inval (a, b)); --- More than one row, so the flush is still running after the invalidation. +-- The first row invalidates the metadata; the second check must repopulate +-- and use it safely. INSERT INTO fktable_inval VALUES ('1', 1), ('2', 2); -- Confirms the cast actually ran and raised the invalidation. Without this @@ -2857,178 +2805,3 @@ DROP TABLE pktable_inval; DROP CAST (fkint AS int4); DROP FUNCTION fkint_to_int4(fkint); DROP TYPE fkint CASCADE; - --- Stranded firing state must not misroute ALTER TABLE ... ADD FOREIGN KEY --- validation into the batched fast path. A caught FK-check error inside a --- subtransaction leaves firing_depth set (its decrement is skipped); a --- following ALTER whose validation runs per-row (forced here by RLS on the --- referenced table, so RI_Initial_Check() bails) would then be wrongly treated --- as running inside trigger firing, batched, and never flushed (a utility --- command has no AfterTriggerEndQuery), silently validating a violating row. -CREATE ROLE regress_fpav_role; -CREATE TABLE fpav_pk (id int PRIMARY KEY); -INSERT INTO fpav_pk VALUES (1); -ALTER TABLE fpav_pk ENABLE ROW LEVEL SECURITY; -CREATE POLICY fpav_pk_all ON fpav_pk FOR ALL USING (true) WITH CHECK (true); -GRANT REFERENCES, SELECT ON fpav_pk TO regress_fpav_role; -CREATE TABLE fpav_fk (a int); -INSERT INTO fpav_fk VALUES (1), (99); -ALTER TABLE fpav_fk OWNER TO regress_fpav_role; -CREATE TABLE fpav_cv_pk (id int PRIMARY KEY); -INSERT INTO fpav_cv_pk VALUES (1); -CREATE TABLE fpav_cv_fk (a int REFERENCES fpav_cv_pk(id)); -GRANT INSERT ON fpav_cv_fk TO regress_fpav_role; -GRANT SELECT, INSERT ON fpav_cv_pk TO regress_fpav_role; -SET ROLE regress_fpav_role; -BEGIN; --- Caught FK violation: leaves firing_depth set if it is not restored. -DO $$ -BEGIN - BEGIN - INSERT INTO fpav_cv_fk VALUES (999); - EXCEPTION WHEN foreign_key_violation THEN - NULL; - END; -END$$; --- Must ERROR on the violating row (99), not silently validate it. -ALTER TABLE fpav_fk ADD CONSTRAINT fpav_fk_fkey - FOREIGN KEY (a) REFERENCES fpav_pk (id); -ROLLBACK; -RESET ROLE; -DROP TABLE fpav_fk, fpav_pk, fpav_cv_fk, fpav_cv_pk; -DROP ROLE regress_fpav_role; - --- Re-entrant fast-path check inside a committing subtransaction. An AFTER --- trigger on one FK table runs FK DML on a second FK table inside a PL/pgSQL --- BEGIN ... EXCEPTION block, so the inner check batches in its own --- trigger-firing cycle nested in the outer check's. The inner cycle must --- register its own end-of-batch callback and flush -- otherwise its FK check --- is skipped (an orphan commits) and its relations leak. -CREATE TABLE fp_inner_pk (id int PRIMARY KEY); -INSERT INTO fp_inner_pk VALUES (1); -CREATE TABLE fp_inner_fk (a int REFERENCES fp_inner_pk (id)); -CREATE TABLE fp_outer_pk (id int PRIMARY KEY); -INSERT INTO fp_outer_pk SELECT g FROM generate_series(1, 64) g; -CREATE FUNCTION fp_reentry_subxact() RETURNS trigger LANGUAGE plpgsql AS $$ -BEGIN - IF NEW.a = 32 THEN - BEGIN - INSERT INTO fp_inner_fk VALUES (999); -- violates; must be caught - EXCEPTION WHEN foreign_key_violation THEN - NULL; - END; - END IF; - RETURN NEW; -END$$; -CREATE TABLE fp_outer_fk (a int REFERENCES fp_outer_pk (id)); -CREATE TRIGGER fp_reentry_subxact_trg AFTER INSERT ON fp_outer_fk - FOR EACH ROW EXECUTE FUNCTION fp_reentry_subxact(); -INSERT INTO fp_outer_fk SELECT g FROM generate_series(1, 64) g; -SELECT count(*) AS outer_rows FROM fp_outer_fk; -- 64, outer batch intact -SELECT count(*) AS inner_rows FROM fp_inner_fk; -- 0, inner check caught -DROP TRIGGER fp_reentry_subxact_trg ON fp_outer_fk; -DROP FUNCTION fp_reentry_subxact(); -DROP TABLE fp_outer_fk, fp_outer_pk, fp_inner_fk, fp_inner_pk; - --- A nested trigger-firing cycle that checks the same constraint must use a --- separate cache entry. The inner violation is caught by its subtransaction, --- while the valid outer row remains buffered and is checked normally. -CREATE TABLE fp_same_pk (id int PRIMARY KEY); -INSERT INTO fp_same_pk VALUES (1); -CREATE TABLE fp_same_fk (a int REFERENCES fp_same_pk (id)); -CREATE FUNCTION fp_reentry_same_constraint() RETURNS trigger LANGUAGE plpgsql AS $$ -BEGIN - IF NEW.a = 1 THEN - BEGIN - INSERT INTO fp_same_fk VALUES (999); - EXCEPTION WHEN foreign_key_violation THEN - NULL; - END; - END IF; - RETURN NEW; -END$$; -CREATE TRIGGER fp_reentry_same_constraint_trg AFTER INSERT ON fp_same_fk - FOR EACH ROW EXECUTE FUNCTION fp_reentry_same_constraint(); -INSERT INTO fp_same_fk VALUES (1); -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; - --- 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/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index 2285e90110e..ea39817ee3d 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -2788,26 +2788,3 @@ drop table defer_trig; drop function whoami(); drop role regress_fn_owner; drop role regress_caller; - --- --- Test a recursive AFTER ROW trigger that nests after-trigger query levels --- deeply enough to grow query_stack mid-fire. Outer levels then resume their --- post-loop cleanup against the relocated stack. --- -create table trigger_recursive (id int); -create function trigger_recursive_fn() returns trigger language plpgsql as $$ -begin - if new.id < 10 then - insert into trigger_recursive values (new.id + 1); - end if; - return new; -end$$; - -create trigger trigger_recursive after insert on trigger_recursive - for each row execute function trigger_recursive_fn(); - -insert into trigger_recursive values (1); -select count(*) from trigger_recursive; - -drop table trigger_recursive; -drop function trigger_recursive_fn(); 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 -- 2.47.3