Date: Fri, 26 Jun 2026 14:45:57 +0800
From: Adam Lee <adam8157@gmail.com>
To: pgsql-hackers@lists.postgresql.org
Cc: Antonin Houska <ah@cybertec.at>,
	=?iso-8859-1?Q?=C1lvaro?= Herrera <alvherre@kurilemu.de>
Subject: CLUSTER progress: wrong index_rebuild_count for tables with TOAST
Message-ID: <aj4gJQMba0kClQmj@Mac>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="MhLLfZjzjFMK15NK"
Content-Disposition: inline
Archived-At: <https://www.postgresql.org/message-id/aj4gJQMba0kClQmj%40Mac>
Precedence: bulk

--MhLLfZjzjFMK15NK
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi,

When I run CLUSTER (or VACUUM FULL / REPACK) on a table that has a TOAST
table, pg_stat_progress_cluster shows a wrong index_rebuild_count. During
the heap scan the count is already equal to the number of indexes, but it
should be 0 until the indexes are rebuilt at the end.

A simple way to reproduce it. Run CLUSTER in one session, and query the
view from another session at the same time:

```
CREATE TABLE t (i int, x text);
INSERT INTO t SELECT g, repeat(md5(g::text), 1000)
  FROM generate_series(1, 5) g;
CREATE INDEX ON t (i);
CLUSTER t USING t_i_idx;

-- phase "seq scanning heap", index_rebuild_count = 2 (should be 0)
```

The reason is that make_new_heap() creates the new TOAST table, and its
TOAST index, before the heap is scanned. Building that index reports
CREATE INDEX progress. CREATE INDEX and the cluster command use the same
progress field for different things, so the cluster command then shows a
CREATE INDEX value as its index_rebuild_count.

The fix is to not report progress for the TOAST index build, because it
is an internal index, not a user CREATE INDEX. The patch also adds an
isolation test that pauses CLUSTER at the start of the heap scan and
checks that index_rebuild_count is 0.

-- 
Adam

--MhLLfZjzjFMK15NK
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v1-0001-Suppress-CREATE-INDEX-progress-while-building-a-T.patch

From 5d701f806b702d1003a4ca6ec56a98cb59b4920c Mon Sep 17 00:00:00 2001
From: Adam Lee <adam8157@gmail.com>
Date: Fri, 26 Jun 2026 14:31:25 +0800
Subject: [PATCH v1] Suppress CREATE INDEX progress while building a TOAST
 index

create_toast_table() built the TOAST table's index without
INDEX_CREATE_SUPPRESS_PROGRESS, so index_build() reported CREATE INDEX
progress for it.  By itself that is merely pointless -- a TOAST index is
never the target of a user-issued CREATE INDEX -- but it is actively
wrong when the TOAST table is created while another command is already
reporting progress for the same backend.

make_new_heap(), used by CLUSTER and VACUUM FULL, is exactly such a
case: it creates the new relation's TOAST table, and thus the TOAST
index, before the old heap is scanned.  CREATE INDEX and REPACK share
the st_progress_param[] slots -- PROGRESS_CREATEIDX_PHASE and
PROGRESS_REPACK_INDEX_REBUILD_COUNT are both slot 9 -- so building the
TOAST index overwrote the in-progress cluster's index_rebuild_count.
pg_stat_progress_cluster therefore showed index_rebuild_count = 2 (the
numeric value of PROGRESS_CREATEIDX_PHASE_BUILD) while the command was
still scanning the heap, where it must read 0 until the indexes are
rebuilt at the end.

Pass INDEX_CREATE_SUPPRESS_PROGRESS for the TOAST index to keep the
build from touching the surrounding command's progress counters.

Also add an isolation test that suspends CLUSTER at the start of the
heap scan, using a new injection point, and confirms index_rebuild_count
is still 0 at that point.
---
 src/backend/access/heap/heapam_handler.c      |  7 +++
 src/backend/catalog/toasting.c                | 14 +++++-
 src/test/modules/injection_points/Makefile    |  1 +
 .../expected/cluster_progress.out             | 30 +++++++++++
 src/test/modules/injection_points/meson.build |  1 +
 .../specs/cluster_progress.spec               | 50 +++++++++++++++++++
 6 files changed, 102 insertions(+), 1 deletion(-)
 create mode 100644 src/test/modules/injection_points/expected/cluster_progress.out
 create mode 100644 src/test/modules/injection_points/specs/cluster_progress.spec

diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 2268cc277bc..10c88953acb 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -45,6 +45,7 @@
 #include "storage/procarray.h"
 #include "storage/smgr.h"
 #include "utils/builtins.h"
+#include "utils/injection_point.h"
 #include "utils/rel.h"
 #include "utils/tuplesort.h"
 
@@ -706,6 +707,12 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap,
 	slot = table_slot_create(OldHeap, NULL);
 	hslot = (BufferHeapTupleTableSlot *) slot;
 
+	/*
+	 * Allow a test to observe progress reporting at this point, with the scan
+	 * phase set but no tuples scanned yet.
+	 */
+	INJECTION_POINT("heap-cluster-scan-start", NULL);
+
 	/*
 	 * Scan through the OldHeap, either in OldIndex order or sequentially;
 	 * copy each tuple into the NewHeap, or transiently to the tuplesort
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 4aa52a4bd25..d95a5ddf8c1 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -325,6 +325,17 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
 	coloptions[0] = 0;
 	coloptions[1] = 0;
 
+	/*
+	 * Build the TOAST index with progress reporting suppressed.  This is an
+	 * internal index, never a user-issued CREATE INDEX, so reporting CREATE
+	 * INDEX progress for it is meaningless.  More importantly, a TOAST table
+	 * (hence this index) can be created while another progress command is
+	 * active for the same backend -- e.g. make_new_heap() during a
+	 * CLUSTER/VACUUM FULL (REPACK).  CREATE INDEX and REPACK share progress
+	 * parameter slots (PROGRESS_CREATEIDX_PHASE vs
+	 * PROGRESS_REPACK_INDEX_REBUILD_COUNT), so an unsuppressed build would
+	 * clobber the enclosing command's progress view.
+	 */
 	index_create(toast_rel, toast_idxname, toastIndexOid, InvalidOid,
 				 InvalidOid, InvalidOid,
 				 indexInfo,
@@ -332,7 +343,8 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
 				 BTREE_AM_OID,
 				 rel->rd_rel->reltablespace,
 				 collationIds, opclassIds, NULL, coloptions, NULL, (Datum) 0,
-				 INDEX_CREATE_IS_PRIMARY, 0, true, true, NULL);
+				 INDEX_CREATE_IS_PRIMARY | INDEX_CREATE_SUPPRESS_PROGRESS,
+				 0, true, true, NULL);
 
 	table_close(toast_rel, NoLock);
 
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index c01d2fb095c..69ee1230046 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 \
+	    cluster_progress \
 	    inplace \
 	    repack \
 	    repack_temporal \
diff --git a/src/test/modules/injection_points/expected/cluster_progress.out b/src/test/modules/injection_points/expected/cluster_progress.out
new file mode 100644
index 00000000000..4fcf82ed402
--- /dev/null
+++ b/src/test/modules/injection_points/expected/cluster_progress.out
@@ -0,0 +1,30 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1_cluster s2_progress s2_wakeup
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+step s1_cluster: CLUSTER cluster_progress USING cluster_progress_i; <waiting ...>
+step s2_progress: 
+	SELECT relid::regclass, phase, index_rebuild_count
+	FROM pg_stat_progress_cluster;
+
+relid           |phase            |index_rebuild_count
+----------------+-----------------+-------------------
+cluster_progress|seq scanning heap|                  0
+(1 row)
+
+step s2_wakeup: SELECT injection_points_wakeup('heap-cluster-scan-start');
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step s1_cluster: <... completed>
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index 59dba1cb023..3f4796102d1 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',
+      'cluster_progress',
       'inplace',
       'repack',
       'repack_temporal',
diff --git a/src/test/modules/injection_points/specs/cluster_progress.spec b/src/test/modules/injection_points/specs/cluster_progress.spec
new file mode 100644
index 00000000000..de9ead458ba
--- /dev/null
+++ b/src/test/modules/injection_points/specs/cluster_progress.spec
@@ -0,0 +1,50 @@
+# Verify pg_stat_progress_cluster.index_rebuild_count during the table-scan
+# phase of CLUSTER.
+#
+# A CLUSTER (REPACK) on a table that has a TOAST relation builds the new
+# table's TOAST index in make_new_heap(), before the heap is scanned.  That
+# internal index build must not report CREATE INDEX progress into the
+# enclosing REPACK command: the two commands share progress slot 9
+# (PROGRESS_CREATEIDX_PHASE vs PROGRESS_REPACK_INDEX_REBUILD_COUNT), so an
+# unsuppressed build leaves index_rebuild_count looking like a CREATE INDEX
+# phase value while the cluster is still scanning the heap.  It must read 0
+# until indexes are actually rebuilt at the end.
+
+setup
+{
+	CREATE EXTENSION injection_points;
+
+	-- A table with a TOAST relation (wide, toastable column) and an index to
+	-- cluster on.
+	CREATE TABLE cluster_progress (i int, t text);
+	INSERT INTO cluster_progress
+		SELECT g, repeat(md5(g::text), 1000) FROM generate_series(1, 5) g;
+	CREATE INDEX cluster_progress_i ON cluster_progress (i);
+}
+
+teardown
+{
+	DROP TABLE cluster_progress;
+	DROP EXTENSION injection_points;
+}
+
+session s1
+setup
+{
+	SELECT injection_points_set_local();
+	SELECT injection_points_attach('heap-cluster-scan-start', 'wait');
+}
+step s1_cluster	{ CLUSTER cluster_progress USING cluster_progress_i; }
+teardown		{ SELECT injection_points_detach('heap-cluster-scan-start'); }
+
+session s2
+# CLUSTER is suspended at the start of the heap scan; index_rebuild_count must
+# be 0 here (indexes are rebuilt only at the end).
+step s2_progress
+{
+	SELECT relid::regclass, phase, index_rebuild_count
+	FROM pg_stat_progress_cluster;
+}
+step s2_wakeup	{ SELECT injection_points_wakeup('heap-cluster-scan-start'); }
+
+permutation s1_cluster s2_progress s2_wakeup
-- 
2.52.0


--MhLLfZjzjFMK15NK--