v3-0001-Fill-testing-gap-for-possible-referential-integri.patch

From 27af6fb012770e06dff7d606a3cf1894cb73730e Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <pj@illuminatedcomputing.com>
Date: Thu, 15 May 2025 10:41:18 -0400
Subject: [PATCH v3 1/3] 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 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.
---
 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 00000000000..0a4c9646fca
--- /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 e3c669a29c7..12b6581d5ab 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -35,6 +35,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 00000000000..94cd151aab9
--- /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