v3-0001-isolationtester-showing-broken-index-only-scans-w.patch

From afa0310803bf72bdaccff265afbdecf26da88435 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Mon, 2 Dec 2024 15:50:04 -0500
Subject: [PATCH v3 1/2] isolationtester showing broken index-only scans with
 GiST v3

Co-authored-by: Matthias van de Meent <boekewurm+postgres@gmail.com>
---
 contrib/btree_gist/.gitignore              |   2 +
 contrib/btree_gist/Makefile                |   3 +
 contrib/btree_gist/expected/btree_gist.out |  69 ++++++++++++
 contrib/btree_gist/meson.build             |   6 ++
 contrib/btree_gist/specs/btree_gist.spec   | 117 +++++++++++++++++++++
 5 files changed, 197 insertions(+)
 create mode 100644 contrib/btree_gist/expected/btree_gist.out
 create mode 100644 contrib/btree_gist/specs/btree_gist.spec

diff --git a/contrib/btree_gist/.gitignore b/contrib/btree_gist/.gitignore
index 5dcb3ff9723..b4903eba657 100644
--- a/contrib/btree_gist/.gitignore
+++ b/contrib/btree_gist/.gitignore
@@ -1,4 +1,6 @@
 # Generated subdirectories
 /log/
 /results/
+/output_iso/
 /tmp_check/
+/tmp_check_iso/
diff --git a/contrib/btree_gist/Makefile b/contrib/btree_gist/Makefile
index 7ac2df26c10..e8cdef2277d 100644
--- a/contrib/btree_gist/Makefile
+++ b/contrib/btree_gist/Makefile
@@ -42,6 +42,9 @@ REGRESS = init int2 int4 int8 float4 float8 cash oid timestamp timestamptz \
         bytea bit varbit numeric uuid not_equal enum bool partitions \
         stratnum without_overlaps
 
+ISOLATION = btree_gist
+ISOLATION_OPTS = --load-extension=btree_gist
+
 SHLIB_LINK += $(filter -lm, $(LIBS))
 
 ifdef USE_PGXS
diff --git a/contrib/btree_gist/expected/btree_gist.out b/contrib/btree_gist/expected/btree_gist.out
new file mode 100644
index 00000000000..88dad12a415
--- /dev/null
+++ b/contrib/btree_gist/expected/btree_gist.out
@@ -0,0 +1,69 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s2_vacuum s2_mod s1_begin s1_prepare_sorted s1_fetch_1 s2_vacuum s1_fetch_all s1_commit
+step s2_vacuum: VACUUM (TRUNCATE false) ios_needs_cleanup_lock;
+step s2_mod: 
+  DELETE FROM ios_needs_cleanup_lock WHERE a > 1;
+
+step s1_begin: BEGIN;
+step s1_prepare_sorted: 
+    DECLARE foo NO SCROLL CURSOR FOR SELECT a FROM ios_needs_cleanup_lock WHERE a > 0 ORDER BY a <-> 0;
+
+step s1_fetch_1: 
+    FETCH FROM foo;
+
+a
+-
+1
+(1 row)
+
+step s2_vacuum: VACUUM (TRUNCATE false) ios_needs_cleanup_lock; <waiting ...>
+step s1_fetch_all: 
+    SELECT pg_sleep_for(INTERVAL '50ms');
+    FETCH ALL FROM foo;
+
+pg_sleep_for
+------------
+            
+(1 row)
+
+a
+-
+(0 rows)
+
+step s2_vacuum: <... completed>
+step s1_commit: COMMIT;
+
+starting permutation: s2_vacuum s2_mod s1_begin s1_prepare_unsorted s1_fetch_1 s2_vacuum s1_fetch_all s1_commit
+step s2_vacuum: VACUUM (TRUNCATE false) ios_needs_cleanup_lock;
+step s2_mod: 
+  DELETE FROM ios_needs_cleanup_lock WHERE a > 1;
+
+step s1_begin: BEGIN;
+step s1_prepare_unsorted: 
+    DECLARE foo NO SCROLL CURSOR FOR SELECT a FROM ios_needs_cleanup_lock WHERE a > 0;
+
+step s1_fetch_1: 
+    FETCH FROM foo;
+
+a
+-
+1
+(1 row)
+
+step s2_vacuum: VACUUM (TRUNCATE false) ios_needs_cleanup_lock; <waiting ...>
+step s1_fetch_all: 
+    SELECT pg_sleep_for(INTERVAL '50ms');
+    FETCH ALL FROM foo;
+
+pg_sleep_for
+------------
+            
+(1 row)
+
+a
+-
+(0 rows)
+
+step s2_vacuum: <... completed>
+step s1_commit: COMMIT;
diff --git a/contrib/btree_gist/meson.build b/contrib/btree_gist/meson.build
index 73b1bbf52a6..51adf635eb9 100644
--- a/contrib/btree_gist/meson.build
+++ b/contrib/btree_gist/meson.build
@@ -94,4 +94,10 @@ tests += {
       'without_overlaps',
     ],
   },
+  'isolation': {
+    'specs': [
+      'btree_gist',
+    ],
+    'regress_args': ['--load-extension=btree_gist'],
+  },
 }
diff --git a/contrib/btree_gist/specs/btree_gist.spec b/contrib/btree_gist/specs/btree_gist.spec
new file mode 100644
index 00000000000..18ad7b4dbd5
--- /dev/null
+++ b/contrib/btree_gist/specs/btree_gist.spec
@@ -0,0 +1,117 @@
+# index-only-scan test showing wrong results with GiST
+#
+setup
+{
+    -- by using a low fillfactor and a wide tuple we can get multiple blocks
+    -- with just few rows
+    CREATE TABLE ios_needs_cleanup_lock (a int NOT NULL, b int not null, pad char(1024) default '')
+    WITH (AUTOVACUUM_ENABLED = false, FILLFACTOR = 10);
+
+    INSERT INTO ios_needs_cleanup_lock SELECT g.i, g.i FROM generate_series(1, 10) g(i);
+
+    CREATE INDEX ios_gist_a ON ios_needs_cleanup_lock USING gist(a);
+}
+
+teardown
+{
+    DROP TABLE ios_needs_cleanup_lock;
+}
+
+
+session s1
+
+# Force an index-only scan, where possible:
+setup {
+    SET enable_bitmapscan = false;
+    SET enable_indexonlyscan = true;
+    SET enable_indexscan = true;
+}
+
+step s1_begin { BEGIN; }
+step s1_commit { COMMIT; }
+
+step s1_prepare_sorted {
+    DECLARE foo NO SCROLL CURSOR FOR SELECT a FROM ios_needs_cleanup_lock WHERE a > 0 ORDER BY a <-> 0;
+}
+
+step s1_prepare_unsorted {
+    DECLARE foo NO SCROLL CURSOR FOR SELECT a FROM ios_needs_cleanup_lock WHERE a > 0;
+}
+
+step s1_fetch_1 {
+    FETCH FROM foo;
+}
+
+step s1_fetch_all {
+    SELECT pg_sleep_for(INTERVAL '50ms');
+    FETCH ALL FROM foo;
+}
+
+
+session s2
+
+# Don't delete row 1 so we have a row for the cursor to "rest" on.
+step s2_mod
+{
+  DELETE FROM ios_needs_cleanup_lock WHERE a > 1;
+}
+
+# Disable truncation, as otherwise we'll just wait for a timeout while trying
+# to acquire the lock
+step s2_vacuum  { VACUUM (TRUNCATE false) ios_needs_cleanup_lock; }
+
+permutation
+  # Vacuum first, to ensure VM exists, otherwise the bitmapscan will consider
+  # VM to be size 0, due to caching. Can't do that in setup because
+  s2_vacuum
+
+  # delete nearly all rows, to make issue visible
+  s2_mod
+  # create a cursor
+  s1_begin
+  s1_prepare_sorted
+
+  # fetch one row from the cursor, that ensures the index scan portion is done
+  # before the vacuum in the next step
+  s1_fetch_1
+
+  # with the bug this vacuum will mark pages as all-visible that the scan in
+  # the next step then considers all-visible, despite all rows from those
+  # pages having been removed.
+  # Because this should block on buffer-level locks, this won't ever be
+  # considered "blocked" by isolation tester, and so we only have a single
+  # step we can work with concurrently.
+  s2_vacuum (*)
+
+  # if this returns any rows, we're busted
+  s1_fetch_all
+
+  s1_commit
+
+permutation
+  # Vacuum first, to ensure VM exists, otherwise the bitmapscan will consider
+  # VM to be size 0, due to caching. Can't do that in setup because
+  s2_vacuum
+
+  # delete nearly all rows, to make issue visible
+  s2_mod
+  # create a cursor
+  s1_begin
+  s1_prepare_unsorted
+
+  # fetch one row from the cursor, that ensures the index scan portion is done
+  # before the vacuum in the next step
+  s1_fetch_1
+
+  # with the bug this vacuum will mark pages as all-visible that the scan in
+  # the next step then considers all-visible, despite all rows from those
+  # pages having been removed.
+  # Because this should block on buffer-level locks, this won't ever be
+  # considered "blocked" by isolation tester, and so we only have a single
+  # step we can work with concurrently.
+  s2_vacuum (*)
+
+  # if this returns any rows, we're busted
+  s1_fetch_all
+
+  s1_commit
-- 
2.45.2