From 1ce6f7ce4000ce9318d0dea00259e4c7352b91cd Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Fri, 11 Sep 2026 17:06:50 +0900 Subject: [PATCH v2] Invalidate RI call information when casts change Changing a cast need not modify pg_constraint, so the RI comparison cache can keep using the old cast function after its replacement. Dropping that function can then make an UPDATE fail with a cache lookup error. Invalidate comparison call information on CASTSOURCETARGET changes. Keep it separate from its hash entry and defer releasing invalidated objects until transaction end. A cast can run DDL and reenter RI checks, so invalidation must not free or overwrite call information still used by an outer comparison. Build new call information in a transaction-owned context and reparent it only when construction succeeds. Add AtEOXact_RI() calls on commit, abort, and prepare to release the detached objects. Subtransaction end is too early, since an outer comparison may still be using them. Test replacement after cache warmup using a committed row, invalid-key rejection, rollback restoring the old cast, and invalidation with a nested comparison of another committed row. Author: Nikolay Samokhvalov Co-authored-by: Amit Langote Discussion: https://postgr.es/m/CAM527d9BgPjeOOYmbCBTd57R145qHCk-dzw9qNq+nOrDq1j__A@mail.gmail.com Backpatch-through: 14 --- src/backend/access/transam/xact.c | 3 + src/backend/utils/adt/ri_triggers.c | 106 ++++++++++++++++++---- src/include/commands/trigger.h | 2 + src/test/regress/expected/foreign_key.out | 87 ++++++++++++++++++ src/test/regress/sql/foreign_key.sql | 79 ++++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 6 files changed, 258 insertions(+), 20 deletions(-) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index b885513f765..3f3f5bc1a3b 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -2472,6 +2472,7 @@ CommitTransaction(void) AtEOXact_Files(true); AtEOXact_ComboCid(); AtEOXact_HashTables(true); + AtEOXact_RI(true); AtEOXact_PgStat(true, is_parallel_worker); AtEOXact_Snapshot(true, false); AtEOXact_ApplyLauncher(true); @@ -2766,6 +2767,7 @@ PrepareTransaction(void) AtEOXact_Files(true); AtEOXact_ComboCid(); AtEOXact_HashTables(true); + AtEOXact_RI(true); /* don't call AtEOXact_PgStat here; we fixed pgstat state above */ AtEOXact_Snapshot(true, true); /* we treat PREPARE as ROLLBACK so far as waking workers goes */ @@ -2990,6 +2992,7 @@ AbortTransaction(void) AtEOXact_Files(false); AtEOXact_ComboCid(); AtEOXact_HashTables(false); + AtEOXact_RI(false); AtEOXact_PgStat(false, is_parallel_worker); AtEOXact_ApplyLauncher(false); AtEOXact_LogicalRepWorkers(false); diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 6239900fa28..5226f535f6f 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -165,21 +165,34 @@ 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; /* * Local data */ +/* Invalidated call information retained until AtEOXact_RI(). */ +static RI_CompareInfo *ri_compare_dead_list = NULL; + static HTAB *ri_constraint_cache = NULL; static HTAB *ri_query_cache = NULL; static HTAB *ri_compare_cache = NULL; @@ -213,10 +226,11 @@ 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, int cacheid, uint32 hashvalue); static void InvalidateConstraintCacheCallBack(Datum arg, int 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); @@ -2434,6 +2448,30 @@ InvalidateConstraintCacheCallBack(Datum arg, int cacheid, uint32 hashvalue) } +/* + * Cast changes can affect any comparison entry. Do not free or overwrite + * call information here: a cast can execute DDL and reenter RI checks while an + * outer comparison is still using it. AtEOXact_RI() releases detached data. + */ +static void +InvalidateCastCacheCallBack(Datum arg, int 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; + } + } +} + + /* * Prepare execution plan for a query to enforce an RI restriction */ @@ -2883,6 +2921,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); } @@ -3071,7 +3113,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)) @@ -3113,7 +3155,7 @@ ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid, * See if we know how to compare two values, and create a new hash entry * if not. */ -static RI_CompareHashEntry * +static RI_CompareInfo * ri_HashCompareOp(Oid eq_opr, Oid typeid) { RI_CompareKey key; @@ -3136,23 +3178,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 @@ -3190,15 +3229,23 @@ 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; } @@ -3230,3 +3277,22 @@ RI_FKey_trigger_type(Oid tgfoid) return RI_TRIGGER_NONE; } + + +/* + * Release comparison call information detached by invalidation callbacks. + * No RI comparison can still reference it at transaction end, on commit, + * abort, or prepare. Do not release it at subtransaction end: an outer + * comparison may still be using an object invalidated by a nested call. + */ +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); + } +} diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index 54be07c9e42..2de516ca11d 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -289,4 +289,6 @@ extern void RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, extern int RI_FKey_trigger_type(Oid tgfoid); +extern void AtEOXact_RI(bool isCommit); + #endif /* TRIGGER_H */ diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 427a55cfee0..1c81705113d 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -983,6 +983,93 @@ CREATE TABLE PKTABLE (ptest1 int, ptest2 inet, ptest3 int, ptest4 inet, PRIMARY ptest3) REFERENCES pktable); ERROR: foreign key constraint "pktable_ptest4_ptest3_fkey" cannot be implemented DETAIL: Key columns "ptest4" of the referencing table and "ptest1" of the referenced table are of incompatible types: inet and integer. +-- Replacing a cast must invalidate cached RI comparison call information. +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 (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk); +INSERT INTO fk_cast_pk VALUES (1); +INSERT INTO fk_cast_fk VALUES ('original', ROW(1)::fk_cast_type); +-- With autocommit, this compares a committed row and commits the updated row. +-- Updating a row inserted in the same transaction would skip the comparison. +UPDATE fk_cast_fk SET id = id WHERE label = 'original'; +BEGIN; +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); +-- Exercise the comparison cache before a new INSERT can rebuild other caches. +UPDATE fk_cast_fk SET id = id WHERE label = 'original'; +INSERT INTO fk_cast_fk VALUES ('replacement', ROW(1)::fk_cast_type); +SAVEPOINT invalid_key; +INSERT INTO fk_cast_fk VALUES ('invalid', 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; +-- Exercise restoration immediately, before any further DDL invalidates caches. +ROLLBACK TO original_cast; +UPDATE fk_cast_fk SET id = id WHERE label = 'original'; +INSERT INTO fk_cast_fk VALUES ('restored', ROW(1)::fk_cast_type); +SELECT count(*) FROM fk_cast_fk; + count +------- + 2 +(1 row) + +ROLLBACK; +DROP TABLE fk_cast_fk, fk_cast_pk; +DROP CAST (fk_cast_type AS int); +DROP FUNCTION fk_cast1(fk_cast_type); +DROP TYPE fk_cast_type; +-- Invalidation during a comparison must not overwrite its call information. +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)'; + -- Compare a different committed row, rebuilding the same cache entry. + UPDATE fk_cast_fk SET id = id WHERE label = 'nested'; + 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 (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk); +INSERT INTO fk_cast_pk VALUES (1); +INSERT INTO fk_cast_fk VALUES ('outer', ROW(1)::fk_cast_type), + ('nested', ROW(1)::fk_cast_type); +UPDATE fk_cast_fk SET id = id WHERE label = 'outer'; +BEGIN; +INSERT INTO fk_cast_guard VALUES (true); +-- Both the outer and nested UPDATE compare rows from earlier transactions. +UPDATE fk_cast_fk SET id = id WHERE label = 'outer'; +SELECT count(*) FROM fk_cast_guard; + count +------- + 0 +(1 row) + +SELECT count(*) FROM fk_cast_fk; + count +------- + 2 +(1 row) + +ROLLBACK; +DROP TABLE fk_cast_fk, fk_cast_pk, fk_cast_guard; +DROP CAST (fk_cast_type AS int); +DROP CAST (fk_cast_type AS bigint); +DROP FUNCTION fk_cast1(fk_cast_type), fk_cast_aux(fk_cast_type); +DROP TYPE fk_cast_type; -- -- Now some cases with inheritance -- Basic 2 table case: 1 column of matching types. diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 983bd856eeb..ba3d5a2ed3a 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -628,6 +628,85 @@ ptest3) REFERENCES pktable(ptest1, ptest2)); CREATE TABLE PKTABLE (ptest1 int, ptest2 inet, ptest3 int, ptest4 inet, PRIMARY KEY(ptest1, ptest2), FOREIGN KEY(ptest4, ptest3) REFERENCES pktable); +-- Replacing a cast must invalidate cached RI comparison call information. +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 (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk); +INSERT INTO fk_cast_pk VALUES (1); +INSERT INTO fk_cast_fk VALUES ('original', ROW(1)::fk_cast_type); + +-- With autocommit, this compares a committed row and commits the updated row. +-- Updating a row inserted in the same transaction would skip the comparison. +UPDATE fk_cast_fk SET id = id WHERE label = 'original'; + +BEGIN; +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); + +-- Exercise the comparison cache before a new INSERT can rebuild other caches. +UPDATE fk_cast_fk SET id = id WHERE label = 'original'; +INSERT INTO fk_cast_fk VALUES ('replacement', ROW(1)::fk_cast_type); +SAVEPOINT invalid_key; +INSERT INTO fk_cast_fk VALUES ('invalid', ROW(2)::fk_cast_type); -- must fail +ROLLBACK TO invalid_key; + +-- Exercise restoration immediately, before any further DDL invalidates caches. +ROLLBACK TO original_cast; +UPDATE fk_cast_fk SET id = id WHERE label = 'original'; +INSERT INTO fk_cast_fk VALUES ('restored', ROW(1)::fk_cast_type); +SELECT count(*) FROM fk_cast_fk; +ROLLBACK; + +DROP TABLE fk_cast_fk, fk_cast_pk; +DROP CAST (fk_cast_type AS int); +DROP FUNCTION fk_cast1(fk_cast_type); +DROP TYPE fk_cast_type; + +-- Invalidation during a comparison must not overwrite its call information. +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)'; + -- Compare a different committed row, rebuilding the same cache entry. + UPDATE fk_cast_fk SET id = id WHERE label = 'nested'; + 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 (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk); +INSERT INTO fk_cast_pk VALUES (1); +INSERT INTO fk_cast_fk VALUES ('outer', ROW(1)::fk_cast_type), + ('nested', ROW(1)::fk_cast_type); +UPDATE fk_cast_fk SET id = id WHERE label = 'outer'; + +BEGIN; +INSERT INTO fk_cast_guard VALUES (true); +-- Both the outer and nested UPDATE compare rows from earlier transactions. +UPDATE fk_cast_fk SET id = id WHERE label = 'outer'; +SELECT count(*) FROM fk_cast_guard; +SELECT count(*) FROM fk_cast_fk; +ROLLBACK; + +DROP TABLE fk_cast_fk, fk_cast_pk, fk_cast_guard; +DROP CAST (fk_cast_type AS int); +DROP CAST (fk_cast_type AS bigint); +DROP FUNCTION fk_cast1(fk_cast_type), fk_cast_aux(fk_cast_type); +DROP TYPE fk_cast_type; + -- -- Now some cases with inheritance -- Basic 2 table case: 1 column of matching types. diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index b6f0897dc84..7205feed9b1 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2399,6 +2399,7 @@ RBTreeIterator REPARSE_JUNCTION_DATA_BUFFER RIX RI_CompareHashEntry +RI_CompareInfo RI_CompareKey RI_ConstraintInfo RI_QueryHashEntry -- 2.47.3