From fd95d3fc9136800ae2018425806be682cc36ab53 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Thu, 6 Aug 2026 21:44:03 +0900
Subject: [PATCH v1] Fix cross-type foreign keys in the batched fast-path FK
 check

ri_FastPathFlushArray() rechecked a concurrently updated PK tuple with a
scan key it built itself, putting found_val, the key of the tuple it had
just locked, into sk_argument, and passing that same slot to
recheck_matched_pk_tuple().  Both operands therefore came from the locked
tuple, and since sk_argument is the operator's right-hand input, the PK
value was read as an FK value.  For a foreign key using a cross-type
equality operator, such as a "date" primary key referenced by a "timestamp"
column, that compares days against microseconds, so the recheck always
failed and a batch that had to follow an update chain reported a violation
even though the version it locked still had the key.

Remove the recheck.  For a same-type key it compared the tuple against
itself and so never rejected anything, which was harmless only because the
loop a few lines below does the real work: it already compares found_val,
read after the chain has been followed, against every buffered FK value,
with the arguments in the order the operator expects.  That makes the
recheck redundant as well as wrong.  Detection is not weakened by dropping
it, since the buffered value that led the scan to a tuple can be matched by
no other row visible to our snapshot.

ri_FastPathProbeOne() passes its original scan key, with the FK value still
in sk_argument, and was never affected; nor were single-row statements or
multi-column foreign keys, which go through it.

Add an isolation test covering the cross-type case, a permutation where
the key really does move away, and a same-type permutation that should
behave identically.

Reported-by: Peter Geoghegan <pg@bowt.ie>
Co-authored-by: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-WznQjX3GByh_Ju7unuzMcik_5PJ5D7i_=qhwk=gPEkhfVQ@mail.gmail.com
Backpatch-through: 19
---
 src/backend/utils/adt/ri_triggers.c           | 43 +++++++--------
 .../expected/fk-crosstype-recheck.out         | 37 +++++++++++++
 src/test/isolation/isolation_schedule         |  1 +
 .../isolation/specs/fk-crosstype-recheck.spec | 54 +++++++++++++++++++
 4 files changed, 111 insertions(+), 24 deletions(-)
 create mode 100644 src/test/isolation/expected/fk-crosstype-recheck.out
 create mode 100644 src/test/isolation/specs/fk-crosstype-recheck.spec

diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 627a9fb38ea..554b58ed06e 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -3178,32 +3178,24 @@ ri_FastPathFlushArray(RI_FastPathEntry *fpentry, TupleTableSlot *fk_slot,
 	{
 		Datum		found_val;
 		bool		found_null;
-		bool		concurrently_updated;
-		ScanKeyData recheck_skey[1];
 
-		if (!ri_LockPKTuple(pk_rel, pk_slot, snapshot, &concurrently_updated))
+		/*
+		 * No key recheck is needed here, so we have no use for
+		 * concurrently_updated.  Unlike ri_FastPathProbeOne(), which takes
+		 * the index scan's word for it that the tuple matches, this path
+		 * compares the key against every buffered FK value below, and it does
+		 * so using found_val, which is read out of the version we actually
+		 * locked.  A concurrent key update is therefore caught by that
+		 * comparison: the batch item that led us to this tuple is left
+		 * unmatched and reported as a violation.
+		 */
+		if (!ri_LockPKTuple(pk_rel, pk_slot, snapshot, NULL))
 			continue;
 
 		/* Extract the PK value from the matched and locked tuple */
 		found_val = slot_getattr(pk_slot, riinfo->pk_attnums[0], &found_null);
 		Assert(!found_null);
 
-		if (concurrently_updated)
-		{
-			/*
-			 * Build a single-key scankey for recheck.  We need the actual PK
-			 * value that was found, not the FK search value.
-			 */
-			ScanKeyEntryInitialize(&recheck_skey[0], 0, 1,
-								   fpmeta->strats[0],
-								   fpmeta->subtypes[0],
-								   idx_rel->rd_indcollation[0],
-								   fpmeta->regops[0],
-								   found_val);
-			if (!recheck_matched_pk_tuple(idx_rel, recheck_skey, 1, pk_slot))
-				continue;
-		}
-
 		/*
 		 * Linear scan to mark all batch items matching this PK value.
 		 * O(batch_size) per match, O(batch_size^2) worst case -- fine for the
@@ -3271,9 +3263,11 @@ ri_FastPathProbeOne(Relation pk_rel, Relation idx_rel,
  * Calls table_tuple_lock() directly with handling specific to RI checks.
  * Returns true if the tuple was successfully locked.
  *
- * Sets *concurrently_updated to true if the locked tuple was reached
- * by following an update chain (tmfd.traversed), indicating the caller
- * should recheck the key.
+ * If concurrently_updated is not NULL, sets *concurrently_updated to true
+ * if the locked tuple was reached by following an update chain
+ * (tmfd.traversed), indicating the caller should recheck the key.  Callers
+ * that compare the locked tuple's key against the value they were looking
+ * for anyway can pass NULL.
  */
 static bool
 ri_LockPKTuple(Relation pk_rel, TupleTableSlot *slot, Snapshot snap,
@@ -3283,7 +3277,8 @@ ri_LockPKTuple(Relation pk_rel, TupleTableSlot *slot, Snapshot snap,
 	TM_Result	result;
 	int			lockflags = TUPLE_LOCK_FLAG_LOCK_UPDATE_IN_PROGRESS;
 
-	*concurrently_updated = false;
+	if (concurrently_updated)
+		*concurrently_updated = false;
 
 	if (!IsolationUsesXactSnapshot())
 		lockflags |= TUPLE_LOCK_FLAG_FIND_LAST_VERSION;
@@ -3296,7 +3291,7 @@ ri_LockPKTuple(Relation pk_rel, TupleTableSlot *slot, Snapshot snap,
 	switch (result)
 	{
 		case TM_Ok:
-			if (tmfd.traversed)
+			if (tmfd.traversed && concurrently_updated)
 				*concurrently_updated = true;
 			return true;
 
diff --git a/src/test/isolation/expected/fk-crosstype-recheck.out b/src/test/isolation/expected/fk-crosstype-recheck.out
new file mode 100644
index 00000000000..875dc856bce
--- /dev/null
+++ b/src/test/isolation/expected/fk-crosstype-recheck.out
@@ -0,0 +1,37 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1b s1away s1back s2ins s1c s2sel
+step s1b: BEGIN;
+step s1away: UPDATE fkct_pk SET k = '2020-06-01' WHERE payload = 'p1';
+step s1back: UPDATE fkct_pk SET k = '2020-01-01' WHERE payload = 'p1';
+step s2ins: INSERT INTO fkct_fk SELECT g, '2020-01-01'::timestamp FROM generate_series(1,2) g; <waiting ...>
+step s1c: COMMIT;
+step s2ins: <... completed>
+step s2sel: SELECT k FROM fkct_pk;
+         k
+----------
+01-01-2020
+(1 row)
+
+
+starting permutation: s1b s1away s2ins s1c s2sel
+step s1b: BEGIN;
+step s1away: UPDATE fkct_pk SET k = '2020-06-01' WHERE payload = 'p1';
+step s2ins: INSERT INTO fkct_fk SELECT g, '2020-01-01'::timestamp FROM generate_series(1,2) g; <waiting ...>
+step s1c: COMMIT;
+step s2ins: <... completed>
+ERROR:  insert or update on table "fkct_fk" violates foreign key constraint "fkct_fk_t_fkey"
+step s2sel: SELECT k FROM fkct_pk;
+         k
+----------
+06-01-2020
+(1 row)
+
+
+starting permutation: s1b s1aways s1backs s2inss s1c
+step s1b: BEGIN;
+step s1aways: UPDATE fkct_pk_same SET k = '2020-06-01' WHERE payload = 'p1';
+step s1backs: UPDATE fkct_pk_same SET k = '2020-01-01' WHERE payload = 'p1';
+step s2inss: INSERT INTO fkct_fk_same SELECT g, '2020-01-01'::timestamp FROM generate_series(1,2) g; <waiting ...>
+step s1c: COMMIT;
+step s2inss: <... completed>
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index df8ce44ede6..ad2042ced69 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -30,6 +30,7 @@ test: detach-partition-concurrently-2
 test: detach-partition-concurrently-3
 test: detach-partition-concurrently-4
 test: fk-contention
+test: fk-crosstype-recheck
 test: fk-deadlock
 test: fk-deadlock2
 test: fk-partitioned-1
diff --git a/src/test/isolation/specs/fk-crosstype-recheck.spec b/src/test/isolation/specs/fk-crosstype-recheck.spec
new file mode 100644
index 00000000000..5d479b8d46c
--- /dev/null
+++ b/src/test/isolation/specs/fk-crosstype-recheck.spec
@@ -0,0 +1,54 @@
+# A foreign key may use a cross-type equality operator: a "date" primary key
+# and a "timestamp" referencing column give "=(date,timestamp without time
+# zone)", whose left input is the PK type and whose right input is the FK type.
+#
+# When the referenced row is updated while a check is locking it, the check
+# has to re-check against the new version of the row.  That re-check must
+# still pass each value to the side of the operator that expects it.  A date
+# counts days and a timestamp counts microseconds, so reading one as the other
+# compares two unrelated numbers.  Both types are pass-by-value, so nothing
+# here turns on how a value is stored, only on which side it is read from.
+#
+# Below the referenced key is present the whole time -- s1 moves it away and
+# puts it back inside one transaction -- so the INSERT must succeed, exactly as
+# it does for the same-type case in the second permutation.
+
+setup
+{
+  CREATE TABLE fkct_pk (k date PRIMARY KEY, payload text);
+  CREATE TABLE fkct_fk (id int, t timestamp REFERENCES fkct_pk(k));
+  INSERT INTO fkct_pk VALUES ('2020-01-01', 'p1');
+
+  CREATE TABLE fkct_pk_same (k timestamp PRIMARY KEY, payload text);
+  CREATE TABLE fkct_fk_same (id int, t timestamp REFERENCES fkct_pk_same(k));
+  INSERT INTO fkct_pk_same VALUES ('2020-01-01', 'p1');
+}
+
+teardown
+{
+  DROP TABLE fkct_fk, fkct_pk, fkct_fk_same, fkct_pk_same;
+}
+
+session s1
+step s1b      { BEGIN; }
+step s1away   { UPDATE fkct_pk SET k = '2020-06-01' WHERE payload = 'p1'; }
+step s1back   { UPDATE fkct_pk SET k = '2020-01-01' WHERE payload = 'p1'; }
+step s1aways  { UPDATE fkct_pk_same SET k = '2020-06-01' WHERE payload = 'p1'; }
+step s1backs  { UPDATE fkct_pk_same SET k = '2020-01-01' WHERE payload = 'p1'; }
+step s1c      { COMMIT; }
+
+# Two rows in one statement, so the checks are batched -- that is what reaches
+# the re-check path under test.
+session s2
+step s2ins    { INSERT INTO fkct_fk SELECT g, '2020-01-01'::timestamp FROM generate_series(1,2) g; }
+step s2inss   { INSERT INTO fkct_fk_same SELECT g, '2020-01-01'::timestamp FROM generate_series(1,2) g; }
+step s2sel    { SELECT k FROM fkct_pk; }
+
+permutation s1b s1away s1back s2ins s1c s2sel
+
+# The mirror image: s1 leaves the key where it moved it, so the INSERT must
+# fail.  Making the re-check accept every concurrently updated tuple would
+# satisfy the permutation above while breaking this one.
+permutation s1b s1away s2ins s1c s2sel
+
+permutation s1b s1aways s1backs s2inss s1c
-- 
2.47.3

