From ea414e6b16be36f28b4150bf1e9605f89b7edb15 Mon Sep 17 00:00:00 2001
From: amitlan <amitlangote09@gmail.com>
Date: Thu, 18 Mar 2021 14:01:27 +0900
Subject: [PATCH v1] Tweak handling of serialization failures during cascaded
 RI update/delete

The current coding for signaling a serializability failure during
cascaded update or delete of a row not visible to the start-of-the-
transaction snapshot involves a hack whereby the table AM forcefully
returns a status code that basically tells the executor that the row
has been concurrently updated such that the executor can issue an
error if needed.  However, the blocks of code that generate other
information to return about the failing row cannot do so in all cases
of the aforementioned hack, which can manifest as Assert failures in
those blocks.

To fix, simply error out immediately in the table AM itself rather
than in the caller, which avoids the gymnastics of returning the
correct failure information.

This also adds an isolation tests for some affected cases.
---
 src/backend/access/heap/heapam.c              | 13 +++++----
 .../isolation/expected/fk-serializable.out    | 19 ++++++++++++
 src/test/isolation/isolation_schedule         |  1 +
 src/test/isolation/specs/fk-serializable.spec | 29 +++++++++++++++++++
 4 files changed, 57 insertions(+), 5 deletions(-)
 create mode 100644 src/test/isolation/expected/fk-serializable.out
 create mode 100644 src/test/isolation/specs/fk-serializable.spec

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 7cb87f4a3b..baf6e2444e 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2925,8 +2925,11 @@ l1:
 	if (crosscheck != InvalidSnapshot && result == TM_Ok)
 	{
 		/* Perform additional check for transaction-snapshot mode RI updates */
+		Assert(IsolationUsesXactSnapshot());
 		if (!HeapTupleSatisfiesVisibility(&tp, crosscheck, buffer))
-			result = TM_Updated;
+				ereport(ERROR,
+						(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
+						 errmsg("could not serialize access due to concurrent update")));
 	}
 
 	if (result != TM_Ok)
@@ -3554,11 +3557,11 @@ l2:
 	if (crosscheck != InvalidSnapshot && result == TM_Ok)
 	{
 		/* Perform additional check for transaction-snapshot mode RI updates */
+		Assert(IsolationUsesXactSnapshot());
 		if (!HeapTupleSatisfiesVisibility(&oldtup, crosscheck, buffer))
-		{
-			result = TM_Updated;
-			Assert(!ItemPointerEquals(&oldtup.t_self, &oldtup.t_data->t_ctid));
-		}
+				ereport(ERROR,
+						(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
+						 errmsg("could not serialize access due to concurrent update")));
 	}
 
 	if (result != TM_Ok)
diff --git a/src/test/isolation/expected/fk-serializable.out b/src/test/isolation/expected/fk-serializable.out
new file mode 100644
index 0000000000..b7bc22a973
--- /dev/null
+++ b/src/test/isolation/expected/fk-serializable.out
@@ -0,0 +1,19 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1spk s2ifk s1dpk s1c
+step s1spk: TABLE fk;
+a              
+
+step s2ifk: INSERT INTO fk VALUES (1);
+step s1dpk: DELETE FROM pk;
+ERROR:  could not serialize access due to concurrent update
+step s1c: COMMIT;
+
+starting permutation: s1spk s2ifk s1upk s1c
+step s1spk: TABLE fk;
+a              
+
+step s2ifk: INSERT INTO fk VALUES (1);
+step s1upk: UPDATE pk SET a = a + 1;
+ERROR:  could not serialize access due to concurrent update
+step s1c: COMMIT;
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 5d6b79e66e..f97e45f0dc 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -29,6 +29,7 @@ test: fk-deadlock
 test: fk-deadlock2
 test: fk-partitioned-1
 test: fk-partitioned-2
+test: fk-serializable
 test: eval-plan-qual
 test: eval-plan-qual-trigger
 test: lock-update-delete
diff --git a/src/test/isolation/specs/fk-serializable.spec b/src/test/isolation/specs/fk-serializable.spec
new file mode 100644
index 0000000000..dfca06fe3d
--- /dev/null
+++ b/src/test/isolation/specs/fk-serializable.spec
@@ -0,0 +1,29 @@
+
+
+setup
+{
+ CREATE TABLE pk (a int PRIMARY KEY);
+ CREATE TABLE fk (a int REFERENCES pk ON DELETE CASCADE ON UPDATE CASCADE);
+ INSERT INTO pk VALUES (1);
+}
+
+teardown
+{
+ DROP TABLE fk, pk;
+}
+
+session "s1"
+
+setup	{ BEGIN ISOLATION LEVEL SERIALIZABLE; }
+step "s1spk" { TABLE fk; }
+step "s1dpk" { DELETE FROM pk; }
+step "s1upk" { UPDATE pk SET a = a + 1; }
+step "s1c"	{ COMMIT; }
+
+
+session "s2"
+
+step "s2ifk" { INSERT INTO fk VALUES (1); }
+
+permutation "s1spk" "s2ifk" "s1dpk" "s1c"
+permutation "s1spk" "s2ifk" "s1upk" "s1c"
-- 
2.24.1