From 8fb5176373c5a3fff05c4bea281397efc4e69884 Mon Sep 17 00:00:00 2001 From: Ayush Tiwari Date: Fri, 15 May 2026 06:06:33 +0000 Subject: [PATCH v2] refint: Remove plan cache from check_foreign_key(). check_foreign_key() had a per-trigger plan cache that was unsafe in two ways. The cascade UPDATE query text embeds the current NEW key values directly in its SET clause and so cannot be reused for later UPDATEs. The cache also had no invalidation mechanism, so dropping a trigger and recreating it with the same name but different arguments (for example, a different number of references) would still find stale plans and either reuse them or fail with "# of plans changed in meantime". refint is example code, so just prepare the SPI plans afresh on each trigger invocation and let SPI_finish() release them. Add regression coverage for both issues. --- contrib/spi/expected/refint.out | 56 +++++++++++++++++++++++++++++++++ contrib/spi/refint.c | 55 ++++++++++---------------------- contrib/spi/sql/refint.sql | 55 ++++++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 38 deletions(-) diff --git a/contrib/spi/expected/refint.out b/contrib/spi/expected/refint.out index 79633603217..2aec5aac6ad 100644 --- a/contrib/spi/expected/refint.out +++ b/contrib/spi/expected/refint.out @@ -88,6 +88,62 @@ NOTICE: check_pkeys_fkey_cascade: 1 tuple(s) of fkeys2 are updated update pkeys set pkey1 = 7, pkey2 = '70' where pkey1 = 10 and pkey2 = '1'; ERROR: duplicate key value violates unique constraint "pkeys_i" DETAIL: Key (pkey1, pkey2)=(7, 70) already exists. +-- Cascade UPDATE plans must use the current new key values. +create table pkeys3 (pkey int4 not null); +create unique index pkeys3_i on pkeys3 (pkey); +create table fkeys3 (fkey int4); +create index fkeys3_i on fkeys3 (fkey); +create trigger check_pkeys3_fkey_cascade + after delete or update on pkeys3 + for each row + execute function check_foreign_key (1, 'cascade', 'pkey', 'fkeys3', 'fkey'); +create trigger check_fkeys3_pkey_exist + after insert or update on fkeys3 + for each row + execute function check_primary_key ('fkey', 'pkeys3', 'pkey'); +insert into pkeys3 values (1), (2); +insert into fkeys3 values (1), (2); +update pkeys3 set pkey = 11 where pkey = 1; +NOTICE: check_pkeys3_fkey_cascade: 1 tuple(s) of fkeys3 are updated +update pkeys3 set pkey = 22 where pkey = 2; +NOTICE: check_pkeys3_fkey_cascade: 1 tuple(s) of fkeys3 are updated +select * from fkeys3 order by fkey; + fkey +------ + 11 + 22 +(2 rows) + +-- Re-creating a trigger with a different number of references must work +-- (the old code cached plans by trigger name, so this raised +-- "# of plans changed in meantime"). +create table pkeys4 (pkey int4 not null); +create unique index pkeys4_i on pkeys4 (pkey); +create table fkeys4a (fkey int4); +create index fkeys4a_i on fkeys4a (fkey); +create table fkeys4b (fkey int4); +create index fkeys4b_i on fkeys4b (fkey); +create trigger check_pkeys4_fkey_cascade + after delete or update on pkeys4 + for each row + execute function check_foreign_key (2, 'cascade', 'pkey', 'fkeys4a', 'fkey', 'fkeys4b', 'fkey'); +insert into pkeys4 values (1); +insert into fkeys4a values (1); +insert into fkeys4b values (1); +delete from pkeys4 where pkey = 1; +NOTICE: check_pkeys4_fkey_cascade: 1 tuple(s) of fkeys4a are deleted +NOTICE: check_pkeys4_fkey_cascade: 1 tuple(s) of fkeys4b are deleted +drop trigger check_pkeys4_fkey_cascade on pkeys4; +create trigger check_pkeys4_fkey_cascade + after delete or update on pkeys4 + for each row + execute function check_foreign_key (1, 'cascade', 'pkey', 'fkeys4a', 'fkey'); +insert into pkeys4 values (2); +insert into fkeys4a values (2); +delete from pkeys4 where pkey = 2; +NOTICE: check_pkeys4_fkey_cascade: 1 tuple(s) of fkeys4a are deleted +drop table pkeys3, fkeys3; +drop table pkeys4, fkeys4a, fkeys4b; SELECT trigger_name, event_manipulation, event_object_schema, event_object_table, action_order, action_condition, action_orientation, action_timing, action_reference_old_table, action_reference_new_table diff --git a/contrib/spi/refint.c b/contrib/spi/refint.c index 48512a664d2..83a9aca9fd9 100644 --- a/contrib/spi/refint.c +++ b/contrib/spi/refint.c @@ -27,8 +27,6 @@ typedef struct SPIPlanPtr *splan; } EPlan; -static EPlan *FPlans = NULL; -static int nFPlans = 0; static EPlan *PPlans = NULL; static int nPPlans = 0; @@ -263,11 +261,10 @@ check_foreign_key(PG_FUNCTION_ARGS) HeapTuple trigtuple = NULL; /* tuple to being changed */ HeapTuple newtuple = NULL; /* tuple to return */ TupleDesc tupdesc; /* tuple description */ - EPlan *plan; /* prepared plan(s) */ - Oid *argtypes = NULL; /* key types to prepare execution plan */ + SPIPlanPtr *splan; /* prepared plan(s) for this call */ + Oid *argtypes; /* key types to prepare execution plan */ bool isnull; /* to know is some column NULL or not */ bool isequal = true; /* are keys in both tuples equal (in UPDATE) */ - char ident[2 * NAMEDATALEN]; /* to identify myself */ int is_update = 0; int ret; int i, @@ -351,23 +348,16 @@ check_foreign_key(PG_FUNCTION_ARGS) kvals = (Datum *) palloc(nkeys * sizeof(Datum)); /* - * Construct ident string as TriggerName $ TriggeredRelationId $ - * OperationType and try to find prepared execution plan(s). + * SPI plans are prepared per trigger invocation and released by + * SPI_finish() below. Earlier versions of this code cached plans across + * invocations keyed by trigger name, but that was unsafe in two ways: the + * cascade UPDATE query text embeds the current NEW key values in its SET + * clause and so cannot be reused for later UPDATEs, and the cache had no + * invalidation mechanism, so a trigger dropped and recreated with the same + * name but different arguments would still find the stale plans. refint + * is example code, so we just keep things simple here. */ - snprintf(ident, sizeof(ident), "%s$%u$%c", trigger->tgname, rel->rd_id, is_update ? 'U' : 'D'); - plan = find_plan(ident, &FPlans, &nFPlans); - - /* if there is no plan(s) then allocate argtypes for preparation */ - if (plan->nplans <= 0) - argtypes = (Oid *) palloc(nkeys * sizeof(Oid)); - - /* - * else - check that we have exactly nrefs plan(s) ready - */ - else if (plan->nplans != nrefs) - /* internal error */ - elog(ERROR, "%s: check_foreign_key: # of plans changed in meantime", - trigger->tgname); + argtypes = (Oid *) palloc(nkeys * sizeof(Oid)); /* For each column in key ... */ for (i = 0; i < nkeys; i++) @@ -415,23 +405,21 @@ check_foreign_key(PG_FUNCTION_ARGS) isequal = false; } - if (plan->nplans <= 0) /* Get typeId of column */ - argtypes[i] = SPI_gettypeid(tupdesc, fnumber); + /* Get typeId of column */ + argtypes[i] = SPI_gettypeid(tupdesc, fnumber); } args_temp = args; nargs -= nkeys; args += nkeys; /* - * If we have to prepare plans ... + * Prepare an SPI plan for each referencing relation. */ - if (plan->nplans <= 0) { SPIPlanPtr pplan; char **args2 = args; - plan->splan = (SPIPlanPtr *) MemoryContextAlloc(TopMemoryContext, - nrefs * sizeof(SPIPlanPtr)); + splan = (SPIPlanPtr *) palloc(nrefs * sizeof(SPIPlanPtr)); for (r = 0; r < nrefs; r++) { @@ -531,15 +519,7 @@ check_foreign_key(PG_FUNCTION_ARGS) /* internal error */ elog(ERROR, "check_foreign_key: SPI_prepare returned %s", SPI_result_code_string(SPI_result)); - /* - * Remember that SPI_prepare places plan in current memory context - * - so, we have to save plan in Top memory context for later use. - */ - if (SPI_keepplan(pplan)) - /* internal error */ - elog(ERROR, "check_foreign_key: SPI_keepplan failed"); - - plan->splan[r] = pplan; + splan[r] = pplan; args2 += nkeys + 1; /* to the next relation */ @@ -549,7 +529,6 @@ check_foreign_key(PG_FUNCTION_ARGS) pfree(sql.data); } - plan->nplans = nrefs; } /* @@ -574,7 +553,7 @@ check_foreign_key(PG_FUNCTION_ARGS) relname = args[0]; - ret = SPI_execp(plan->splan[r], kvals, NULL, tcount); + ret = SPI_execp(splan[r], kvals, NULL, tcount); /* we have no NULLs - so we pass ^^^^ here */ if (ret < 0) diff --git a/contrib/spi/sql/refint.sql b/contrib/spi/sql/refint.sql index 63458127917..bbd90ff7e8b 100644 --- a/contrib/spi/sql/refint.sql +++ b/contrib/spi/sql/refint.sql @@ -85,6 +85,61 @@ delete from pkeys where pkey1 = 40 and pkey2 = '4'; update pkeys set pkey1 = 7, pkey2 = '70' where pkey1 = 50 and pkey2 = '5'; update pkeys set pkey1 = 7, pkey2 = '70' where pkey1 = 10 and pkey2 = '1'; +-- Cascade UPDATE plans must use the current new key values. +create table pkeys3 (pkey int4 not null); +create unique index pkeys3_i on pkeys3 (pkey); +create table fkeys3 (fkey int4); +create index fkeys3_i on fkeys3 (fkey); + +create trigger check_pkeys3_fkey_cascade + after delete or update on pkeys3 + for each row + execute function check_foreign_key (1, 'cascade', 'pkey', 'fkeys3', 'fkey'); + +create trigger check_fkeys3_pkey_exist + after insert or update on fkeys3 + for each row + execute function check_primary_key ('fkey', 'pkeys3', 'pkey'); + +insert into pkeys3 values (1), (2); +insert into fkeys3 values (1), (2); +update pkeys3 set pkey = 11 where pkey = 1; +update pkeys3 set pkey = 22 where pkey = 2; +select * from fkeys3 order by fkey; + +-- Re-creating a trigger with a different number of references must work +-- (the old code cached plans by trigger name, so this raised +-- "# of plans changed in meantime"). +create table pkeys4 (pkey int4 not null); +create unique index pkeys4_i on pkeys4 (pkey); +create table fkeys4a (fkey int4); +create index fkeys4a_i on fkeys4a (fkey); +create table fkeys4b (fkey int4); +create index fkeys4b_i on fkeys4b (fkey); + +create trigger check_pkeys4_fkey_cascade + after delete or update on pkeys4 + for each row + execute function check_foreign_key (2, 'cascade', 'pkey', 'fkeys4a', 'fkey', 'fkeys4b', 'fkey'); + +insert into pkeys4 values (1); +insert into fkeys4a values (1); +insert into fkeys4b values (1); +delete from pkeys4 where pkey = 1; + +drop trigger check_pkeys4_fkey_cascade on pkeys4; +create trigger check_pkeys4_fkey_cascade + after delete or update on pkeys4 + for each row + execute function check_foreign_key (1, 'cascade', 'pkey', 'fkeys4a', 'fkey'); + +insert into pkeys4 values (2); +insert into fkeys4a values (2); +delete from pkeys4 where pkey = 2; + +drop table pkeys3, fkeys3; +drop table pkeys4, fkeys4a, fkeys4b; + SELECT trigger_name, event_manipulation, event_object_schema, event_object_table, action_order, action_condition, action_orientation, action_timing, action_reference_old_table, action_reference_new_table -- 2.43.0