From 6429f7d583be9999bfb524a4a9d6a27edbdf110c Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Sat, 15 Aug 2026 13:24:11 -0400
Subject: [PATCH v2 2/4] Add an isolation test for the GIN pending list cleanup
 gap

ginbulkdelete() cleans the GIN pending list from inside its "if (stats == NULL)"
block, so it only does so on VACUUM's first index pass.  The set of TIDs to
delete is rebuilt for every pass, though, so a later pass can be handed a TID
whose only index entry is still sitting in the pending list.  That pass sweeps
the entry tree and the posting trees, leaves the pending entry alone, and the
heap pass then frees the line pointer underneath it.  A later row recycles the
line pointer, and an index scan returns rows a sequential scan does not.

A tuple only has to become dead after the first pass to reach this.  An aborted
UPDATE does that at once: HeapTupleSatisfiesVacuum() reports HEAPTUPLE_DEAD for
an aborted xmin whatever OldestXmin is, and the pending list entry aminsert
already wrote is not rolled back with it.

The test makes VACUUM need more than one index pass by leaving dead line
pointers over more pages than a 64kB maintenance_work_mem can track, stops it at
a new injection point once the first pass has cleaned the pending list, and
rolls back an UPDATE of rows on pages the heap scan has not reached yet.  A
later pass collects those aborted row versions.  Fresh rows then recycle the
line pointers.

Step scan_incomplete asserts that the heap scan really did stop short.  Without
it the test would go on passing if a change to the dead item store, or to the
page count, ever left VACUUM with a single index pass and so stopped exercising
anything.

Both counts in the expected output are 0, which is what a ginbulkdelete() that
cleans the pending list on every call produces; applied locally, that makes the
test pass with no change to the expected output.  As it stands the test fails,
reporting 50 for via_index against via_heap's 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/backend/access/gin/ginvacuum.c            |   2 +
 src/test/modules/injection_points/Makefile    |   1 +
 .../expected/gin_pending_cleanup.out          |  61 ++++++++++
 src/test/modules/injection_points/meson.build |   1 +
 .../specs/gin_pending_cleanup.spec            | 106 ++++++++++++++++++
 5 files changed, 171 insertions(+)
 create mode 100644 src/test/modules/injection_points/expected/gin_pending_cleanup.out
 create mode 100644 src/test/modules/injection_points/specs/gin_pending_cleanup.spec

diff --git a/src/backend/access/gin/ginvacuum.c b/src/backend/access/gin/ginvacuum.c
index 58a97e2d2..f5a143f37 100644
--- a/src/backend/access/gin/ginvacuum.c
+++ b/src/backend/access/gin/ginvacuum.c
@@ -648,6 +648,8 @@ ginbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 						 false, true, stats);
 	}
 
+	INJECTION_POINT("gin-bulkdelete-pending-cleaned", NULL);
+
 	/* we'll re-count the tuples each time */
 	stats->num_index_tuples = 0;
 	gvs.result = stats;
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 54e2857c4..6efdc566b 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -13,6 +13,7 @@ REGRESS = injection_points hashagg reindex_conc vacuum
 REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
 
 ISOLATION = basic \
+	    gin_pending_cleanup \
 	    gin_vacuum_relock \
 	    inplace \
 	    reindex_concurrently_deferred \
diff --git a/src/test/modules/injection_points/expected/gin_pending_cleanup.out b/src/test/modules/injection_points/expected/gin_pending_cleanup.out
new file mode 100644
index 000000000..009996784
--- /dev/null
+++ b/src/test/modules/injection_points/expected/gin_pending_cleanup.out
@@ -0,0 +1,61 @@
+Parsed test spec with 2 sessions
+
+starting permutation: vacuum_gin scan_incomplete ghost_begin ghost_update ghost_rollback release recycle count_via_index count_via_heap
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+step vacuum_gin: VACUUM (INDEX_CLEANUP ON) gin_pending; <waiting ...>
+step scan_incomplete: 
+	SELECT heap_blks_scanned < heap_blks_total AS scan_incomplete
+	FROM pg_stat_progress_vacuum WHERE relid = 'gin_pending'::regclass;
+
+scan_incomplete
+---------------
+t              
+(1 row)
+
+step ghost_begin: BEGIN;
+step ghost_update: UPDATE gin_pending SET tags = ARRAY['ghost'] WHERE id > 11900;
+step ghost_rollback: ROLLBACK;
+step release: 
+	SELECT injection_points_detach('gin-bulkdelete-pending-cleaned');
+	SELECT injection_points_wakeup('gin-bulkdelete-pending-cleaned');
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step vacuum_gin: <... completed>
+step recycle: INSERT INTO gin_pending
+					  SELECT 900000 + g, ARRAY['recycled']
+					  FROM generate_series(1, 12000) g;
+step count_via_index: 
+	SET enable_seqscan = off;
+	SET enable_indexscan = on;
+	SET enable_bitmapscan = on;
+	SELECT count(*) AS via_index FROM gin_pending WHERE tags @> ARRAY['ghost'];
+
+via_index
+---------
+        0
+(1 row)
+
+step count_via_heap: 
+	SET enable_seqscan = on;
+	SET enable_indexscan = off;
+	SET enable_bitmapscan = off;
+	SELECT count(*) AS via_heap FROM gin_pending WHERE tags @> ARRAY['ghost'];
+
+via_heap
+--------
+       0
+(1 row)
+
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index 69a12d3cc..bac87e8b6 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -44,6 +44,7 @@ tests += {
   'isolation': {
     'specs': [
       'basic',
+      'gin_pending_cleanup',
       'gin_vacuum_relock',
       'inplace',
       'reindex_concurrently_deferred',
diff --git a/src/test/modules/injection_points/specs/gin_pending_cleanup.spec b/src/test/modules/injection_points/specs/gin_pending_cleanup.spec
new file mode 100644
index 000000000..1b326095a
--- /dev/null
+++ b/src/test/modules/injection_points/specs/gin_pending_cleanup.spec
@@ -0,0 +1,106 @@
+# ginbulkdelete() drained the GIN pending list only on VACUUM's first index
+# pass, because the call sat inside its "if (stats == NULL)" block.  The set of
+# TIDs to delete is rebuilt for every pass, though, so a later pass could be
+# handed a TID whose only index entry was still sitting in the pending list.
+# That pass swept the entry tree and the posting trees, left the pending entry
+# alone, and the heap pass then freed the line pointer underneath it.
+#
+# A tuple only has to become dead after the first pass to reach this.  An
+# aborted UPDATE does that at once: HeapTupleSatisfiesVacuum() reports
+# HEAPTUPLE_DEAD for an aborted xmin whatever OldestXmin is, and the pending
+# list entry aminsert already wrote is not rolled back with it.
+#
+# Both counts below must be 0.  Without the fix the index keeps an entry for a
+# line pointer that VACUUM freed, a later row recycles that line pointer, and
+# via_index reports the recycled rows while via_heap reports none.  Nothing
+# repairs the index afterwards: VACUUM only removes TIDs it collected, and this
+# one is LP_UNUSED by then.
+#
+# The setup deliberately makes VACUUM need several index passes: the dead line
+# pointers cover far more pages than maintenance_work_mem can track.  Step
+# scan_incomplete asserts that this still holds, so that the test cannot go on
+# passing once it has stopped reaching a second pass.
+
+setup
+{
+	CREATE EXTENSION injection_points;
+
+	CREATE TABLE gin_pending (id int, tags text[])
+		WITH (autovacuum_enabled = off);
+	INSERT INTO gin_pending
+		SELECT g, ARRAY['v' || g] FROM generate_series(1, 12000) g;
+	CREATE INDEX gin_pending_idx ON gin_pending USING gin (tags)
+		WITH (fastupdate = on, gin_pending_list_limit = 65536);
+
+	-- Free space on the last pages, so that the aborted UPDATE below keeps its
+	-- new row version on the page the row is already on.
+	DELETE FROM gin_pending WHERE id > 11400 AND id % 2 = 0;
+}
+
+# VACUUM cannot run alongside other statements in one setup block.  INDEX_CLEANUP
+# ON so that the bypass cannot skip freeing these line pointers, which is what
+# leaves the free space the aborted UPDATE below relies on.
+setup	{ VACUUM (INDEX_CLEANUP ON) gin_pending; }
+
+setup
+{
+	-- The dead line pointers the VACUUM below is supposed to remove.  They
+	-- span more pages than maintenance_work_mem can hold at once.
+	DELETE FROM gin_pending WHERE id <= 11400 AND id % 4 <> 0;
+}
+
+teardown
+{
+	DROP TABLE gin_pending;
+	DROP EXTENSION injection_points;
+}
+
+session s_vacuum
+setup
+{
+	SET maintenance_work_mem = '64kB';
+	SELECT injection_points_set_local();
+	SELECT injection_points_attach('gin-bulkdelete-pending-cleaned', 'wait');
+}
+# Stops once the first index pass has drained the pending list.
+step vacuum_gin		{ VACUUM (INDEX_CLEANUP ON) gin_pending; }
+# Recycles the line pointers the VACUUM freed.
+step recycle		{ INSERT INTO gin_pending
+					  SELECT 900000 + g, ARRAY['recycled']
+					  FROM generate_series(1, 12000) g; }
+step count_via_index
+{
+	SET enable_seqscan = off;
+	SET enable_indexscan = on;
+	SET enable_bitmapscan = on;
+	SELECT count(*) AS via_index FROM gin_pending WHERE tags @> ARRAY['ghost'];
+}
+step count_via_heap
+{
+	SET enable_seqscan = on;
+	SET enable_indexscan = off;
+	SET enable_bitmapscan = off;
+	SELECT count(*) AS via_heap FROM gin_pending WHERE tags @> ARRAY['ghost'];
+}
+
+session s_writer
+# The heap scan must still have pages left, or there is no second index pass
+# and no page left holding the row the ghost below lands on.
+step scan_incomplete
+{
+	SELECT heap_blks_scanned < heap_blks_total AS scan_incomplete
+	FROM pg_stat_progress_vacuum WHERE relid = 'gin_pending'::regclass;
+}
+step ghost_begin	{ BEGIN; }
+# Leaves a dead row version, and a pending list entry for it, on a page the
+# heap scan has not reached yet.
+step ghost_update	{ UPDATE gin_pending SET tags = ARRAY['ghost'] WHERE id > 11900; }
+step ghost_rollback	{ ROLLBACK; }
+# Detach before waking, so the remaining index passes do not stop here again.
+step release
+{
+	SELECT injection_points_detach('gin-bulkdelete-pending-cleaned');
+	SELECT injection_points_wakeup('gin-bulkdelete-pending-cleaned');
+}
+
+permutation vacuum_gin scan_incomplete ghost_begin ghost_update ghost_rollback release recycle count_via_index count_via_heap
-- 
2.53.0

