From: Noah Misch <noah@leadboat.com>
To: amitlangote09@gmail.com
To: zhjwpku@gmail.com
Cc: pgsql-hackers@postgresql.org
Subject: Re: ri_Fast* crash w/ nullable UNIQUE constraint
Date: Sun, 5 Jul 2026 14:47:20 -0700
Message-ID: <20260705214720.33@rfd.leadboat.com> (raw)
In-Reply-To: <20260705210533.ee.noahmisch@microsoft.com>
References: <20260705210533.ee.noahmisch@microsoft.com>
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.
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.cindex 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.outnew file mode 100644index 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_scheduleindex 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.specnew file mode 100644index 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 s1cAttachments:
[text/plain] test-fk-unique-nullable-v0.patch (6.9K, ../20260705214720.33@rfd.leadboat.com/2-test-fk-unique-nullable-v0.patch)
download | inline diff:
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.cindex 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.outnew file mode 100644index 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_scheduleindex 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.specnew file mode 100644index 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: noah@leadboat.com, amitlangote09@gmail.com, zhjwpku@gmail.com
Subject: Re: ri_Fast* crash w/ nullable UNIQUE constraint
In-Reply-To: <20260705214720.33@rfd.leadboat.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox