From 38379138cbd633bc5c63e1880acc900527c49df1 Mon Sep 17 00:00:00 2001 From: Ayush Tiwari Date: Fri, 15 May 2026 00:00:00 +0000 Subject: [PATCH v1] refint: Avoid reusing cascade UPDATE plans. check_foreign_key() caches prepared plans for later trigger invocations. That is wrong for cascade UPDATE plans because the generated UPDATE query embeds the current NEW key values in its SET clause, so a second cascade UPDATE through the same trigger reuses the first UPDATE's new key value for the referencing row. Prepare those plans per execution instead, while leaving the cached plan path in place for restrict, cascade DELETE, and setnull actions. Add a regression test that performs two cascade UPDATEs with different new key values. --- contrib/spi/expected/refint.out | 28 ++++++++++++++ contrib/spi/refint.c | 67 ++++++++++++++++++++++----------- contrib/spi/sql/refint.sql | 25 ++++++++++++ 3 files changed, 99 insertions(+), 21 deletions(-) diff --git a/contrib/spi/expected/refint.out b/contrib/spi/expected/refint.out index 79633603217..cc6fc82d0ff 100644 --- a/contrib/spi/expected/refint.out +++ b/contrib/spi/expected/refint.out @@ -88,6 +88,34 @@ 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) + +drop table pkeys3; +drop table fkeys3; 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 5428b511c16..dd65cb481c9 100644 --- a/contrib/spi/refint.c +++ b/contrib/spi/refint.c @@ -263,10 +263,13 @@ 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) */ + EPlan *plan = NULL; /* prepared plan(s) */ + SPIPlanPtr *splan = NULL; /* plan(s) to execute */ Oid *argtypes = NULL; /* 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) */ + bool cache_plan; /* should the plan be saved across calls? */ + int nplans = 0; char ident[2 * NAMEDATALEN]; /* to identify myself */ int is_update = 0; int ret; @@ -351,20 +354,30 @@ 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). + * Cascade UPDATE query text includes the current new key values in its SET + * clause, so it cannot be reused for later updates. */ - snprintf(ident, sizeof(ident), "%s$%u$%c", trigger->tgname, rel->rd_id, is_update ? 'U' : 'D'); - plan = find_plan(ident, &FPlans, &nFPlans); + cache_plan = !(action == 'c' && is_update); + + if (cache_plan) + { + /* + * Construct ident string as TriggerName $ TriggeredRelationId $ + * OperationType and try to find prepared execution plan(s). + */ + snprintf(ident, sizeof(ident), "%s$%u$%c", trigger->tgname, rel->rd_id, is_update ? 'U' : 'D'); + plan = find_plan(ident, &FPlans, &nFPlans); + nplans = plan->nplans; + } /* if there is no plan(s) then allocate argtypes for preparation */ - if (plan->nplans <= 0) + if (nplans <= 0) argtypes = (Oid *) palloc(nkeys * sizeof(Oid)); /* * else - check that we have exactly nrefs plan(s) ready */ - else if (plan->nplans != nrefs) + else if (nplans != nrefs) /* internal error */ elog(ERROR, "%s: check_foreign_key: # of plans changed in meantime", trigger->tgname); @@ -415,7 +428,7 @@ check_foreign_key(PG_FUNCTION_ARGS) isequal = false; } - if (plan->nplans <= 0) /* Get typeId of column */ + if (nplans <= 0) /* Get typeId of column */ argtypes[i] = SPI_gettypeid(tupdesc, fnumber); } args_temp = args; @@ -425,13 +438,16 @@ check_foreign_key(PG_FUNCTION_ARGS) /* * If we have to prepare plans ... */ - if (plan->nplans <= 0) + if (nplans <= 0) { SPIPlanPtr pplan; char **args2 = args; - plan->splan = (SPIPlanPtr *) MemoryContextAlloc(TopMemoryContext, - nrefs * sizeof(SPIPlanPtr)); + if (cache_plan) + splan = (SPIPlanPtr *) MemoryContextAlloc(TopMemoryContext, + nrefs * sizeof(SPIPlanPtr)); + else + splan = (SPIPlanPtr *) palloc(nrefs * sizeof(SPIPlanPtr)); for (r = 0; r < nrefs; r++) { @@ -539,15 +555,18 @@ 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"); + if (cache_plan) + { + /* + * 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 */ @@ -557,8 +576,14 @@ check_foreign_key(PG_FUNCTION_ARGS) pfree(sql.data); } - plan->nplans = nrefs; + if (cache_plan) + { + plan->splan = splan; + plan->nplans = nrefs; + } } + else + splan = plan->splan; /* * If UPDATE and key is not changed ... @@ -582,7 +607,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..2d33fbf44f9 100644 --- a/contrib/spi/sql/refint.sql +++ b/contrib/spi/sql/refint.sql @@ -85,6 +85,31 @@ 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; + +drop table pkeys3; +drop table fkeys3; + 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