From b8da871e385c2ea32df534aee24718668cc0e4bf Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Thu, 3 Sep 2026 22:26:43 +0900 Subject: [PATCH v1 1/2] Take RI fast-path snapshot after locking referenced relation ri_FastPathCheck() acquired its scan snapshot before opening the referenced relation. If it then waited for the relation lock in READ COMMITTED mode, a referenced row committed during the wait would not be visible to the old snapshot. The check could consequently report a foreign key violation even though the referenced row existed. The batched path does not have this problem, because it opens the relations before acquiring the snapshot used to check the batch. Consequently, batching currently masks the problem for ordinary DML. Fix the per-row path before removing batching and making that path handle those checks. Take the snapshot after opening the referenced relation and reloading the constraint information. This also agrees with the SPI path, which acquires the referenced-relation lock before selecting the snapshot used for the check. Add isolation-test coverage for the visibility of a referenced row committed after the referencing transaction has executed an earlier command. A later command can see such a row in READ COMMITTED, but not in REPEATABLE READ or SERIALIZABLE. Discussion: https://postgr.es/m/CA+HiwqEhm+_=bs=2wavAJz-UqC+1KebD31++mapJQQGweE8iQQ@mail.gmail.com Backpatch-through: 19 --- src/backend/utils/adt/ri_triggers.c | 14 +++++- src/test/isolation/expected/fk-snapshot-2.out | 44 +++++++++++++++++++ src/test/isolation/specs/fk-snapshot-2.spec | 15 +++++++ 3 files changed, 72 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 439376a6cc2..60394fb9934 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -2882,7 +2882,6 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo, * ri_PerformCheck(). */ CommandCounterIncrement(); - snapshot = RegisterSnapshot(GetTransactionSnapshot()); INJECTION_POINT("ri-before-pk-lock", NULL); @@ -2893,6 +2892,19 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo, idx_rel = index_open(riinfo->conindid, AccessShareLock); + /* + * Only now take the snapshot the scan will use. Acquiring it before + * table_open() would let an unbounded amount of time pass while we wait + * for the lock, during which another transaction can commit the very row + * we are about to look for. The scan would not see it and the check + * would report a violation for a key that exists. + * + * The SPI path does not have this problem: for this check it passes + * InvalidSnapshot, so SPI takes the snapshot after the referenced-relation + * lock has been acquired. + */ + snapshot = RegisterSnapshot(GetTransactionSnapshot()); + slot = table_slot_create(pk_rel, NULL); GetUserIdAndSecContext(&saved_userid, &saved_sec_context); diff --git a/src/test/isolation/expected/fk-snapshot-2.out b/src/test/isolation/expected/fk-snapshot-2.out index 7333643e9ac..376460a00e5 100644 --- a/src/test/isolation/expected/fk-snapshot-2.out +++ b/src/test/isolation/expected/fk-snapshot-2.out @@ -59,3 +59,47 @@ step s1c: COMMIT; step s2ins: <... completed> ERROR: could not serialize access due to concurrent delete step s2c: COMMIT; + +starting permutation: s1rr s2rr s2sel s1ins s1c s2ins2 s2c +step s1rr: BEGIN ISOLATION LEVEL REPEATABLE READ; +step s2rr: BEGIN ISOLATION LEVEL REPEATABLE READ; +step s2sel: SELECT count(*) FROM parent; +count +----- + 1 +(1 row) + +step s1ins: INSERT INTO parent VALUES (2); +step s1c: COMMIT; +step s2ins2: INSERT INTO child VALUES (2, 2); +ERROR: insert or update on table "child" violates foreign key constraint "child_parent_id_fkey" +step s2c: COMMIT; + +starting permutation: s1ser s2ser s2sel s1ins s1c s2ins2 s2c +step s1ser: BEGIN ISOLATION LEVEL SERIALIZABLE; +step s2ser: BEGIN ISOLATION LEVEL SERIALIZABLE; +step s2sel: SELECT count(*) FROM parent; +count +----- + 1 +(1 row) + +step s1ins: INSERT INTO parent VALUES (2); +step s1c: COMMIT; +step s2ins2: INSERT INTO child VALUES (2, 2); +ERROR: insert or update on table "child" violates foreign key constraint "child_parent_id_fkey" +step s2c: COMMIT; + +starting permutation: s1rc s2rc s2sel s1ins s1c s2ins2 s2c +step s1rc: BEGIN ISOLATION LEVEL READ COMMITTED; +step s2rc: BEGIN ISOLATION LEVEL READ COMMITTED; +step s2sel: SELECT count(*) FROM parent; +count +----- + 1 +(1 row) + +step s1ins: INSERT INTO parent VALUES (2); +step s1c: COMMIT; +step s2ins2: INSERT INTO child VALUES (2, 2); +step s2c: COMMIT; diff --git a/src/test/isolation/specs/fk-snapshot-2.spec b/src/test/isolation/specs/fk-snapshot-2.spec index 94cd151aab9..5b0144c0f70 100644 --- a/src/test/isolation/specs/fk-snapshot-2.spec +++ b/src/test/isolation/specs/fk-snapshot-2.spec @@ -21,6 +21,7 @@ step s1rr { BEGIN ISOLATION LEVEL REPEATABLE READ; } step s1ser { BEGIN ISOLATION LEVEL SERIALIZABLE; } step s1del { DELETE FROM parent WHERE parent_id = 1; } step s1c { COMMIT; } +step s1ins { INSERT INTO parent VALUES (2); } session s2 step s2rc { BEGIN ISOLATION LEVEL READ COMMITTED; } @@ -28,6 +29,8 @@ step s2rr { BEGIN ISOLATION LEVEL REPEATABLE READ; } step s2ser { BEGIN ISOLATION LEVEL SERIALIZABLE; } step s2ins { INSERT INTO child VALUES (1, 1); } step s2c { COMMIT; } +step s2sel { SELECT count(*) FROM parent; } +step s2ins2 { INSERT INTO child VALUES (2, 2); } # Violates referential integrity unless we use a crosscheck snapshot, # which is up-to-date compared with the transaction's snapshot. @@ -48,3 +51,15 @@ permutation s1ser s2ser s2ins s1del s2c s1c # We raise a concurrent update error # which is good enough: permutation s1ser s2ser s1del s2ins s1c s2c + +# A parent row committed by another transaction after this one took its +# snapshot. RI_FKey_check passes detectNewRows = false, so the check runs +# under the transaction snapshot rather than a current one, and the row is +# correctly not visible: referencing it is a violation. Only the parent-side +# checks need to see rows committed since the snapshot. +permutation s1rr s2rr s2sel s1ins s1c s2ins2 s2c +permutation s1ser s2ser s2sel s1ins s1c s2ins2 s2c + +# The same order in READ COMMITTED, where the check's snapshot is a fresh one +# and the parent row is visible. +permutation s1rc s2rc s2sel s1ins s1c s2ins2 s2c -- 2.47.3