From 0311881433859e138271e36aec7ce99653c162cd Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Thu, 16 Jul 2026 18:16:51 +0900 Subject: [PATCH v1 1/6] Handle nullable referenced key in RI fast-path check The RI fast-path FK check asserted that the referenced key is never NULL, in ri_FastPathFlushArray() and in recheck_matched_pk_tuple(). That holds for a primary key, but a foreign key may reference any unique column, and a UNIQUE column is nullable. The assertion is reachable under READ COMMITTED. ri_LockPKTuple() locks the matched PK tuple with TUPLE_LOCK_FLAG_FIND_LAST_VERSION, so when a concurrent transaction commits a key-changing UPDATE while the check waits, the lock follows the update chain to the latest version. If that version now has NULL in the referenced column, the fast path reaches the assert; in a non-assert build it would compare against the NULL and treat it as a match. A NULL referenced key cannot equal any (non-null) FK value, so treat it as "referenced row not found" and let the ordinary foreign-key violation be raised. This matches the SPI path, whose requalifying "pkatt = $n" evaluates to NULL for such a row, so the row is not returned and the check reports a violation. Both fix sites are on the update-chain recheck path: the value read back from the chased-to tuple in ri_FastPathFlushArray() (the single-column SK_SEARCHARRAY flush), and the scan key built from that tuple in recheck_matched_pk_tuple(), which both the single-column and multi-column (ri_FastPathFlushLoop()) flush routines use to requalify a concurrently updated tuple. Add an isolation test exercising both flush routines: a concurrent UPDATE sets the referenced key to NULL while an FK INSERT waits on the row, and the INSERT must report a foreign-key violation rather than trip the assert. Reported-by: Noah Misch Discussion: https://postgr.es/m/20260705210533.ee.noahmisch@microsoft.com Backpatch-through: 19 --- src/backend/utils/adt/ri_triggers.c | 25 +++++++++--- .../expected/fk-fastpath-null-key.out | 17 ++++++++ src/test/isolation/isolation_schedule | 1 + .../isolation/specs/fk-fastpath-null-key.spec | 40 +++++++++++++++++++ 4 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 src/test/isolation/expected/fk-fastpath-null-key.out create mode 100644 src/test/isolation/specs/fk-fastpath-null-key.spec diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 627a9fb38ea..d2b969e31ae 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 00000000000..3efe1e781b2 --- /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); +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); +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 b8ebe92553c..cbc40f20713 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 00000000000..51e11b101dc --- /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 -- 2.47.3