Date: Sun, 5 Jul 2026 14:47:20 -0700
From: Noah Misch <noah@leadboat.com>
To: amitlangote09@gmail.com, zhjwpku@gmail.com
Cc: pgsql-hackers@postgresql.org
Subject: Re: ri_Fast* crash w/ nullable UNIQUE constraint
Message-ID: <20260705214720.33@rfd.leadboat.com>
References: <20260705210533.ee.noahmisch@microsoft.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="MhVYHFhmcPooip95"
Content-Disposition: inline
In-Reply-To: <20260705210533.ee.noahmisch@microsoft.com>
User-Agent: Mutt/2.3.0 (2026-01-25)
Archived-At: 
 <https://www.postgresql.org/message-id/20260705214720.33%40rfd.leadboat.com>
Precedence: bulk


--MhVYHFhmcPooip95
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sun, Jul 05, 2026 at 02:05:33PM -0700, Noah Misch wrote:
> attached demo patch shows a crash from this.  I had Opus 4.8 write the demo,
> and it included a fix that I've not vetted.  But I've vetted that reverting
> the src/backend changes and running "make -C src/test/isolation check" does
> see the crash:
> 
>   TRAP: failed Assert("!isnull[i]"), File: "ri_triggers.c", Line: 3431, PID: 297407

Here's the attachment I forgot.

--MhVYHFhmcPooip95
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=test-fk-unique-nullable-v0.patch

commit 31376ce (HEAD -> fk-fastpath-null-key)
Author:     Noah Misch <nm@cqla.c.codeql-202606a.internal>
AuthorDate: Wed Jul 1 18:56:36 2026 +0000
Commit:     Noah Misch <nm@cqla.c.codeql-202606a.internal>
CommitDate: Wed Jul 1 18:56:36 2026 +0000

    Fix RI fast-path crash when referenced key is concurrently set to NULL
    
    A foreign key may reference a nullable UNIQUE column, not only a NOT NULL
    primary key.  The RI fast-path check, however, assumed that a referenced
    key column could never be NULL ("A PK column can never be set to NULL").
    
    Under READ COMMITTED, ri_LockPKTuple() locks with
    TUPLE_LOCK_FLAG_FIND_LAST_VERSION, so it can chase an update chain to a
    newer version of the referenced row.  If a concurrent transaction set the
    referenced key to NULL and committed, the fast path follows the chain to
    that NULL version.  recheck_matched_pk_tuple() (per-row loop flush) and
    ri_FastPathFlushArray() (SK_SEARCHARRAY flush) then read a NULL key where
    they asserted one could not occur, tripping an assertion in cassert
    builds and, in production builds, dereferencing a NULL datum for
    by-reference key types or reporting a spurious violation for by-value
    types.
    
    Treat a NULL referenced key as "not a match", which is what the SPI path
    does implicitly: its requalifying "pkatt = $n" evaluates to NULL and the
    row is not returned, yielding an ordinary foreign-key violation.
    
    Add an isolation test covering both the per-row and array flush paths.
    
    The fast path and the NULL assumption were introduced by commit
    2da86c1ef9; the array-path variant came with the batching in b7b27eb41a5.
    
    Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
    Claude-Session: https://claude.ai/code/session_01Uoxu6AaEfa1135tvCA2dHJ
---
 src/backend/utils/adt/ri_triggers.c                | 25 +++++++++++---
 .../isolation/expected/fk-fastpath-null-key.out    | 17 +++++++++
 src/test/isolation/isolation_schedule              |  1 +
 src/test/isolation/specs/fk-fastpath-null-key.spec | 40 ++++++++++++++++++++++
 4 files changed, 78 insertions(+), 5 deletions(-)

diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 627a9fb..d2b969e 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -3184,9 +3184,19 @@ ri_FastPathFlushArray(RI_FastPathEntry *fpentry, TupleTableSlot *fk_slot,
 		if (!ri_LockPKTuple(pk_rel, pk_slot, snapshot, &concurrently_updated))
 			continue;
 
-		/* Extract the PK value from the matched and locked tuple */
+		/*
+		 * Extract the PK value from the matched and locked tuple.
+		 *
+		 * A foreign key may reference a nullable unique column, not just a NOT
+		 * NULL primary key.  If ri_LockPKTuple() chased an update chain to a
+		 * version whose referenced key is now NULL, that version cannot equal
+		 * any buffered (non-null) FK value, so skip it.  This mirrors the SPI
+		 * path, where the requalifying "pkatt = $n" yields NULL and the row is
+		 * not returned.
+		 */
 		found_val = slot_getattr(pk_slot, riinfo->pk_attnums[0], &found_null);
-		Assert(!found_null);
+		if (found_null)
+			continue;
 
 		if (concurrently_updated)
 		{
@@ -3427,9 +3437,14 @@ recheck_matched_pk_tuple(Relation idxrel, ScanKeyData *skeys, int nkeys,
 	{
 		ScanKeyData *skey = &skeys[i];
 
-		/* A PK column can never be set to NULL. */
-		Assert(!isnull[i]);
-		if (!DatumGetBool(FunctionCall2Coll(&skey->sk_func,
+		/*
+		 * A foreign key may reference a nullable unique column, so the version
+		 * we chased the update chain to may have a NULL in a key column.  A
+		 * NULL never equals the value we searched for, so treat it as no
+		 * match, as the SPI path's requalification would.
+		 */
+		if (isnull[i] ||
+			!DatumGetBool(FunctionCall2Coll(&skey->sk_func,
 											skey->sk_collation,
 											values[i],
 											skey->sk_argument)))
diff --git a/src/test/isolation/expected/fk-fastpath-null-key.out b/src/test/isolation/expected/fk-fastpath-null-key.out
new file mode 100644
index 0000000..3efe1e7
--- /dev/null
+++ b/src/test/isolation/expected/fk-fastpath-null-key.out
@@ -0,0 +1,17 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1b s1upd_null s2ins s1c
+step s1b: BEGIN;
+step s1upd_null: UPDATE pktable SET u = NULL WHERE u = 5;
+step s2ins: INSERT INTO fktable VALUES (5); <waiting ...>
+step s1c: COMMIT;
+step s2ins: <... completed>
+ERROR:  insert or update on table "fktable" violates foreign key constraint "fktable_a_fkey"
+
+starting permutation: s1b s1upd_null s2ins_arr s1c
+step s1b: BEGIN;
+step s1upd_null: UPDATE pktable SET u = NULL WHERE u = 5;
+step s2ins_arr: INSERT INTO fktable VALUES (5), (6); <waiting ...>
+step s1c: COMMIT;
+step s2ins_arr: <... completed>
+ERROR:  insert or update on table "fktable" violates foreign key constraint "fktable_a_fkey"
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index b8ebe92..cbc40f2 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -38,6 +38,7 @@ test: fk-snapshot
 test: fk-snapshot-2
 test: fk-snapshot-3
 test: fk-concurrent-pk-upd
+test: fk-fastpath-null-key
 test: subxid-overflow
 test: eval-plan-qual
 test: eval-plan-qual-trigger
diff --git a/src/test/isolation/specs/fk-fastpath-null-key.spec b/src/test/isolation/specs/fk-fastpath-null-key.spec
new file mode 100644
index 0000000..51e11b1
--- /dev/null
+++ b/src/test/isolation/specs/fk-fastpath-null-key.spec
@@ -0,0 +1,40 @@
+# A foreign key may reference a nullable UNIQUE column, not only a NOT NULL
+# primary key.  Test that the RI fast-path check copes when a concurrent
+# transaction sets the referenced key to NULL.
+#
+# s2's INSERT probes the PK index, finds the (u=5) row, and blocks on s1's
+# in-progress key-changing UPDATE.  After s1 commits, the tuple lock follows
+# the update chain (table_tuple_lock with TUPLE_LOCK_FLAG_FIND_LAST_VERSION)
+# to the now-NULL version.  The check must treat that as "referenced row not
+# found" and raise an ordinary foreign-key violation -- not assume that a
+# referenced key can never be NULL.
+#
+# Two permutations exercise the two fast-path flush routines: a single-row
+# INSERT goes through ri_FastPathFlushLoop()/recheck_matched_pk_tuple(), while
+# a multi-row single-column INSERT goes through ri_FastPathFlushArray().
+
+setup
+{
+  CREATE TABLE pktable (u int UNIQUE, c int);
+  CREATE TABLE fktable (a int REFERENCES pktable (u));
+  INSERT INTO pktable VALUES (5, 1), (6, 2);
+}
+
+teardown
+{
+  DROP TABLE fktable, pktable;
+}
+
+session s1
+step s1b		{ BEGIN; }
+step s1upd_null	{ UPDATE pktable SET u = NULL WHERE u = 5; }
+step s1c		{ COMMIT; }
+
+session s2
+# single-row batch -> per-row loop flush path
+step s2ins		{ INSERT INTO fktable VALUES (5); }
+# multi-row single-column batch -> SK_SEARCHARRAY flush path
+step s2ins_arr	{ INSERT INTO fktable VALUES (5), (6); }
+
+permutation s1b s1upd_null s2ins s1c
+permutation s1b s1upd_null s2ins_arr s1c

--MhVYHFhmcPooip95--