v1-0002-refint-parameterize-cascade-UPDATE-new-key-values.patch

From c4331d930e94803e180f335e0dad5f9b8a0b5717 Mon Sep 17 00:00:00 2001
From: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Date: Tue, 12 May 2026 17:53:11 +0000
Subject: [PATCH v1 2/2] refint: parameterize cascade UPDATE new key values

Previously, contrib/spi/refint's check_foreign_key() embedded the new
key values into the prepared cascade UPDATE query text as literals.
The preceding patch fixed the immediate NULL crash, but the same code
path has two other latent issues:

* Because the new values were baked into the prepared SQL, the cached
  plan reused the first new key values for every subsequent UPDATE.
  Adding the operation type to the plan cache key (commit 8cfbdf8f4df)
  helped distinguish update vs delete, but the SET RHS was still
  hard-coded into each cached plan.

* Char-like new values were wrapped in single quotes without escaping,
  so a new value containing a single quote produced malformed SQL.

Switch the cascade UPDATE path to use SPI parameters for the new key
values.  This naturally handles NULLs via the SPI nulls array, lets a
single cached plan handle any subsequent UPDATE, and avoids quoting
issues altogether.

Add a refint regression test that runs two cascade UPDATEs in a row to
exercise cached-plan reuse with different key values, including a NULL.

Discussion: https://postgr.es/m/19476-bd04ea6241345303@postgresql.org
---
 contrib/spi/expected/refint.out | 31 +++++++++++++
 contrib/spi/refint.c            | 77 ++++++++++++---------------------
 contrib/spi/sql/refint.sql      | 28 ++++++++++++
 3 files changed, 87 insertions(+), 49 deletions(-)

diff --git a/contrib/spi/expected/refint.out b/contrib/spi/expected/refint.out
index 79633603217..16b8990a80d 100644
--- a/contrib/spi/expected/refint.out
+++ b/contrib/spi/expected/refint.out
@@ -111,3 +111,34 @@ SELECT trigger_name, event_manipulation, event_object_schema, event_object_table
 DROP TABLE pkeys;
 DROP TABLE fkeys;
 DROP TABLE fkeys2;
+-- Cascading updates should use parameters for new key values so that the
+-- cached plan can be reused for subsequent updates with different keys.
+CREATE TABLE c_null (id int4);
+CREATE UNIQUE INDEX c_null_i ON c_null(id);
+CREATE TABLE b_null (refb int4);
+CREATE INDEX b_null_i ON b_null(refb);
+CREATE TRIGGER c_null_fkey_cascade
+	after delete or update on c_null
+	for each row
+	execute function check_foreign_key (1, 'cascade', 'id', 'b_null', 'refb');
+CREATE TRIGGER b_null_pkey_exist
+	after insert or update on b_null
+	for each row
+	execute function check_primary_key ('refb', 'c_null', 'id');
+INSERT INTO c_null VALUES (10);
+INSERT INTO b_null VALUES (10);
+UPDATE c_null SET id = NULL WHERE id = 10;
+NOTICE:  c_null_fkey_cascade: 1 tuple(s) of b_null are updated
+INSERT INTO c_null VALUES (20);
+INSERT INTO b_null VALUES (20);
+UPDATE c_null SET id = 30 WHERE id = 20;
+NOTICE:  c_null_fkey_cascade: 1 tuple(s) of b_null are updated
+SELECT * FROM b_null ORDER BY refb NULLS FIRST;
+ refb 
+------
+     
+   30
+(2 rows)
+
+DROP TABLE c_null;
+DROP TABLE b_null;
diff --git a/contrib/spi/refint.c b/contrib/spi/refint.c
index 0063410f27e..bc89015a12c 100644
--- a/contrib/spi/refint.c
+++ b/contrib/spi/refint.c
@@ -248,11 +248,12 @@ check_foreign_key(PG_FUNCTION_ARGS)
 	Trigger    *trigger;		/* to get trigger name */
 	int			nargs;			/* # of args specified in CREATE TRIGGER */
 	char	  **args;			/* arguments: as described above */
-	char	  **args_temp;
 	int			nrefs;			/* number of references (== # of plans) */
 	char		action;			/* 'R'estrict | 'S'etnull | 'C'ascade */
 	int			nkeys;			/* # of key columns */
+	int			nparams;		/* # of query parameters */
 	Datum	   *kvals;			/* key values */
+	char	   *nulls = NULL;	/* null markers for key values */
 	char	   *relname;		/* referencing relation name */
 	Relation	rel;			/* triggered relation */
 	HeapTuple	trigtuple = NULL;	/* tuple to being changed */
@@ -343,7 +344,13 @@ check_foreign_key(PG_FUNCTION_ARGS)
 	 * We use SPI plan preparation feature, so allocate space to place key
 	 * values.
 	 */
-	kvals = (Datum *) palloc(nkeys * sizeof(Datum));
+	nparams = (action == 'c' && is_update) ? 2 * nkeys : nkeys;
+	kvals = (Datum *) palloc(nparams * sizeof(Datum));
+	if (nparams > nkeys)
+	{
+		nulls = (char *) palloc(nparams * sizeof(char));
+		memset(nulls, ' ', nparams * sizeof(char));
+	}
 
 	/*
 	 * Construct ident string as TriggerName $ TriggeredRelationId $
@@ -354,7 +361,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
 
 	/* if there is no plan(s) then allocate argtypes for preparation */
 	if (plan->nplans <= 0)
-		argtypes = (Oid *) palloc(nkeys * sizeof(Oid));
+		argtypes = (Oid *) palloc(nparams * sizeof(Oid));
 
 	/*
 	 * else - check that we have exactly nrefs plan(s) ready
@@ -391,6 +398,9 @@ check_foreign_key(PG_FUNCTION_ARGS)
 			return PointerGetDatum((newtuple == NULL) ? trigtuple : newtuple);
 		}
 
+		if (plan->nplans <= 0)
+			argtypes[i] = SPI_gettypeid(tupdesc, fnumber);
+
 		/*
 		 * If UPDATE then get column value from new tuple being inserted and
 		 * compare is this the same as old one. For the moment we use string
@@ -398,6 +408,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
 		 */
 		if (newtuple != NULL)
 		{
+			bool		newisnull;
 			char	   *oldval = SPI_getvalue(trigtuple, tupdesc, fnumber);
 			char	   *newval;
 
@@ -408,12 +419,17 @@ check_foreign_key(PG_FUNCTION_ARGS)
 			newval = SPI_getvalue(newtuple, tupdesc, fnumber);
 			if (newval == NULL || strcmp(oldval, newval) != 0)
 				isequal = false;
-		}
 
-		if (plan->nplans <= 0)	/* Get typeId of column */
-			argtypes[i] = SPI_gettypeid(tupdesc, fnumber);
+			if (action == 'c')
+			{
+				kvals[nkeys + i] = SPI_getbinval(newtuple, tupdesc,
+												 fnumber, &newisnull);
+				nulls[nkeys + i] = newisnull ? 'n' : ' ';
+				if (plan->nplans <= 0)
+					argtypes[nkeys + i] = argtypes[i];
+			}
+		}
 	}
-	args_temp = args;
 	nargs -= nkeys;
 	args += nkeys;
 
@@ -468,50 +484,14 @@ check_foreign_key(PG_FUNCTION_ARGS)
 			{
 				if (is_update == 1)
 				{
-					int			fn;
-					char	   *nv;
 					int			k;
 
 					snprintf(sql, sizeof(sql), "update %s set ", relname);
 					for (k = 1; k <= nkeys; k++)
 					{
-						int			is_char_type = 0;
-						char	   *type;
-
-						fn = SPI_fnumber(tupdesc, args_temp[k - 1]);
-						Assert(fn > 0); /* already checked above */
-						nv = SPI_getvalue(newtuple, tupdesc, fn);
-						type = SPI_gettype(tupdesc, fn);
-
-						if (strcmp(type, "text") == 0 ||
-							strcmp(type, "varchar") == 0 ||
-							strcmp(type, "char") == 0 ||
-							strcmp(type, "bpchar") == 0 ||
-							strcmp(type, "date") == 0 ||
-							strcmp(type, "timestamp") == 0)
-							is_char_type = 1;
-#ifdef	DEBUG_QUERY
-						elog(DEBUG4, "check_foreign_key Debug value %s type %s %d",
-							 nv, type, is_char_type);
-#endif
-
-						/*
-						 * is_char_type =1 i set ' ' for define a new value.
-						 *
-						 * SPI_getvalue() returns NULL for SQL NULL values, so
-						 * emit the NULL keyword rather than passing a NULL
-						 * pointer to snprintf("%s"), which is undefined
-						 * behavior.
-						 */
-						if (nv == NULL)
-							snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql),
-									 " %s = NULL %s ",
-									 args2[k], (k < nkeys) ? ", " : "");
-						else
-							snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql),
-									 " %s = %s%s%s %s ",
-									 args2[k], (is_char_type > 0) ? "'" : "",
-									 nv, (is_char_type > 0) ? "'" : "", (k < nkeys) ? ", " : "");
+						snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql),
+								 " %s = $%d %s ",
+								 args2[k], nkeys + k, (k < nkeys) ? ", " : "");
 					}
 					strcat(sql, " where ");
 				}
@@ -546,7 +526,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
 			}
 
 			/* Prepare plan for query */
-			pplan = SPI_prepare(sql, nkeys, argtypes);
+			pplan = SPI_prepare(sql, nparams, argtypes);
 			if (pplan == NULL)
 				/* internal error */
 				elog(ERROR, "check_foreign_key: SPI_prepare returned %s", SPI_result_code_string(SPI_result));
@@ -591,8 +571,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
 
 		relname = args[0];
 
-		ret = SPI_execp(plan->splan[r], kvals, NULL, tcount);
-		/* we have no NULLs - so we pass   ^^^^  here */
+		ret = SPI_execp(plan->splan[r], kvals, nulls, tcount);
 
 		if (ret < 0)
 			ereport(ERROR,
diff --git a/contrib/spi/sql/refint.sql b/contrib/spi/sql/refint.sql
index 63458127917..d460ad58d53 100644
--- a/contrib/spi/sql/refint.sql
+++ b/contrib/spi/sql/refint.sql
@@ -95,3 +95,31 @@ SELECT trigger_name, event_manipulation, event_object_schema, event_object_table
 DROP TABLE pkeys;
 DROP TABLE fkeys;
 DROP TABLE fkeys2;
+
+-- Cascading updates should use parameters for new key values so that the
+-- cached plan can be reused for subsequent updates with different keys.
+CREATE TABLE c_null (id int4);
+CREATE UNIQUE INDEX c_null_i ON c_null(id);
+CREATE TABLE b_null (refb int4);
+CREATE INDEX b_null_i ON b_null(refb);
+
+CREATE TRIGGER c_null_fkey_cascade
+	after delete or update on c_null
+	for each row
+	execute function check_foreign_key (1, 'cascade', 'id', 'b_null', 'refb');
+
+CREATE TRIGGER b_null_pkey_exist
+	after insert or update on b_null
+	for each row
+	execute function check_primary_key ('refb', 'c_null', 'id');
+
+INSERT INTO c_null VALUES (10);
+INSERT INTO b_null VALUES (10);
+UPDATE c_null SET id = NULL WHERE id = 10;
+INSERT INTO c_null VALUES (20);
+INSERT INTO b_null VALUES (20);
+UPDATE c_null SET id = 30 WHERE id = 20;
+SELECT * FROM b_null ORDER BY refb NULLS FIRST;
+
+DROP TABLE c_null;
+DROP TABLE b_null;
-- 
2.43.0