Skip to content

Commit 0ec3f04

Browse files
committed
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 no match 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. Reported-by: Noah Misch <noah@leadboat.com> Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Discussion: https://postgr.es/m/20260705210533.ee.noahmisch@microsoft.com Backpatch-through: 19
1 parent dea28d4 commit 0ec3f04

4 files changed

Lines changed: 78 additions & 5 deletions

File tree

src/backend/utils/adt/ri_triggers.c

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3199,9 +3199,19 @@ ri_FastPathFlushArray(RI_FastPathEntry *fpentry, TupleTableSlot *fk_slot,
31993199
if (!ri_LockPKTuple(pk_rel, pk_slot, snapshot, &concurrently_updated))
32003200
continue;
32013201

3202-
/* Extract the PK value from the matched and locked tuple */
3202+
/*
3203+
* Extract the PK value from the matched and locked tuple.
3204+
*
3205+
* A foreign key may reference a nullable unique column, not just a
3206+
* NOT NULL primary key. If ri_LockPKTuple() chased an update chain
3207+
* to a version whose referenced key is now NULL, that version cannot
3208+
* equal any buffered (non-null) FK value, so skip it. This mirrors
3209+
* the SPI path, where the requalifying "pkatt = $n" yields NULL and
3210+
* the row is not returned.
3211+
*/
32033212
found_val = slot_getattr(pk_slot, riinfo->pk_attnums[0], &found_null);
3204-
Assert(!found_null);
3213+
if (found_null)
3214+
continue;
32053215

32063216
if (concurrently_updated)
32073217
{
@@ -3453,9 +3463,14 @@ recheck_matched_pk_tuple(Relation idxrel, ScanKeyData *skeys, int nkeys,
34533463
{
34543464
ScanKeyData *skey = &skeys[i];
34553465

3456-
/* A PK column can never be set to NULL. */
3457-
Assert(!isnull[i]);
3458-
if (!DatumGetBool(FunctionCall2Coll(&skey->sk_func,
3466+
/*
3467+
* A foreign key may reference a nullable unique column, so the
3468+
* version we chased the update chain to may have a NULL in a key
3469+
* column. A NULL never equals the value we searched for, so treat it
3470+
* as no match, as the SPI path's requalification would.
3471+
*/
3472+
if (isnull[i] ||
3473+
!DatumGetBool(FunctionCall2Coll(&skey->sk_func,
34593474
skey->sk_collation,
34603475
values[i],
34613476
skey->sk_argument)))
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
Parsed test spec with 2 sessions
2+
3+
starting permutation: s1b s1upd_null s2ins s1c
4+
step s1b: BEGIN;
5+
step s1upd_null: UPDATE pktable SET u = NULL WHERE u = 5;
6+
step s2ins: INSERT INTO fktable VALUES (5); <waiting ...>
7+
step s1c: COMMIT;
8+
step s2ins: <... completed>
9+
ERROR: insert or update on table "fktable" violates foreign key constraint "fktable_a_fkey"
10+
11+
starting permutation: s1b s1upd_null s2ins_arr s1c
12+
step s1b: BEGIN;
13+
step s1upd_null: UPDATE pktable SET u = NULL WHERE u = 5;
14+
step s2ins_arr: INSERT INTO fktable VALUES (5), (6); <waiting ...>
15+
step s1c: COMMIT;
16+
step s2ins_arr: <... completed>
17+
ERROR: insert or update on table "fktable" violates foreign key constraint "fktable_a_fkey"

src/test/isolation/isolation_schedule

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ test: fk-snapshot
3838
test: fk-snapshot-2
3939
test: fk-snapshot-3
4040
test: fk-concurrent-pk-upd
41+
test: fk-fastpath-null-key
4142
test: subxid-overflow
4243
test: eval-plan-qual
4344
test: eval-plan-qual-trigger
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# A foreign key may reference a nullable UNIQUE column, not only a NOT NULL
2+
# primary key. Test that the RI fast-path check copes when a concurrent
3+
# transaction sets the referenced key to NULL.
4+
#
5+
# s2's INSERT probes the PK index, finds the (u=5) row, and blocks on s1's
6+
# in-progress key-changing UPDATE. After s1 commits, the tuple lock follows
7+
# the update chain (table_tuple_lock with TUPLE_LOCK_FLAG_FIND_LAST_VERSION)
8+
# to the now-NULL version. The check must treat that as "referenced row not
9+
# found" and raise an ordinary foreign-key violation -- not assume that a
10+
# referenced key can never be NULL.
11+
#
12+
# Two permutations exercise the two fast-path flush routines: a single-row
13+
# INSERT goes through ri_FastPathFlushLoop()/recheck_matched_pk_tuple(), while
14+
# a multi-row single-column INSERT goes through ri_FastPathFlushArray().
15+
16+
setup
17+
{
18+
CREATE TABLE pktable (u int UNIQUE, c int);
19+
CREATE TABLE fktable (a int REFERENCES pktable (u));
20+
INSERT INTO pktable VALUES (5, 1), (6, 2);
21+
}
22+
23+
teardown
24+
{
25+
DROP TABLE fktable, pktable;
26+
}
27+
28+
session s1
29+
step s1b { BEGIN; }
30+
step s1upd_null { UPDATE pktable SET u = NULL WHERE u = 5; }
31+
step s1c { COMMIT; }
32+
33+
session s2
34+
# single-row batch -> per-row loop flush path
35+
step s2ins { INSERT INTO fktable VALUES (5); }
36+
# multi-row single-column batch -> SK_SEARCHARRAY flush path
37+
step s2ins_arr { INSERT INTO fktable VALUES (5), (6); }
38+
39+
permutation s1b s1upd_null s2ins s1c
40+
permutation s1b s1upd_null s2ins_arr s1c

0 commit comments

Comments
 (0)