From 94bbd92222c2aac3c07fb84e769eb41435356916 Mon Sep 17 00:00:00 2001 From: Nik Samokhvalov Date: Thu, 10 Sep 2026 15:44:08 -0700 Subject: [PATCH 2/2] Invalidate RI call information when casts change Cast replacement need not modify pg_constraint, so the constraint-cache callback does not refresh the cast functions cached by RI comparisons and fast-path foreign-key checks. Dropping the old function after an equivalent replacement can make a valid insert fail with a stale function OID. Invalidate both caches on CASTSOURCETARGET changes. Keep comparison call information separate from its hash entry and defer releasing invalidated objects until transaction end, as already done for fast-path metadata. A cast can run DDL and reenter RI checks, so invalidation must not free or overwrite call information still used by an outer comparison. This retains function-local caching without adding per-row function-info copies. Keep unfinished fast-path metadata in a transaction-owned context until construction succeeds, so failed rebuilds do not leak backend memory. Add regression coverage for replacement after INSERT/UPDATE cache warmup, invalid-key rejection, rollback restoring the old cast, and invalidation with a nested RI check during a comparison. Check that failed rebuilds release their provisional contexts on subtransaction abort. --- src/backend/utils/adt/ri_triggers.c | 137 +++++++++++++++------- src/test/regress/expected/foreign_key.out | 100 ++++++++++++++++ src/test/regress/sql/foreign_key.sql | 85 ++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 4 files changed, 281 insertions(+), 42 deletions(-) diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 8c8edc1..6abd381 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -164,7 +164,7 @@ typedef struct FastPathMeta * fn_mcxt for the cached FmgrInfos above. Cast and equality functions * (e.g. record_eq()) use fn_mcxt as scratch space, caching state there * and keeping a pointer to it in FmgrInfo.fn_extra. Give them a context - * of their own, created with this struct and destroyed with it in + * of their own, which also owns this struct and is destroyed by * AtEOXact_RI(). * * Note this context must not be reset while the FmgrInfos remain in use, @@ -207,15 +207,25 @@ typedef struct RI_CompareKey Oid typeid; /* the data type to apply it to */ } RI_CompareKey; +/* + * Cached call information is detached on invalidation, but kept until the end + * of the transaction in case an active comparison still references it. + */ +typedef struct RI_CompareInfo +{ + FmgrInfo eq_opr_finfo; /* call info for equality fn */ + FmgrInfo cast_func_finfo; /* in case we must coerce input */ + MemoryContext context; + struct RI_CompareInfo *next_dead; +} RI_CompareInfo; + /* * RI_CompareHashEntry */ typedef struct RI_CompareHashEntry { RI_CompareKey key; - bool valid; /* successfully initialized? */ - FmgrInfo eq_opr_finfo; /* call info for equality fn */ - FmgrInfo cast_func_finfo; /* in case we must coerce input */ + RI_CompareInfo *info; /* NULL if invalid */ } RI_CompareHashEntry; /* @@ -233,6 +243,7 @@ static dclist_head ri_constraint_cache_valid_list; * InvalidateConstraintCacheCallBack(). */ static FastPathMeta *ri_fpmeta_dead_list = NULL; +static RI_CompareInfo *ri_compare_dead_list = NULL; /* * Local function prototypes @@ -261,11 +272,13 @@ static bool ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid, Datum lhs, Datum rhs); static void ri_InitHashTables(void); +static void InvalidateCastCacheCallBack(Datum arg, SysCacheIdentifier cacheid, + uint32 hashvalue); static void InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid, uint32 hashvalue); static SPIPlanPtr ri_FetchPreparedPlan(RI_QueryKey *key); static void ri_HashPreparedPlan(RI_QueryKey *key, SPIPlanPtr plan); -static RI_CompareHashEntry *ri_HashCompareOp(Oid eq_opr, Oid typeid); +static RI_CompareInfo *ri_HashCompareOp(Oid eq_opr, Oid typeid); static void ri_CheckTrigger(FunctionCallInfo fcinfo, const char *funcname, int tgkind); @@ -2548,6 +2561,34 @@ InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid, } +/* + * Cast changes can affect any comparison or fast-path entry. Do not free or + * overwrite call information here: a cast can execute DDL and reenter RI checks + * while an outer call is still using it. AtEOXact_RI() releases detached data. + */ +static void +InvalidateCastCacheCallBack(Datum arg, SysCacheIdentifier cacheid, + uint32 hashvalue) +{ + HASH_SEQ_STATUS status; + RI_CompareHashEntry *entry; + + hash_seq_init(&status, ri_compare_cache); + while ((entry = hash_seq_search(&status)) != NULL) + { + if (entry->info != NULL) + { + entry->info->next_dead = ri_compare_dead_list; + ri_compare_dead_list = entry->info; + entry->info = NULL; + } + } + + /* Fast-path metadata contains copies of the cached call information. */ + InvalidateConstraintCacheCallBack(arg, cacheid, 0); +} + + /* * Prepare execution plan for a query to enforce an RI restriction */ @@ -3143,24 +3184,23 @@ ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo, Relation fk_rel, Relation idx_rel) { FastPathMeta *fpmeta; - MemoryContext oldcxt = MemoryContextSwitchTo(TopMemoryContext); + MemoryContext context; Assert(riinfo != NULL && riinfo->valid); Assert(riinfo->fpmeta == NULL); - fpmeta = palloc_object(FastPathMeta); - fpmeta->next_dead = NULL; - - /* Scratch context for the cached FmgrInfos' fn_mcxt; see FastPathMeta. */ - fpmeta->scratch_cxt = AllocSetContextCreate(TopMemoryContext, - "RI fast-path finfo scratch", - ALLOCSET_SMALL_SIZES); + /* Keep incomplete metadata subject to normal error cleanup. */ + context = AllocSetContextCreate(CurTransactionContext, + "RI fast-path finfo scratch", + ALLOCSET_SMALL_SIZES); + fpmeta = MemoryContextAllocZero(context, sizeof(FastPathMeta)); + fpmeta->scratch_cxt = context; for (int i = 0; i < riinfo->nkeys; i++) { Oid eq_opr = riinfo->pf_eq_oprs[i]; Oid typeid = RIAttType(fk_rel, riinfo->fk_attnums[i]); Oid lefttype; - RI_CompareHashEntry *entry = ri_HashCompareOp(eq_opr, typeid); + RI_CompareInfo *entry = ri_HashCompareOp(eq_opr, typeid); int idx_col; /* @@ -3193,8 +3233,8 @@ ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo, &fpmeta->subtypes[i]); } + MemoryContextSetParent(context, TopMemoryContext); riinfo->fpmeta = fpmeta; - MemoryContextSwitchTo(oldcxt); } /* @@ -3465,6 +3505,10 @@ ri_InitHashTables(void) ri_compare_cache = hash_create("RI compare cache", RI_INIT_QUERYHASHSIZE, &ctl, HASH_ELEM | HASH_BLOBS); + + CacheRegisterSyscacheCallback(CASTSOURCETARGET, + InvalidateCastCacheCallBack, + (Datum) 0); } @@ -3653,7 +3697,7 @@ static bool ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid, Datum lhs, Datum rhs) { - RI_CompareHashEntry *entry = ri_HashCompareOp(eq_opr, typeid); + RI_CompareInfo *entry = ri_HashCompareOp(eq_opr, typeid); /* Do we need to cast the values? */ if (OidIsValid(entry->cast_func_finfo.fn_oid)) @@ -3698,7 +3742,7 @@ ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid, * its right-hand input, a cast function to coerce the value before * comparison. */ -static RI_CompareHashEntry * +static RI_CompareInfo * ri_HashCompareOp(Oid eq_opr, Oid typeid) { RI_CompareKey key; @@ -3721,23 +3765,20 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid) &key, HASH_ENTER, &found); if (!found) - entry->valid = false; + entry->info = NULL; /* - * If not already initialized, do so. Since we'll keep this hash entry - * for the life of the backend, put any subsidiary info for the function - * cache structs into TopMemoryContext. + * If not already initialized, build a new generation of call information. + * Use a separate context so invalidation cannot affect active callers. */ - if (!entry->valid) + if (entry->info == NULL) { Oid lefttype, righttype, castfunc; CoercionPathType pathtype; - - /* We always need to know how to call the equality operator */ - fmgr_info_cxt(get_opcode(eq_opr), &entry->eq_opr_finfo, - TopMemoryContext); + MemoryContext context; + RI_CompareInfo *info; /* * If we chose to use a cast from FK to PK type, we may have to apply @@ -3782,15 +3823,22 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid) format_type_be(lefttype)); } } + /* Leave incomplete entries subject to normal error cleanup. */ + context = AllocSetContextCreate(CurTransactionContext, + "RI compare info", + ALLOCSET_SMALL_SIZES); + info = MemoryContextAllocZero(context, sizeof(RI_CompareInfo)); + info->context = context; + fmgr_info_cxt(get_opcode(eq_opr), &info->eq_opr_finfo, context); if (OidIsValid(castfunc)) - fmgr_info_cxt(castfunc, &entry->cast_func_finfo, - TopMemoryContext); + fmgr_info_cxt(castfunc, &info->cast_func_finfo, context); else - entry->cast_func_finfo.fn_oid = InvalidOid; - entry->valid = true; + info->cast_func_finfo.fn_oid = InvalidOid; + MemoryContextSetParent(context, TopMemoryContext); + entry->info = info; } - return entry; + return entry->info; } @@ -3827,30 +3875,35 @@ RI_FKey_trigger_type(Oid tgfoid) * AtEOXact_RI * End-of-transaction cleanup for referential integrity. * - * Currently this only releases fast-path metadata detached during the - * transaction. InvalidateConstraintCacheCallBack() cannot free a - * FastPathMeta when it detaches one, because an RI check further up the - * stack may still hold a pointer into it. It queues them on - * ri_fpmeta_dead_list instead, and we release them here, where no such - * reference can exist. isCommit is accepted for consistency with the - * other AtEOXact_* routines but is not used: the release is the same on - * the commit and the abort path. + * Release comparison and fast-path call information detached during the + * transaction. Invalidation callbacks cannot free it immediately, because + * an RI check further up the stack may still hold a pointer into it. + * We release the queued objects 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 + * subtransaction: detached call information stays reachable from the dead + * lists whichever subtransaction detached it, and a check holding a pointer * into one may be running at an outer level, so releasing at subtransaction * end would be unsafe as well as unnecessary. */ void AtEOXact_RI(bool isCommit) { + while (ri_compare_dead_list != NULL) + { + RI_CompareInfo *dead = ri_compare_dead_list; + + ri_compare_dead_list = dead->next_dead; + MemoryContextDelete(dead->context); + } + while (ri_fpmeta_dead_list != NULL) { FastPathMeta *dead = ri_fpmeta_dead_list; ri_fpmeta_dead_list = dead->next_dead; MemoryContextDelete(dead->scratch_cxt); - pfree(dead); } } diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 3386a17..70c05d9 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -3948,3 +3948,103 @@ 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) +-- Replacing a cast must invalidate both comparison and fast-path caches. +BEGIN; +CREATE TYPE fk_cast_type AS (v int); +CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int + LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v'; +CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT; +CREATE TABLE fk_cast_pk (id int PRIMARY KEY); +CREATE TABLE fk_cast_fk (id fk_cast_type REFERENCES fk_cast_pk); +INSERT INTO fk_cast_pk VALUES (1); +INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type); +UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type; +SAVEPOINT original_cast; +DROP CAST (fk_cast_type AS int); +CREATE FUNCTION fk_cast2(fk_cast_type) RETURNS int + LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v'; +CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast2(fk_cast_type) AS IMPLICIT; +DROP FUNCTION fk_cast1(fk_cast_type); +INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type); +UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type; +SAVEPOINT invalid_key; +INSERT INTO fk_cast_fk VALUES (ROW(2)::fk_cast_type); -- must fail +ERROR: insert or update on table "fk_cast_fk" violates foreign key constraint "fk_cast_fk_id_fkey" +DETAIL: Key (id)=((2)) is not present in table "fk_cast_pk". +ROLLBACK TO invalid_key; +-- Restoring the old cast must invalidate the replacement's cache entries too. +ROLLBACK TO original_cast; +-- Failed rebuilds must not accumulate metadata across subtransaction aborts. +SAVEPOINT missing_cast; +DROP CAST (fk_cast_type AS int); +DO $$ +DECLARE + before_count bigint; + after_count bigint; +BEGIN + SELECT count(*) INTO before_count FROM pg_backend_memory_contexts + WHERE name LIKE 'RI %'; + FOR i IN 1..10 LOOP + BEGIN + INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type); + RAISE EXCEPTION 'missing cast was not detected'; + EXCEPTION WHEN internal_error THEN + IF SQLERRM NOT LIKE 'no conversion function%' THEN + RAISE; + END IF; + END; + END LOOP; + SELECT count(*) INTO after_count FROM pg_backend_memory_contexts + WHERE name LIKE 'RI %'; + IF after_count > before_count THEN + RAISE EXCEPTION 'RI contexts leaked across failed checks'; + END IF; +END $$; +ROLLBACK TO missing_cast; +INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type); +UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type; +SELECT count(*) FROM fk_cast_fk; + count +------- + 2 +(1 row) + +ROLLBACK; +-- A cast invalidation and nested RI check must not overwrite call information +-- still being used by the outer comparison. +BEGIN; +CREATE TYPE fk_cast_type AS (v int); +CREATE TABLE fk_cast_guard (armed bool); +CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int LANGUAGE plpgsql AS $$ +BEGIN + IF EXISTS (SELECT FROM fk_cast_guard) THEN + DELETE FROM fk_cast_guard; + EXECUTE 'DROP CAST (fk_cast_type AS bigint)'; + INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type); + END IF; + RETURN $1.v; +END $$; +CREATE FUNCTION fk_cast_aux(fk_cast_type) RETURNS bigint + LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v::bigint'; +CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT; +CREATE CAST (fk_cast_type AS bigint) WITH FUNCTION fk_cast_aux(fk_cast_type); +CREATE TABLE fk_cast_pk (id int PRIMARY KEY); +CREATE TABLE fk_cast_fk (id fk_cast_type REFERENCES fk_cast_pk); +INSERT INTO fk_cast_pk VALUES (1); +INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type); +UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type; +INSERT INTO fk_cast_guard VALUES (true); +UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type; +SELECT count(*) FROM fk_cast_fk; + count +------- + 2 +(1 row) + +SELECT count(*) FROM fk_cast_guard; + count +------- + 0 +(1 row) + +ROLLBACK; diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 2f857ce..ec46ec5 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -2900,3 +2900,88 @@ DROP TABLE pktable_inval; DROP CAST (fkint AS int4); DROP FUNCTION fkint_to_int4(fkint); DROP TYPE fkint CASCADE; + +-- Replacing a cast must invalidate both comparison and fast-path caches. +BEGIN; +CREATE TYPE fk_cast_type AS (v int); +CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int + LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v'; +CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT; +CREATE TABLE fk_cast_pk (id int PRIMARY KEY); +CREATE TABLE fk_cast_fk (id fk_cast_type REFERENCES fk_cast_pk); +INSERT INTO fk_cast_pk VALUES (1); +INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type); +UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type; +SAVEPOINT original_cast; +DROP CAST (fk_cast_type AS int); +CREATE FUNCTION fk_cast2(fk_cast_type) RETURNS int + LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v'; +CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast2(fk_cast_type) AS IMPLICIT; +DROP FUNCTION fk_cast1(fk_cast_type); +INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type); +UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type; +SAVEPOINT invalid_key; +INSERT INTO fk_cast_fk VALUES (ROW(2)::fk_cast_type); -- must fail +ROLLBACK TO invalid_key; +-- Restoring the old cast must invalidate the replacement's cache entries too. +ROLLBACK TO original_cast; +-- Failed rebuilds must not accumulate metadata across subtransaction aborts. +SAVEPOINT missing_cast; +DROP CAST (fk_cast_type AS int); +DO $$ +DECLARE + before_count bigint; + after_count bigint; +BEGIN + SELECT count(*) INTO before_count FROM pg_backend_memory_contexts + WHERE name LIKE 'RI %'; + FOR i IN 1..10 LOOP + BEGIN + INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type); + RAISE EXCEPTION 'missing cast was not detected'; + EXCEPTION WHEN internal_error THEN + IF SQLERRM NOT LIKE 'no conversion function%' THEN + RAISE; + END IF; + END; + END LOOP; + SELECT count(*) INTO after_count FROM pg_backend_memory_contexts + WHERE name LIKE 'RI %'; + IF after_count > before_count THEN + RAISE EXCEPTION 'RI contexts leaked across failed checks'; + END IF; +END $$; +ROLLBACK TO missing_cast; +INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type); +UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type; +SELECT count(*) FROM fk_cast_fk; +ROLLBACK; + +-- A cast invalidation and nested RI check must not overwrite call information +-- still being used by the outer comparison. +BEGIN; +CREATE TYPE fk_cast_type AS (v int); +CREATE TABLE fk_cast_guard (armed bool); +CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int LANGUAGE plpgsql AS $$ +BEGIN + IF EXISTS (SELECT FROM fk_cast_guard) THEN + DELETE FROM fk_cast_guard; + EXECUTE 'DROP CAST (fk_cast_type AS bigint)'; + INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type); + END IF; + RETURN $1.v; +END $$; +CREATE FUNCTION fk_cast_aux(fk_cast_type) RETURNS bigint + LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v::bigint'; +CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT; +CREATE CAST (fk_cast_type AS bigint) WITH FUNCTION fk_cast_aux(fk_cast_type); +CREATE TABLE fk_cast_pk (id int PRIMARY KEY); +CREATE TABLE fk_cast_fk (id fk_cast_type REFERENCES fk_cast_pk); +INSERT INTO fk_cast_pk VALUES (1); +INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type); +UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type; +INSERT INTO fk_cast_guard VALUES (true); +UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type; +SELECT count(*) FROM fk_cast_fk; +SELECT count(*) FROM fk_cast_guard; +ROLLBACK; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index e11d3ae..58a395d 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2491,6 +2491,7 @@ RBTreeIterator REPARSE_JUNCTION_DATA_BUFFER RIX RI_CompareHashEntry +RI_CompareInfo RI_CompareKey RI_ConstraintInfo RI_QueryHashEntry -- 2.50.1 (Apple Git-155)