From 2325ae403196f73865f7c6b3224db925ca406969 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Thu, 10 Jun 2021 23:35:16 +1200
Subject: [PATCH v3 1/2] Use tuple-level SIREAD locks for index-only scans.

When index-only scans manage to avoid fetching heap tuples,
previously we'd predicate-lock the whole heap page (since commit
cdf91edb).  Commits c01262a8 and 6f38d4da made it possible to lock the
tuple instead with only a TID, to match the behavior of regular index
scans and avoid some false conflicts.

Discussion: https://postgr.es/m/CAEepm%3D2GK3FVdnt5V3d%2Bh9njWipCv_fNL%3DwjxyUhzsF%3D0PcbNg%40mail.gmail.com
---
 src/backend/executor/nodeIndexonlyscan.c      | 12 ++++--
 .../isolation/expected/index-only-scan-2.out  | 15 +++++++
 .../isolation/expected/index-only-scan-3.out  | 16 ++++++++
 src/test/isolation/isolation_schedule         |  2 +
 .../isolation/specs/index-only-scan-2.spec    | 39 +++++++++++++++++++
 .../isolation/specs/index-only-scan-3.spec    | 33 ++++++++++++++++
 6 files changed, 113 insertions(+), 4 deletions(-)
 create mode 100644 src/test/isolation/expected/index-only-scan-2.out
 create mode 100644 src/test/isolation/expected/index-only-scan-3.out
 create mode 100644 src/test/isolation/specs/index-only-scan-2.spec
 create mode 100644 src/test/isolation/specs/index-only-scan-3.spec

diff --git a/src/backend/executor/nodeIndexonlyscan.c b/src/backend/executor/nodeIndexonlyscan.c
index 0754e28a9a..f7185b4519 100644
--- a/src/backend/executor/nodeIndexonlyscan.c
+++ b/src/backend/executor/nodeIndexonlyscan.c
@@ -243,12 +243,16 @@ IndexOnlyNext(IndexOnlyScanState *node)
 
 		/*
 		 * If we didn't access the heap, then we'll need to take a predicate
-		 * lock explicitly, as if we had.  For now we do that at page level.
+		 * lock explicitly, as if we had.  We don't have the inserter's xid,
+		 * but that's only used by PredicateLockTID to check if it matches the
+		 * caller's top level xid, and it can't possibly have been inserted by
+		 * us or the page wouldn't be all visible.
 		 */
 		if (!tuple_from_heap)
-			PredicateLockPage(scandesc->heapRelation,
-							  ItemPointerGetBlockNumber(tid),
-							  estate->es_snapshot);
+			PredicateLockTID(scandesc->heapRelation,
+							 tid,
+							 estate->es_snapshot,
+							 InvalidTransactionId);
 
 		return slot;
 	}
diff --git a/src/test/isolation/expected/index-only-scan-2.out b/src/test/isolation/expected/index-only-scan-2.out
new file mode 100644
index 0000000000..fef5b8d398
--- /dev/null
+++ b/src/test/isolation/expected/index-only-scan-2.out
@@ -0,0 +1,15 @@
+Parsed test spec with 2 sessions
+
+starting permutation: r1 r2 w1 w2 c1 c2
+step r1: SELECT id1 FROM account WHERE id1 = 1;
+id1            
+
+1              
+step r2: SELECT id2 FROM account WHERE id2 = 2;
+id2            
+
+2              
+step w1: UPDATE account SET balance = 200 WHERE id1 = 1;
+step w2: UPDATE account SET balance = 200 WHERE id2 = 2;
+step c1: COMMIT;
+step c2: COMMIT;
diff --git a/src/test/isolation/expected/index-only-scan-3.out b/src/test/isolation/expected/index-only-scan-3.out
new file mode 100644
index 0000000000..efef162779
--- /dev/null
+++ b/src/test/isolation/expected/index-only-scan-3.out
@@ -0,0 +1,16 @@
+Parsed test spec with 2 sessions
+
+starting permutation: r1 r2 w1 w2 c1 c2
+step r1: SELECT id1 FROM account WHERE id1 = 2;
+id1            
+
+2              
+step r2: SELECT id2 FROM account WHERE id2 = 1;
+id2            
+
+1              
+step w1: UPDATE account SET balance = 200 WHERE id1 = 1;
+step w2: UPDATE account SET balance = 200 WHERE id2 = 2;
+step c1: COMMIT;
+step c2: COMMIT;
+ERROR:  could not serialize access due to read/write dependencies among transactions
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index f4c01006fc..570aedb900 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -17,6 +17,8 @@ test: partial-index
 test: two-ids
 test: multiple-row-versions
 test: index-only-scan
+test: index-only-scan-2
+test: index-only-scan-3
 test: predicate-lock-hot-tuple
 test: update-conflict-out
 test: deadlock-simple
diff --git a/src/test/isolation/specs/index-only-scan-2.spec b/src/test/isolation/specs/index-only-scan-2.spec
new file mode 100644
index 0000000000..cd3c599af8
--- /dev/null
+++ b/src/test/isolation/specs/index-only-scan-2.spec
@@ -0,0 +1,39 @@
+# Test the granularity of predicate locks on heap tuples, for index-only scans.
+
+# Access the accounts through two different indexes, so that index predicate
+# locks don't confuse matters; here we only want to test heap locking.  Since
+# s1 and s2 access different heap tuples there is no cycle, so both
+# transactions should be able to commit.  Before PostgreSQL 15, a serialization
+# failure was reported because of a page-level SIREAD lock on the heap that
+# produced a false conflict.
+
+setup
+{
+  CREATE TABLE account (id1 int, id2 int, balance int);
+  CREATE UNIQUE INDEX ON account(id1);
+  CREATE UNIQUE INDEX ON account(id2);
+  INSERT INTO account VALUES (1, 1, 100), (2, 2, 100);
+}
+setup
+{
+  VACUUM account;
+}
+
+teardown
+{
+  DROP TABLE account;
+}
+
+session "s1"
+setup { BEGIN ISOLATION LEVEL SERIALIZABLE; SET enable_seqscan = off; }
+step "r1" { SELECT id1 FROM account WHERE id1 = 1; }
+step "w1" { UPDATE account SET balance = 200 WHERE id1 = 1; }
+step "c1" { COMMIT; }
+
+session "s2"
+setup { BEGIN ISOLATION LEVEL SERIALIZABLE; SET enable_seqscan = off; }
+step "r2" { SELECT id2 FROM account WHERE id2 = 2; }
+step "w2" { UPDATE account SET balance = 200 WHERE id2 = 2; }
+step "c2" { COMMIT; }
+
+permutation "r1" "r2" "w1" "w2" "c1" "c2"
diff --git a/src/test/isolation/specs/index-only-scan-3.spec b/src/test/isolation/specs/index-only-scan-3.spec
new file mode 100644
index 0000000000..5f04923002
--- /dev/null
+++ b/src/test/isolation/specs/index-only-scan-3.spec
@@ -0,0 +1,33 @@
+# A variant of index-only-scan-2.spec that is a true conflict, detected via
+# heap tuple locks only.
+
+setup
+{
+  CREATE TABLE account (id1 int, id2 int, balance int);
+  CREATE UNIQUE INDEX ON account(id1);
+  CREATE UNIQUE INDEX ON account(id2);
+  INSERT INTO account VALUES (1, 1, 100), (2, 2, 100);
+}
+setup
+{
+  VACUUM account;
+}
+
+teardown
+{
+  DROP TABLE account;
+}
+
+session "s1"
+setup { BEGIN ISOLATION LEVEL SERIALIZABLE; SET enable_seqscan = off; }
+step "r1" { SELECT id1 FROM account WHERE id1 = 2; }
+step "w1" { UPDATE account SET balance = 200 WHERE id1 = 1; }
+step "c1" { COMMIT; }
+
+session "s2"
+setup { BEGIN ISOLATION LEVEL SERIALIZABLE; SET enable_seqscan = off; }
+step "r2" { SELECT id2 FROM account WHERE id2 = 1; }
+step "w2" { UPDATE account SET balance = 200 WHERE id2 = 2; }
+step "c2" { COMMIT; }
+
+permutation "r1" "r2" "w1" "w2" "c1" "c2"
-- 
2.30.2

