From a2b4102a21ad730ce46b059acf49d72151e979f6 Mon Sep 17 00:00:00 2001
From: =?utf8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Thu, 11 Sep 2025 18:11:46 +0200
Subject: [PATCH] Fill testing gap for possible referential integrity violation

This commit adds a missing isolation test for (non-PERIOD) foreign keys.
With REPEATABLE READ, one transaction can insert a referencing row while
another deletes the referenced row, and both see a valid state.  But
after they have committed, the table violates referential integrity.

If the INSERT precedes the DELETE, we use a crosscheck snapshot to see
the just-added row, so that the DELETE can raise a foreign key error.
You can see the table violate referential integrity if you change
ri_restrict to pass false for detectNewRows to ri_PerformCheck.

A crosscheck snapshot is not needed when the DELETE comes first, because
the INSERT's trigger takes a FOR KEY SHARE lock that sees the row now
marked for deletion, waits for that transaction to commit, and raises a
serialization error.  I (Paul) added a test for that too though.

We already have a similar test (in ri-triggers.spec) for SERIALIZABLE
snapshot isolation showing that you can implement foreign keys with just
pl/pgSQL, but that test does nothing to validate ri_triggers.c.  We also
have tests (in fk-snapshot.spec) for other concurrency scenarios, but
not this one: we test concurrently deleting both the referencing and
referenced row, when the constraint activates a cascade/set null action.
But those tests don't exercise ri_restrict, and the consequence of
omitting a crosscheck comparison is different: a serialization failure,
not a referential integrity violation.

Author: Paul Jungwirth <pj@illuminatedcomputing.com>
Reviewed-by: Rustam ALLAKOV <rustamallakov@gmail.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/CA+renyUp=xja80rBaB6NpY3RRdi750y046x28bo_xg29zKY72Q@mail.gmail.com
---
 src/test/isolation/expected/fk-snapshot-2.out | 61 +++++++++++++++++++
 src/test/isolation/isolation_schedule         |  1 +
 src/test/isolation/specs/fk-snapshot-2.spec   | 50 +++++++++++++++
 3 files changed, 112 insertions(+)
 create mode 100644 src/test/isolation/expected/fk-snapshot-2.out
 create mode 100644 src/test/isolation/specs/fk-snapshot-2.spec

diff --git a/src/test/isolation/expected/fk-snapshot-2.out b/src/test/isolation/expected/fk-snapshot-2.out
new file mode 100644
index 0000000000..0a4c9646fc
--- /dev/null
+++ b/src/test/isolation/expected/fk-snapshot-2.out
@@ -0,0 +1,61 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1rr s2rr s2ins s1del s2c s1c
+step s1rr: BEGIN ISOLATION LEVEL REPEATABLE READ;
+step s2rr: BEGIN ISOLATION LEVEL REPEATABLE READ;
+step s2ins: INSERT INTO child VALUES (1, 1);
+step s1del: DELETE FROM parent WHERE parent_id = 1; <waiting ...>
+step s2c: COMMIT;
+step s1del: <... completed>
+ERROR:  update or delete on table "parent" violates foreign key constraint "child_parent_id_fkey" on table "child"
+step s1c: COMMIT;
+
+starting permutation: s1rr s2rr s1del s2ins s1c s2c
+step s1rr: BEGIN ISOLATION LEVEL REPEATABLE READ;
+step s2rr: BEGIN ISOLATION LEVEL REPEATABLE READ;
+step s1del: DELETE FROM parent WHERE parent_id = 1;
+step s2ins: INSERT INTO child VALUES (1, 1); <waiting ...>
+step s1c: COMMIT;
+step s2ins: <... completed>
+ERROR:  could not serialize access due to concurrent update
+step s2c: COMMIT;
+
+starting permutation: s1rc s2rc s2ins s1del s2c s1c
+step s1rc: BEGIN ISOLATION LEVEL READ COMMITTED;
+step s2rc: BEGIN ISOLATION LEVEL READ COMMITTED;
+step s2ins: INSERT INTO child VALUES (1, 1);
+step s1del: DELETE FROM parent WHERE parent_id = 1; <waiting ...>
+step s2c: COMMIT;
+step s1del: <... completed>
+ERROR:  update or delete on table "parent" violates foreign key constraint "child_parent_id_fkey" on table "child"
+step s1c: COMMIT;
+
+starting permutation: s1rc s2rc s1del s2ins s1c s2c
+step s1rc: BEGIN ISOLATION LEVEL READ COMMITTED;
+step s2rc: BEGIN ISOLATION LEVEL READ COMMITTED;
+step s1del: DELETE FROM parent WHERE parent_id = 1;
+step s2ins: INSERT INTO child VALUES (1, 1); <waiting ...>
+step s1c: COMMIT;
+step s2ins: <... completed>
+ERROR:  insert or update on table "child" violates foreign key constraint "child_parent_id_fkey"
+step s2c: COMMIT;
+
+starting permutation: s1ser s2ser s2ins s1del s2c s1c
+step s1ser: BEGIN ISOLATION LEVEL SERIALIZABLE;
+step s2ser: BEGIN ISOLATION LEVEL SERIALIZABLE;
+step s2ins: INSERT INTO child VALUES (1, 1);
+step s1del: DELETE FROM parent WHERE parent_id = 1; <waiting ...>
+step s2c: COMMIT;
+step s1del: <... completed>
+ERROR:  update or delete on table "parent" violates foreign key constraint "child_parent_id_fkey" on table "child"
+step s1c: COMMIT;
+
+starting permutation: s1ser s2ser s1del s2ins s1c s2c
+step s1ser: BEGIN ISOLATION LEVEL SERIALIZABLE;
+step s2ser: BEGIN ISOLATION LEVEL SERIALIZABLE;
+step s1del: DELETE FROM parent WHERE parent_id = 1;
+step s2ins: INSERT INTO child VALUES (1, 1); <waiting ...>
+step s1c: COMMIT;
+step s2ins: <... completed>
+ERROR:  could not serialize access due to concurrent update
+step s2c: COMMIT;
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 9f1e997d81..130525dfd3 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -36,6 +36,7 @@ test: fk-deadlock2
 test: fk-partitioned-1
 test: fk-partitioned-2
 test: fk-snapshot
+test: fk-snapshot-2
 test: subxid-overflow
 test: eval-plan-qual
 test: eval-plan-qual-trigger
diff --git a/src/test/isolation/specs/fk-snapshot-2.spec b/src/test/isolation/specs/fk-snapshot-2.spec
new file mode 100644
index 0000000000..94cd151aab
--- /dev/null
+++ b/src/test/isolation/specs/fk-snapshot-2.spec
@@ -0,0 +1,50 @@
+# RI Trigger test
+#
+# Test C-based referential integrity enforcement.
+# Under REPEATABLE READ we need some snapshot trickery in C,
+# or we would permit things that violate referential integrity.
+
+setup
+{
+  CREATE TABLE parent (parent_id SERIAL NOT NULL PRIMARY KEY);
+  CREATE TABLE child (
+	child_id SERIAL NOT NULL PRIMARY KEY,
+	parent_id INTEGER REFERENCES parent);
+  INSERT INTO parent VALUES(1);
+}
+
+teardown { DROP TABLE parent, child; }
+
+session s1
+step s1rc	{ BEGIN ISOLATION LEVEL READ COMMITTED; }
+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; }
+
+session s2
+step s2rc	{ BEGIN ISOLATION LEVEL READ COMMITTED; }
+step s2rr	{ BEGIN ISOLATION LEVEL REPEATABLE READ; }
+step s2ser	{ BEGIN ISOLATION LEVEL SERIALIZABLE; }
+step s2ins	{ INSERT INTO child VALUES (1, 1); }
+step s2c	{ COMMIT; }
+
+# Violates referential integrity unless we use a crosscheck snapshot,
+# which is up-to-date compared with the transaction's snapshot.
+permutation s1rr s2rr s2ins s1del s2c s1c
+
+# Raises a can't-serialize exception
+# when the INSERT trigger does SELECT FOR KEY SHARE:
+permutation s1rr s2rr s1del s2ins s1c s2c
+
+# Test the same scenarios in READ COMMITTED:
+# A crosscheck snapshot is not required here.
+permutation s1rc s2rc s2ins s1del s2c s1c
+permutation s1rc s2rc s1del s2ins s1c s2c
+
+# Test the same scenarios in SERIALIZABLE:
+# We should report the FK violation:
+permutation s1ser s2ser s2ins s1del s2c s1c
+# We raise a concurrent update error
+# which is good enough:
+permutation s1ser s2ser s1del s2ins s1c s2c
-- 
2.39.5

