On Fri, Sep 11, 2026 at 2:25 AM Amit Langote <[email protected]> wrote: > For 0001, SPI's FOR KEY SHARE also requires UPDATE privilege on at > least one column. I've used ExecCheckOneRelPerms() to cover that along > with column-level SELECT. The tests exercise both per-row and batched > checks, including rejection without UPDATE and acceptance with UPDATE > on an unrelated column.
I kept testing with my AI harness and found another case on master and PG19. Replacing a loose cross-type equality member leaves the FK's stored operator unchanged. An uncached fast-path check then errors; warmed metadata is not invalidated by the pg_amop change. The replacement calls the same int48eq function, so the family semantics are unchanged and SPI continues to enforce the FK normally. Attached are standalone fixes for master a625fc57 and PG19 f4b511ae. They invalidate the metadata on pg_amop changes and use SPI unless the stored operator is still an equality member. On master, buffered rows also need the SPI fallback if the family changes between AFTER triggers. Both assertion builds pass the native foreign_key test, full regression and isolation suites, and the injection-point suites. The tests cover warmed and uncached metadata and missing keys on both sides of the DDL. Thanks, Nik
From 2020efd6cca71b90916e4bc3625b485fabba453f Mon Sep 17 00:00:00 2001 From: Nik Samokhvalov <[email protected]> Date: Mon, 14 Sep 2026 17:17:55 -0700 Subject: [PATCH] Check RI fast-path operator family membership A loose cross-type equality member can be replaced without changing an existing foreign key's recorded operator. Invalidate fast-path metadata on pg_amop changes and fall back to SPI unless that operator is still an equality member of the referenced index's operator family. Operator family DDL can also occur between AFTER triggers while a batch has buffered rows. Recheck eligibility when flushing and use the same SPI plan as the per-row path for any rows that can no longer be checked by a direct index probe. Add regression coverage for warmed and uncached metadata, and for valid and missing keys on both sides of a mid-batch operator family change. --- src/backend/utils/adt/ri_triggers.c | 294 +++++++++++++++------- src/test/regress/expected/foreign_key.out | 141 +++++++++++ src/test/regress/sql/foreign_key.sql | 88 +++++++ 3 files changed, 428 insertions(+), 95 deletions(-) diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index c46f789..66bfcfe 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -355,6 +355,9 @@ static void ri_InitHashTables(void); static void InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid, uint32 hashvalue); static SPIPlanPtr ri_FetchPreparedPlan(RI_QueryKey *key); +static SPIPlanPtr ri_FetchPreparedCheckPlan(RI_QueryKey *key, + const RI_ConstraintInfo *riinfo, + Relation fk_rel, Relation pk_rel); static void ri_HashPreparedPlan(RI_QueryKey *key, SPIPlanPtr plan); static RI_CompareHashEntry *ri_HashCompareOp(Oid eq_opr, Oid typeid); @@ -378,6 +381,9 @@ static bool ri_FastPathBatchAdd(RI_ConstraintInfo *riinfo, Relation fk_rel, TupleTableSlot *newslot); static void ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel, RI_ConstraintInfo *riinfo); +static void ri_FastPathBatchFlushSPI(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, @@ -565,92 +571,7 @@ RI_FKey_check(TriggerData *trigdata) /* Fetch or prepare a saved plan for the real check */ ri_BuildQueryKey(&qkey, riinfo, RI_PLAN_CHECK_LOOKUPPK); - - if ((qplan = ri_FetchPreparedPlan(&qkey)) == NULL) - { - StringInfoData querybuf; - char pkrelname[MAX_QUOTED_REL_NAME_LEN]; - char attname[MAX_QUOTED_NAME_LEN]; - char paramname[16]; - const char *querysep; - Oid queryoids[RI_MAX_NUMKEYS]; - const char *pk_only; - - /* ---------- - * The query string built is - * SELECT 1 FROM [ONLY] <pktable> x WHERE pkatt1 = $1 [AND ...] - * FOR KEY SHARE OF x - * The type id's for the $ parameters are those of the - * corresponding FK attributes. - * - * But for temporal FKs we need to make sure - * the FK's range is completely covered. - * So we use this query instead: - * SELECT 1 - * FROM ( - * SELECT pkperiodatt AS r - * FROM [ONLY] pktable x - * WHERE pkatt1 = $1 [AND ...] - * AND pkperiodatt && $n - * FOR KEY SHARE OF x - * ) x1 - * HAVING $n <@ range_agg(x1.r) - * Note if FOR KEY SHARE ever allows GROUP BY and HAVING - * we can make this a bit simpler. - * ---------- - */ - initStringInfo(&querybuf); - pk_only = pk_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ? - "" : "ONLY "; - quoteRelationName(pkrelname, pk_rel); - if (riinfo->hasperiod) - { - quoteOneName(attname, - RIAttName(pk_rel, riinfo->pk_attnums[riinfo->nkeys - 1])); - - appendStringInfo(&querybuf, - "SELECT 1 FROM (SELECT %s AS r FROM %s%s x", - attname, pk_only, pkrelname); - } - else - { - appendStringInfo(&querybuf, "SELECT 1 FROM %s%s x", - pk_only, pkrelname); - } - querysep = "WHERE"; - for (int i = 0; i < riinfo->nkeys; i++) - { - Oid pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]); - Oid fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]); - - quoteOneName(attname, - RIAttName(pk_rel, riinfo->pk_attnums[i])); - sprintf(paramname, "$%d", i + 1); - ri_GenerateQual(&querybuf, querysep, - attname, pk_type, - riinfo->pf_eq_oprs[i], - paramname, fk_type); - querysep = "AND"; - queryoids[i] = fk_type; - } - appendStringInfoString(&querybuf, " FOR KEY SHARE OF x"); - if (riinfo->hasperiod) - { - Oid fk_type = RIAttType(fk_rel, riinfo->fk_attnums[riinfo->nkeys - 1]); - - appendStringInfoString(&querybuf, ") x1 HAVING "); - sprintf(paramname, "$%d", riinfo->nkeys); - ri_GenerateQual(&querybuf, "", - paramname, fk_type, - riinfo->agged_period_contained_by_oper, - "pg_catalog.range_agg", ANYMULTIRANGEOID); - appendStringInfoString(&querybuf, "(x1.r)"); - } - - /* Prepare and save the plan */ - qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids, - &qkey, fk_rel, pk_rel); - } + qplan = ri_FetchPreparedCheckPlan(&qkey, riinfo, fk_rel, pk_rel); /* * Now check that foreign key exists in PK table @@ -2601,7 +2522,7 @@ get_ri_constraint_root(Oid constrOid) } /* - * Callback for pg_constraint inval events + * Callback for pg_constraint and pg_amop inval events * * While most syscache callbacks just flush all their entries, pg_constraint * gets enough update traffic that it's probably worth being smarter. @@ -2626,6 +2547,10 @@ InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid, Assert(ri_constraint_cache != NULL); + /* pg_amop changes can affect any constraint's fast-path metadata. */ + if (cacheid == AMOPOPID) + hashvalue = 0; + /* * If the list of currently valid entries gets excessively large, we mark * them all invalid so we can empty the list. This arrangement avoids @@ -2719,6 +2644,105 @@ ri_PlanCheck(const char *querystr, int nargs, const Oid *argtypes, return qplan; } +/* + * Fetch or prepare the plan used to check a foreign key row via SPI. + */ +static SPIPlanPtr +ri_FetchPreparedCheckPlan(RI_QueryKey *qkey, + const RI_ConstraintInfo *riinfo, + Relation fk_rel, Relation pk_rel) +{ + SPIPlanPtr qplan; + + if ((qplan = ri_FetchPreparedPlan(qkey)) == NULL) + { + StringInfoData querybuf; + char pkrelname[MAX_QUOTED_REL_NAME_LEN]; + char attname[MAX_QUOTED_NAME_LEN]; + char paramname[16]; + const char *querysep; + Oid queryoids[RI_MAX_NUMKEYS]; + const char *pk_only; + + /* ---------- + * The query string built is + * SELECT 1 FROM [ONLY] <pktable> x WHERE pkatt1 = $1 [AND ...] + * FOR KEY SHARE OF x + * The type id's for the $ parameters are those of the + * corresponding FK attributes. + * + * But for temporal FKs we need to make sure + * the FK's range is completely covered. + * So we use this query instead: + * SELECT 1 + * FROM ( + * SELECT pkperiodatt AS r + * FROM [ONLY] pktable x + * WHERE pkatt1 = $1 [AND ...] + * AND pkperiodatt && $n + * FOR KEY SHARE OF x + * ) x1 + * HAVING $n <@ range_agg(x1.r) + * Note if FOR KEY SHARE ever allows GROUP BY and HAVING + * we can make this a bit simpler. + * ---------- + */ + initStringInfo(&querybuf); + pk_only = pk_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ? + "" : "ONLY "; + quoteRelationName(pkrelname, pk_rel); + if (riinfo->hasperiod) + { + quoteOneName(attname, + RIAttName(pk_rel, riinfo->pk_attnums[riinfo->nkeys - 1])); + + appendStringInfo(&querybuf, + "SELECT 1 FROM (SELECT %s AS r FROM %s%s x", + attname, pk_only, pkrelname); + } + else + { + appendStringInfo(&querybuf, "SELECT 1 FROM %s%s x", + pk_only, pkrelname); + } + querysep = "WHERE"; + for (int i = 0; i < riinfo->nkeys; i++) + { + Oid pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]); + Oid fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]); + + quoteOneName(attname, + RIAttName(pk_rel, riinfo->pk_attnums[i])); + sprintf(paramname, "$%d", i + 1); + ri_GenerateQual(&querybuf, querysep, + attname, pk_type, + riinfo->pf_eq_oprs[i], + paramname, fk_type); + querysep = "AND"; + queryoids[i] = fk_type; + } + appendStringInfoString(&querybuf, " FOR KEY SHARE OF x"); + if (riinfo->hasperiod) + { + Oid fk_type = RIAttType(fk_rel, riinfo->fk_attnums[riinfo->nkeys - 1]); + + appendStringInfoString(&querybuf, ") x1 HAVING "); + sprintf(paramname, "$%d", riinfo->nkeys); + ri_GenerateQual(&querybuf, "", + paramname, fk_type, + riinfo->agged_period_contained_by_oper, + "pg_catalog.range_agg", ANYMULTIRANGEOID); + appendStringInfoString(&querybuf, "(x1.r)"); + } + + /* Prepare and save the plan */ + qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids, + qkey, fk_rel, pk_rel); + } + + return qplan; +} + /* * Perform a query to enforce an RI restriction */ @@ -3073,6 +3097,13 @@ ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel, if (fpentry->batch_count == 0) return; + /* The operator family may have changed since these rows were buffered. */ + if (!ri_check_fastpath_index(riinfo, pk_rel, idx_rel)) + { + ri_FastPathBatchFlushSPI(fpentry, fk_rel, riinfo); + 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 @@ -3177,6 +3208,53 @@ ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel, MemoryContextSwitchTo(oldcxt); } +/* + * Check buffered rows through SPI after the index becomes unsuitable for + * direct probing. This can happen if user code changes the operator family + * between AFTER triggers in the same firing cycle. + */ +static void +ri_FastPathBatchFlushSPI(RI_FastPathEntry *fpentry, Relation fk_rel, + RI_ConstraintInfo *riinfo) +{ + RI_QueryKey qkey; + SPIPlanPtr qplan; + + if (fpentry->batch_count == 0) + return; + + /* Protect the batch array from reentrant checks, as in the direct path. */ + Assert(!fpentry->flushing); + fpentry->flushing = true; + PG_TRY(); + { + SPI_connect(); + ri_BuildQueryKey(&qkey, riinfo, RI_PLAN_CHECK_LOOKUPPK); + qplan = ri_FetchPreparedCheckPlan(&qkey, riinfo, fk_rel, + fpentry->pk_rel); + + for (int i = 0; i < fpentry->batch_count; i++) + { + ExecStoreHeapTuple(fpentry->batch[i], fpentry->fk_slot, false); + ri_PerformCheck(riinfo, &qkey, qplan, + fk_rel, fpentry->pk_rel, + NULL, fpentry->fk_slot, + false, false, SPI_OK_SELECT); + } + + if (SPI_finish() != SPI_OK_FINISH) + elog(ERROR, "SPI_finish failed"); + } + PG_FINALLY(); + { + fpentry->flushing = false; + fpentry->batch_count = 0; + } + PG_END_TRY(); + + MemoryContextReset(fpentry->flush_cxt); +} + /* * ri_FastPathFlushLoop * Multi-column fallback: probe the index once per buffered row. @@ -3566,6 +3644,32 @@ ri_check_fastpath_index(RI_ConstraintInfo *riinfo, } } + /* + * The equality operator stored in pg_constraint must still be an equality + * member of the index opfamily. A loose cross-type member can be + * replaced without changing the constraint itself; leave that case to + * SPI, which continues to use the operator recorded by the constraint. + */ + for (int i = 0; i < riinfo->nkeys; i++) + { + int idx_col; + + for (idx_col = 0; idx_col < idx_rel->rd_index->indnkeyatts; idx_col++) + { + if (idx_rel->rd_index->indkey.values[idx_col] == + riinfo->pk_attnums[i]) + break; + } + Assert(idx_col < idx_rel->rd_index->indnkeyatts); + + if (get_op_opfamily_strategy(riinfo->pf_eq_oprs[i], + idx_rel->rd_opfamily[idx_col]) != BTEqualStrategyNumber) + { + riinfo->fastpath_state = RI_FASTPATH_UNUSABLE; + return false; + } + } + riinfo->fastpath_state = RI_FASTPATH_USABLE; return true; } @@ -4022,10 +4126,13 @@ ri_InitHashTables(void) RI_INIT_CONSTRAINTHASHSIZE, &ctl, HASH_ELEM | HASH_BLOBS); - /* Arrange to flush cache on pg_constraint changes */ + /* Arrange to flush cache on pg_constraint or pg_amop changes */ CacheRegisterSyscacheCallback(CONSTROID, InvalidateConstraintCacheCallBack, (Datum) 0); + CacheRegisterSyscacheCallback(AMOPOPID, + InvalidateConstraintCacheCallBack, + (Datum) 0); ctl.keysize = sizeof(RI_QueryKey); ctl.entrysize = sizeof(RI_QueryHashEntry); @@ -4794,14 +4901,11 @@ ri_FastPathGetEntry(RI_ConstraintInfo *riinfo, Relation fk_rel) { /* * Invalidation can reset the cached eligibility while an entry is - * still in use. Its held index remains usable, even if REINDEX - * CONCURRENTLY has replaced it with an equivalent new index. + * still in use. REINDEX CONCURRENTLY leaves its held index usable, + * but an operator family change can require falling back to SPI. + * Leave any buffered rows for the end-of-batch callback to check. */ - bool usable; - - usable = ri_check_fastpath_index(riinfo, entry->pk_rel, entry->idx_rel); - Assert(usable); - if (!usable) + if (!ri_check_fastpath_index(riinfo, entry->pk_rel, entry->idx_rel)) return NULL; } diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 8d81240..a5a428f 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -1030,6 +1030,147 @@ 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 loose cross-type operator family member must invalidate the +-- fast-path metadata. The FK continues using its stored equality operator, +-- which need no longer be a member of the index family. Use the very same +-- implementation for the replacement, keeping the family semantics unchanged. +create schema fk_opfamily; +set search_path = fk_opfamily, pg_catalog; +create operator family fam using btree; +create operator class int_ops for type integer using btree family fam as + operator 1 <(integer,integer), operator 2 <=(integer,integer), + operator 3 =(integer,integer), operator 4 >=(integer,integer), + operator 5 >(integer,integer), function 1 btint4cmp(integer,integer); +alter operator family fam using btree add + operator 1 <(integer,bigint), operator 2 <=(integer,bigint), + operator 3 =(integer,bigint), operator 4 >=(integer,bigint), + operator 5 >(integer,bigint), + operator 1 <(bigint,integer), operator 2 <=(bigint,integer), + operator 3 =(bigint,integer), operator 4 >=(bigint,integer), + operator 5 >(bigint,integer), + operator 1 <(bigint,bigint), operator 2 <=(bigint,bigint), + operator 3 =(bigint,bigint), operator 4 >=(bigint,bigint), + operator 5 >(bigint,bigint), + function 1 (integer,bigint) btint48cmp(integer,bigint), + function 1 (bigint,integer) btint84cmp(bigint,integer), + function 1 (bigint,bigint) btint8cmp(bigint,bigint); +create operator =#= (leftarg=integer, rightarg=bigint, function=int48eq); +create table p(k integer); +create unique index p_idx on p(k int_ops); +create table warm(k bigint references p(k)); +create table cold(k bigint references p(k)); +insert into p values (1), (2); +insert into warm values (1); +select amvalidate(oid) from pg_opclass +where opcnamespace = 'fk_opfamily'::regnamespace; + amvalidate +------------ + t +(1 row) + +select exists (select from pg_backend_memory_contexts + where name = 'RI fast-path finfo scratch') as metadata_cached; + metadata_cached +----------------- + t +(1 row) + +-- Change only pg_amop after warming the cache. +begin; +alter operator family fam using btree drop operator 3(integer,bigint); +alter operator family fam using btree add operator 3 =#=(integer,bigint); +commit; +select amvalidate(oid) from pg_opclass +where opcnamespace = 'fk_opfamily'::regnamespace; + amvalidate +------------ + t +(1 row) + +select exists (select from pg_backend_memory_contexts + where name = 'RI fast-path finfo scratch') as metadata_cached; + metadata_cached +----------------- + f +(1 row) + +insert into warm values (2); +insert into warm values (99); +ERROR: insert or update on table "warm" violates foreign key constraint "warm_k_fkey" +DETAIL: Key (k)=(99) is not present in table "p". +-- This constraint has no cached fast-path metadata yet. +insert into cold values (2); +insert into cold values (99); +ERROR: insert or update on table "cold" violates foreign key constraint "cold_k_fkey" +DETAIL: Key (k)=(99) is not present in table "p". +select * from warm order by k; + k +--- + 1 + 2 +(2 rows) + +select * from cold order by k; + k +--- + 2 +(1 row) + +-- Change the family between AFTER triggers. The RI trigger sorts first, so +-- rows buffered before the DDL must also be checked through SPI at batch end. +alter operator family fam using btree drop operator 3(integer,bigint); +alter operator family fam using btree add operator 3 =(integer,bigint); +create table batch(k bigint references p(k)); +create function change_family() returns trigger language plpgsql as $$ +begin + if new.k = 1 then + alter operator family fam using btree drop operator 3(integer,bigint); + alter operator family fam using btree add operator 3 =#=(integer,bigint); + end if; + return null; +end +$$; +create trigger zzz_change_family after insert on batch + for each row execute function change_family(); +begin; +insert into batch values (1), (2); +select * from batch order by k; + k +--- + 1 + 2 +(2 rows) + +rollback; +-- No later RI trigger is needed to notice that the index became unsuitable. +begin; +insert into batch values (1); +select * from batch; + k +--- + 1 +(1 row) + +rollback; +-- Reject missing keys both after and before the DDL boundary. +insert into batch values (1), (99); +ERROR: insert or update on table "batch" violates foreign key constraint "batch_k_fkey" +DETAIL: Key (k)=(99) is not present in table "p". +insert into batch values (99), (1); +ERROR: insert or update on table "batch" violates foreign key constraint "batch_k_fkey" +DETAIL: Key (k)=(99) is not present in table "p". +select * from batch; + k +--- +(0 rows) + +reset search_path; +drop table fk_opfamily.warm, fk_opfamily.cold, fk_opfamily.batch, fk_opfamily.p; +drop function fk_opfamily.change_family(); +drop operator class fk_opfamily.int_ops using btree; +drop operator family fk_opfamily.fam using btree; +drop operator fk_opfamily.=#=(integer,bigint); +drop schema fk_opfamily; -- -- 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 184d9ef..ba4c408 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -692,6 +692,94 @@ 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 loose cross-type operator family member must invalidate the +-- fast-path metadata. The FK continues using its stored equality operator, +-- which need no longer be a member of the index family. Use the very same +-- implementation for the replacement, keeping the family semantics unchanged. +create schema fk_opfamily; +set search_path = fk_opfamily, pg_catalog; +create operator family fam using btree; +create operator class int_ops for type integer using btree family fam as + operator 1 <(integer,integer), operator 2 <=(integer,integer), + operator 3 =(integer,integer), operator 4 >=(integer,integer), + operator 5 >(integer,integer), function 1 btint4cmp(integer,integer); +alter operator family fam using btree add + operator 1 <(integer,bigint), operator 2 <=(integer,bigint), + operator 3 =(integer,bigint), operator 4 >=(integer,bigint), + operator 5 >(integer,bigint), + operator 1 <(bigint,integer), operator 2 <=(bigint,integer), + operator 3 =(bigint,integer), operator 4 >=(bigint,integer), + operator 5 >(bigint,integer), + operator 1 <(bigint,bigint), operator 2 <=(bigint,bigint), + operator 3 =(bigint,bigint), operator 4 >=(bigint,bigint), + operator 5 >(bigint,bigint), + function 1 (integer,bigint) btint48cmp(integer,bigint), + function 1 (bigint,integer) btint84cmp(bigint,integer), + function 1 (bigint,bigint) btint8cmp(bigint,bigint); +create operator =#= (leftarg=integer, rightarg=bigint, function=int48eq); +create table p(k integer); +create unique index p_idx on p(k int_ops); +create table warm(k bigint references p(k)); +create table cold(k bigint references p(k)); +insert into p values (1), (2); +insert into warm values (1); +select amvalidate(oid) from pg_opclass +where opcnamespace = 'fk_opfamily'::regnamespace; +select exists (select from pg_backend_memory_contexts + where name = 'RI fast-path finfo scratch') as metadata_cached; +-- Change only pg_amop after warming the cache. +begin; +alter operator family fam using btree drop operator 3(integer,bigint); +alter operator family fam using btree add operator 3 =#=(integer,bigint); +commit; +select amvalidate(oid) from pg_opclass +where opcnamespace = 'fk_opfamily'::regnamespace; +select exists (select from pg_backend_memory_contexts + where name = 'RI fast-path finfo scratch') as metadata_cached; +insert into warm values (2); +insert into warm values (99); +-- This constraint has no cached fast-path metadata yet. +insert into cold values (2); +insert into cold values (99); +select * from warm order by k; +select * from cold order by k; +-- Change the family between AFTER triggers. The RI trigger sorts first, so +-- rows buffered before the DDL must also be checked through SPI at batch end. +alter operator family fam using btree drop operator 3(integer,bigint); +alter operator family fam using btree add operator 3 =(integer,bigint); +create table batch(k bigint references p(k)); +create function change_family() returns trigger language plpgsql as $$ +begin + if new.k = 1 then + alter operator family fam using btree drop operator 3(integer,bigint); + alter operator family fam using btree add operator 3 =#=(integer,bigint); + end if; + return null; +end +$$; +create trigger zzz_change_family after insert on batch + for each row execute function change_family(); +begin; +insert into batch values (1), (2); +select * from batch order by k; +rollback; +-- No later RI trigger is needed to notice that the index became unsuitable. +begin; +insert into batch values (1); +select * from batch; +rollback; +-- Reject missing keys both after and before the DDL boundary. +insert into batch values (1), (99); +insert into batch values (99), (1); +select * from batch; +reset search_path; +drop table fk_opfamily.warm, fk_opfamily.cold, fk_opfamily.batch, fk_opfamily.p; +drop function fk_opfamily.change_family(); +drop operator class fk_opfamily.int_ops using btree; +drop operator family fk_opfamily.fam using btree; +drop operator fk_opfamily.=#=(integer,bigint); +drop schema fk_opfamily; + -- -- Now some cases with inheritance -- Basic 2 table case: 1 column of matching types. base-commit: a625fc570c22e199471e1a2656e2c32b8dc0c0fd -- 2.50.1 (Apple Git-155)
From ff40ce13e736ec7824b583a213f388faf4e7d6c4 Mon Sep 17 00:00:00 2001 From: Nik Samokhvalov <[email protected]> Date: Mon, 14 Sep 2026 17:19:36 -0700 Subject: [PATCH] Check RI fast-path operator family membership A loose cross-type equality member can be replaced without changing an existing foreign key's recorded operator. Invalidate fast-path metadata on pg_amop changes and fall back to SPI unless that operator is still an equality member of the referenced index's operator family. Add regression coverage for warmed and uncached metadata, including an operator family change between AFTER triggers. PG19 has no batching code, so no batch-flush fallback is needed on this branch. Backpatch-through: 19 --- src/backend/utils/adt/ri_triggers.c | 37 +++++- src/test/regress/expected/foreign_key.out | 141 ++++++++++++++++++++++ src/test/regress/sql/foreign_key.sql | 88 ++++++++++++++ 3 files changed, 264 insertions(+), 2 deletions(-) diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 5d55a2006e..4eb6731ac1 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -2481,7 +2481,7 @@ get_ri_constraint_root(Oid constrOid) } /* - * Callback for pg_constraint inval events + * Callback for pg_constraint and pg_amop inval events * * While most syscache callbacks just flush all their entries, pg_constraint * gets enough update traffic that it's probably worth being smarter. @@ -2506,6 +2506,10 @@ InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid, Assert(ri_constraint_cache != NULL); + /* pg_amop changes can affect any constraint's fast-path metadata. */ + if (cacheid == AMOPOPID) + hashvalue = 0; + /* * If the list of currently valid entries gets excessively large, we mark * them all invalid so we can empty the list. This arrangement avoids @@ -3047,6 +3051,32 @@ ri_check_fastpath_index(RI_ConstraintInfo *riinfo, } } + /* + * The equality operator stored in pg_constraint must still be an equality + * member of the index opfamily. A loose cross-type member can be + * replaced without changing the constraint itself; leave that case to + * SPI, which continues to use the operator recorded by the constraint. + */ + for (int i = 0; i < riinfo->nkeys; i++) + { + int idx_col; + + for (idx_col = 0; idx_col < idx_rel->rd_index->indnkeyatts; idx_col++) + { + if (idx_rel->rd_index->indkey.values[idx_col] == + riinfo->pk_attnums[i]) + break; + } + Assert(idx_col < idx_rel->rd_index->indnkeyatts); + + if (get_op_opfamily_strategy(riinfo->pf_eq_oprs[i], + idx_rel->rd_opfamily[idx_col]) != BTEqualStrategyNumber) + { + riinfo->fastpath_state = RI_FASTPATH_UNUSABLE; + return false; + } + } + riinfo->fastpath_state = RI_FASTPATH_USABLE; return true; } @@ -3503,10 +3533,13 @@ ri_InitHashTables(void) RI_INIT_CONSTRAINTHASHSIZE, &ctl, HASH_ELEM | HASH_BLOBS); - /* Arrange to flush cache on pg_constraint changes */ + /* Arrange to flush cache on pg_constraint or pg_amop changes */ CacheRegisterSyscacheCallback(CONSTROID, InvalidateConstraintCacheCallBack, (Datum) 0); + CacheRegisterSyscacheCallback(AMOPOPID, + InvalidateConstraintCacheCallBack, + (Datum) 0); ctl.keysize = sizeof(RI_QueryKey); ctl.entrysize = sizeof(RI_QueryHashEntry); diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 9cea669b6f..06addc4dd8 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -1030,6 +1030,147 @@ 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 loose cross-type operator family member must invalidate the +-- fast-path metadata. The FK continues using its stored equality operator, +-- which need no longer be a member of the index family. Use the very same +-- implementation for the replacement, keeping the family semantics unchanged. +create schema fk_opfamily; +set search_path = fk_opfamily, pg_catalog; +create operator family fam using btree; +create operator class int_ops for type integer using btree family fam as + operator 1 <(integer,integer), operator 2 <=(integer,integer), + operator 3 =(integer,integer), operator 4 >=(integer,integer), + operator 5 >(integer,integer), function 1 btint4cmp(integer,integer); +alter operator family fam using btree add + operator 1 <(integer,bigint), operator 2 <=(integer,bigint), + operator 3 =(integer,bigint), operator 4 >=(integer,bigint), + operator 5 >(integer,bigint), + operator 1 <(bigint,integer), operator 2 <=(bigint,integer), + operator 3 =(bigint,integer), operator 4 >=(bigint,integer), + operator 5 >(bigint,integer), + operator 1 <(bigint,bigint), operator 2 <=(bigint,bigint), + operator 3 =(bigint,bigint), operator 4 >=(bigint,bigint), + operator 5 >(bigint,bigint), + function 1 (integer,bigint) btint48cmp(integer,bigint), + function 1 (bigint,integer) btint84cmp(bigint,integer), + function 1 (bigint,bigint) btint8cmp(bigint,bigint); +create operator =#= (leftarg=integer, rightarg=bigint, function=int48eq); +create table p(k integer); +create unique index p_idx on p(k int_ops); +create table warm(k bigint references p(k)); +create table cold(k bigint references p(k)); +insert into p values (1), (2); +insert into warm values (1); +select amvalidate(oid) from pg_opclass +where opcnamespace = 'fk_opfamily'::regnamespace; + amvalidate +------------ + t +(1 row) + +select exists (select from pg_backend_memory_contexts + where name = 'RI fast-path finfo scratch') as metadata_cached; + metadata_cached +----------------- + t +(1 row) + +-- Change only pg_amop after warming the cache. +begin; +alter operator family fam using btree drop operator 3(integer,bigint); +alter operator family fam using btree add operator 3 =#=(integer,bigint); +commit; +select amvalidate(oid) from pg_opclass +where opcnamespace = 'fk_opfamily'::regnamespace; + amvalidate +------------ + t +(1 row) + +select exists (select from pg_backend_memory_contexts + where name = 'RI fast-path finfo scratch') as metadata_cached; + metadata_cached +----------------- + f +(1 row) + +insert into warm values (2); +insert into warm values (99); +ERROR: insert or update on table "warm" violates foreign key constraint "warm_k_fkey" +DETAIL: Key (k)=(99) is not present in table "p". +-- This constraint has no cached fast-path metadata yet. +insert into cold values (2); +insert into cold values (99); +ERROR: insert or update on table "cold" violates foreign key constraint "cold_k_fkey" +DETAIL: Key (k)=(99) is not present in table "p". +select * from warm order by k; + k +--- + 1 + 2 +(2 rows) + +select * from cold order by k; + k +--- + 2 +(1 row) + +-- Change the family between AFTER triggers. The RI trigger sorts first, so +-- rows buffered before the DDL must also be checked through SPI at batch end. +alter operator family fam using btree drop operator 3(integer,bigint); +alter operator family fam using btree add operator 3 =(integer,bigint); +create table batch(k bigint references p(k)); +create function change_family() returns trigger language plpgsql as $$ +begin + if new.k = 1 then + alter operator family fam using btree drop operator 3(integer,bigint); + alter operator family fam using btree add operator 3 =#=(integer,bigint); + end if; + return null; +end +$$; +create trigger zzz_change_family after insert on batch + for each row execute function change_family(); +begin; +insert into batch values (1), (2); +select * from batch order by k; + k +--- + 1 + 2 +(2 rows) + +rollback; +-- No later RI trigger is needed to notice that the index became unsuitable. +begin; +insert into batch values (1); +select * from batch; + k +--- + 1 +(1 row) + +rollback; +-- Reject missing keys both after and before the DDL boundary. +insert into batch values (1), (99); +ERROR: insert or update on table "batch" violates foreign key constraint "batch_k_fkey" +DETAIL: Key (k)=(99) is not present in table "p". +insert into batch values (99), (1); +ERROR: insert or update on table "batch" violates foreign key constraint "batch_k_fkey" +DETAIL: Key (k)=(99) is not present in table "p". +select * from batch; + k +--- +(0 rows) + +reset search_path; +drop table fk_opfamily.warm, fk_opfamily.cold, fk_opfamily.batch, fk_opfamily.p; +drop function fk_opfamily.change_family(); +drop operator class fk_opfamily.int_ops using btree; +drop operator family fk_opfamily.fam using btree; +drop operator fk_opfamily.=#=(integer,bigint); +drop schema fk_opfamily; -- -- 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 07d8921931..789d221361 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -692,6 +692,94 @@ 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 loose cross-type operator family member must invalidate the +-- fast-path metadata. The FK continues using its stored equality operator, +-- which need no longer be a member of the index family. Use the very same +-- implementation for the replacement, keeping the family semantics unchanged. +create schema fk_opfamily; +set search_path = fk_opfamily, pg_catalog; +create operator family fam using btree; +create operator class int_ops for type integer using btree family fam as + operator 1 <(integer,integer), operator 2 <=(integer,integer), + operator 3 =(integer,integer), operator 4 >=(integer,integer), + operator 5 >(integer,integer), function 1 btint4cmp(integer,integer); +alter operator family fam using btree add + operator 1 <(integer,bigint), operator 2 <=(integer,bigint), + operator 3 =(integer,bigint), operator 4 >=(integer,bigint), + operator 5 >(integer,bigint), + operator 1 <(bigint,integer), operator 2 <=(bigint,integer), + operator 3 =(bigint,integer), operator 4 >=(bigint,integer), + operator 5 >(bigint,integer), + operator 1 <(bigint,bigint), operator 2 <=(bigint,bigint), + operator 3 =(bigint,bigint), operator 4 >=(bigint,bigint), + operator 5 >(bigint,bigint), + function 1 (integer,bigint) btint48cmp(integer,bigint), + function 1 (bigint,integer) btint84cmp(bigint,integer), + function 1 (bigint,bigint) btint8cmp(bigint,bigint); +create operator =#= (leftarg=integer, rightarg=bigint, function=int48eq); +create table p(k integer); +create unique index p_idx on p(k int_ops); +create table warm(k bigint references p(k)); +create table cold(k bigint references p(k)); +insert into p values (1), (2); +insert into warm values (1); +select amvalidate(oid) from pg_opclass +where opcnamespace = 'fk_opfamily'::regnamespace; +select exists (select from pg_backend_memory_contexts + where name = 'RI fast-path finfo scratch') as metadata_cached; +-- Change only pg_amop after warming the cache. +begin; +alter operator family fam using btree drop operator 3(integer,bigint); +alter operator family fam using btree add operator 3 =#=(integer,bigint); +commit; +select amvalidate(oid) from pg_opclass +where opcnamespace = 'fk_opfamily'::regnamespace; +select exists (select from pg_backend_memory_contexts + where name = 'RI fast-path finfo scratch') as metadata_cached; +insert into warm values (2); +insert into warm values (99); +-- This constraint has no cached fast-path metadata yet. +insert into cold values (2); +insert into cold values (99); +select * from warm order by k; +select * from cold order by k; +-- Change the family between AFTER triggers. The RI trigger sorts first, so +-- rows buffered before the DDL must also be checked through SPI at batch end. +alter operator family fam using btree drop operator 3(integer,bigint); +alter operator family fam using btree add operator 3 =(integer,bigint); +create table batch(k bigint references p(k)); +create function change_family() returns trigger language plpgsql as $$ +begin + if new.k = 1 then + alter operator family fam using btree drop operator 3(integer,bigint); + alter operator family fam using btree add operator 3 =#=(integer,bigint); + end if; + return null; +end +$$; +create trigger zzz_change_family after insert on batch + for each row execute function change_family(); +begin; +insert into batch values (1), (2); +select * from batch order by k; +rollback; +-- No later RI trigger is needed to notice that the index became unsuitable. +begin; +insert into batch values (1); +select * from batch; +rollback; +-- Reject missing keys both after and before the DDL boundary. +insert into batch values (1), (99); +insert into batch values (99), (1); +select * from batch; +reset search_path; +drop table fk_opfamily.warm, fk_opfamily.cold, fk_opfamily.batch, fk_opfamily.p; +drop function fk_opfamily.change_family(); +drop operator class fk_opfamily.int_ops using btree; +drop operator family fk_opfamily.fam using btree; +drop operator fk_opfamily.=#=(integer,bigint); +drop schema fk_opfamily; + -- -- Now some cases with inheritance -- Basic 2 table case: 1 column of matching types. base-commit: f4b511ae93a587982ac025c7a2512586fe87489d -- 2.50.1 (Apple Git-155)
