[Dmitrii Bondar <d.bondar@postgrespro.ru>]

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.

Attachments:

test.sql (text/plain, 1.1K) download
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;

v1-0001-Triggers-test-fix.patch (text/x-diff, 4.2K) download
From 57e13d3dde3b8d071347bb8f46298bd32ab2bbed Mon Sep 17 00:00:00 2001
From: Bondar Dmitrii <d.bondar@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