Hi, Hackers!
I was testing a connection pooler with `make installcheck` and noticed
that `check_foreign_key()` from the `refint` library reuses the same
cached plan for cascade `UPDATE`/`DELETE` operations. As a result, a
cascade `DELETE` is applied after an `UPDATE` command on the primary key
table (which should not happen after the commit
https://github.com/postgres/postgres/commit/d489fdfc7f4ccf0010fe0397e7272bdfc257e8f2).
I have attached a file (test.sql) to reproduce an issue and the
solution.
Regards,
Dmitrii Bondar.
CREATE FUNCTION check_primary_key ()
RETURNS trigger
AS '$libdir/refint.so'
LANGUAGE C;
CREATE FUNCTION check_foreign_key ()
RETURNS trigger
AS '$libdir/refint.so'
LANGUAGE C;
create table pkeys (pkey1 int4 not null, pkey2 text not null);
create table fkeys (fkey1 int4, fkey2 text);
create index fkeys_i on fkeys (fkey1, fkey2);
insert into pkeys values (10, '1');
insert into pkeys values (20, '2');
create unique index pkeys_i on pkeys (pkey1, pkey2);
create trigger check_fkeys_pkey_exist
before insert or update on fkeys
for each row
execute function
check_primary_key ('fkey1', 'fkey2', 'pkeys', 'pkey1', 'pkey2');
create trigger check_pkeys_fkey_cascade
before delete or update on pkeys
for each row
execute procedure
check_foreign_key (1, 'cascade', 'pkey1', 'pkey2', 'fkeys', 'fkey1',
'fkey2');
insert into fkeys values (10, '1');
insert into fkeys values (20, '2');
select * from fkeys;
delete from pkeys where pkey1 = 10 and pkey2 = '1';
-- cascade drop. Good. --
select * from fkeys;
update pkeys set pkey1 = 7, pkey2 = '70' where pkey1 = 20 and pkey2 = '2';
-- cascade drop. Bad. --
select * from fkeys;From 57e13d3dde3b8d071347bb8f46298bd32ab2bbed Mon Sep 17 00:00:00 2001
From: Bondar Dmitrii <d.bon...@postgrespro.ru>
Date: Wed, 29 Jan 2025 16:31:56 +0700
Subject: [PATCH v1] Triggers test fix
---
contrib/spi/refint.c | 23 +++++++++++++++++------
src/test/regress/expected/triggers.out | 11 ++++++-----
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/contrib/spi/refint.c b/contrib/spi/refint.c
index e1aef7cd2a..45a80bfacb 100644
--- a/contrib/spi/refint.c
+++ b/contrib/spi/refint.c
@@ -28,6 +28,7 @@ static EPlan *FPlans = NULL;
static int nFPlans = 0;
static EPlan *PPlans = NULL;
static int nPPlans = 0;
+static bool update_in_progress = false;
static EPlan *find_plan(char *ident, EPlan **eplan, int *nplans);
@@ -98,6 +99,10 @@ check_primary_key(PG_FUNCTION_ARGS)
nargs = trigger->tgnargs;
args = trigger->tgargs;
+ /* Do not check if It's a cascade update from check_foreign_key */
+ if (update_in_progress)
+ return PointerGetDatum(tuple);
+
if (nargs % 2 != 1) /* odd number of arguments! */
/* internal error */
elog(ERROR, "check_primary_key: odd number of arguments should be specified");
@@ -335,10 +340,10 @@ check_foreign_key(PG_FUNCTION_ARGS)
kvals = (Datum *) palloc(nkeys * sizeof(Datum));
/*
- * Construct ident string as TriggerName $ TriggeredRelationId and try to
- * find prepared execution plan(s).
+ * Construct ident string as TriggerName $ TriggeredRelationId $ OperationType
+ * and try to find prepared execution plan(s).
*/
- snprintf(ident, sizeof(ident), "%s$%u", trigger->tgname, rel->rd_id);
+ 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 */
@@ -570,10 +575,11 @@ check_foreign_key(PG_FUNCTION_ARGS)
relname = args[0];
- snprintf(ident, sizeof(ident), "%s$%u", trigger->tgname, rel->rd_id);
plan = find_plan(ident, &FPlans, &nFPlans);
+ update_in_progress = (is_update == 1);
ret = SPI_execp(plan->splan[r], kvals, NULL, tcount);
/* we have no NULLs - so we pass ^^^^ here */
+ update_in_progress = false;
if (ret < 0)
ereport(ERROR,
@@ -592,10 +598,15 @@ check_foreign_key(PG_FUNCTION_ARGS)
}
else
{
+ const char* operation;
+
+ if (action == 'c')
+ operation = is_update ? "updated" : "deleted";
+ else
+ operation = "set to null";
#ifdef REFINT_VERBOSE
elog(NOTICE, "%s: " UINT64_FORMAT " tuple(s) of %s are %s",
- trigger->tgname, SPI_processed, relname,
- (action == 'c') ? "deleted" : "set to null");
+ trigger->tgname, SPI_processed, relname, operation);
#endif
}
args += nkeys + 1; /* to the next relation */
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index e38304bee5..cc02b81bb2 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -116,12 +116,13 @@ delete from pkeys where pkey1 = 40 and pkey2 = '4';
NOTICE: check_pkeys_fkey_cascade: 1 tuple(s) of fkeys are deleted
NOTICE: check_pkeys_fkey_cascade: 1 tuple(s) of fkeys2 are deleted
update pkeys set pkey1 = 7, pkey2 = '70' where pkey1 = 50 and pkey2 = '5';
-NOTICE: check_pkeys_fkey_cascade: 1 tuple(s) of fkeys are deleted
-ERROR: "check_fkeys2_fkey_restrict": tuple is referenced in "fkeys"
-CONTEXT: SQL statement "delete from fkeys2 where fkey21 = $1 and fkey22 = $2 "
+NOTICE: check_pkeys_fkey_cascade: 1 tuple(s) of fkeys are updated
+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';
-NOTICE: check_pkeys_fkey_cascade: 1 tuple(s) of fkeys are deleted
-NOTICE: check_pkeys_fkey_cascade: 1 tuple(s) of fkeys2 are deleted
+NOTICE: check_pkeys_fkey_cascade: 1 tuple(s) of fkeys are updated
+NOTICE: check_pkeys_fkey_cascade: 1 tuple(s) of fkeys2 are updated
+ERROR: duplicate key value violates unique constraint "pkeys_i"
+DETAIL: Key (pkey1, pkey2)=(7, 70) already exists.
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.34.1
