Feedback-ID: ie3de48e3:Fastmail
Date: Thu, 10 Sep 2026 11:24:55 +0200
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: Zsolt Parragi <zsolt.parragi@percona.com>
Cc: Christophe Pettus <xof@thebuild.com>,
	pgsql-hackers@lists.postgresql.org,
 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Subject: Re: REPACK (CONCURRENTLY) doesn't handle invalid indexes
Message-ID: <aqJ0Qt-3teW-BhSN@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="ih3nxo6vxmekktdf"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ap-3KAqj28ED0TeA@alvherre.pgsql>
Archived-At: 
 <https://www.postgresql.org/message-id/aqJ0Qt-3teW-BhSN%40alvherre.pgsql>
Precedence: bulk


--ih3nxo6vxmekktdf
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2026-Sep-08, Álvaro Herrera wrote:

> Yes, but I think the question is in which direction should we fix said
> bug.  My preference is to go for Kyotaro's suggestion: have both REPACK
> and REPACK (CONCURRENTLY) raise an error with an invalid index, asking
> the user to drop it.
> 
> Would anybody oppose that?

Concretely, something like this.  (Hmm, I guess this should be noted in
repack.sgml as well.)

I don't want to touch the behavior of REINDEX, CLUSTER or VACUUM FULL in
pg19 at this stage, much less within the context of an "open item"; we
can discuss that for pg20 afterwards.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Uno puede defenderse de los ataques; contra los elogios se esta indefenso"

--ih3nxo6vxmekktdf
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename=0001-Fail-REPACK-in-presence-of-isready-indvalid-indexes.patch

From a0d1612ceed68b29ae1c1df01cf3702022286a2b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Thu, 10 Sep 2026 10:49:48 +0200
Subject: [PATCH] Fail REPACK in presence of !isready !indvalid indexes

Like VACUUM FULL, non-concurrent REPACK would try to rebuild such
indexes, which can sometimes succeed.  Concurrent REPACK would however
fail.  The inconsistency is not good, so make them both throw an error
quickly to force the user to make a decision on those indexes (most
likely, drop them).

Reported-by: Zsolt Parragi <zsolt.parragi@percona.com>
Suggested-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/CAN4CZFO5A3YE0Dd-bn7eKrB20pECO3=U0wKg1z2rO=DxgWJJHQ@mail.gmail.com
---
 contrib/test_decoding/expected/repack.out | 16 ++++++
 contrib/test_decoding/sql/repack.sql      |  8 +++
 src/backend/commands/repack.c             | 69 +++++++++++++++++++++++
 3 files changed, 93 insertions(+)

diff --git a/contrib/test_decoding/expected/repack.out b/contrib/test_decoding/expected/repack.out
index ac5473137d1..3d6392def8a 100644
--- a/contrib/test_decoding/expected/repack.out
+++ b/contrib/test_decoding/expected/repack.out
@@ -51,6 +51,22 @@ SELECT * FROM rpk_missing;
 (3 rows)
 
 DROP TABLE rpk_missing;
+-- Verify handling of !valid !isready indexes
+CREATE TABLE repack_conc_invidx (i int PRIMARY KEY, j int);
+INSERT INTO repack_conc_invidx VALUES (1, 0), (2, 0);
+CREATE UNIQUE INDEX CONCURRENTLY repack_conc_invidx_uq ON repack_conc_invidx (j);
+ERROR:  could not create unique index "repack_conc_invidx_uq"
+DETAIL:  Key (j)=(0) is duplicated.
+CREATE INDEX CONCURRENTLY repack_conc_invalid_expr ON repack_conc_invidx ((1/j));
+ERROR:  division by zero
+REPACK repack_conc_invidx;
+ERROR:  cannot execute REPACK on relation "repack_conc_invidx"
+DETAIL:  Some invalid indexes cannot be processed correctly: "repack_conc_invidx_uq", "repack_conc_invalid_expr".
+HINT:  Use DROP INDEX or REINDEX.
+REPACK (CONCURRENTLY) repack_conc_invidx;
+ERROR:  cannot execute REPACK on relation "repack_conc_invidx"
+DETAIL:  Some invalid indexes cannot be processed correctly: "repack_conc_invidx_uq", "repack_conc_invalid_expr".
+HINT:  Use DROP INDEX or REINDEX.
 -- Error cases for concurrent mode
 -- Doesn't like partitioned tables
 CREATE TABLE clstrpart (a int) PARTITION BY RANGE (a);
diff --git a/contrib/test_decoding/sql/repack.sql b/contrib/test_decoding/sql/repack.sql
index e995c72d28d..9960a3de16c 100644
--- a/contrib/test_decoding/sql/repack.sql
+++ b/contrib/test_decoding/sql/repack.sql
@@ -33,6 +33,14 @@ REPACK (CONCURRENTLY) rpk_missing;
 SELECT * FROM rpk_missing;
 DROP TABLE rpk_missing;
 
+-- Verify handling of !valid !isready indexes
+CREATE TABLE repack_conc_invidx (i int PRIMARY KEY, j int);
+INSERT INTO repack_conc_invidx VALUES (1, 0), (2, 0);
+CREATE UNIQUE INDEX CONCURRENTLY repack_conc_invidx_uq ON repack_conc_invidx (j);
+CREATE INDEX CONCURRENTLY repack_conc_invalid_expr ON repack_conc_invidx ((1/j));
+REPACK repack_conc_invidx;
+REPACK (CONCURRENTLY) repack_conc_invidx;
+
 -- Error cases for concurrent mode
 
 -- Doesn't like partitioned tables
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 2924d884b10..c3075bae2a6 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -159,6 +159,7 @@ static bool cluster_rel_recheck(RepackCommand cmd, Relation OldHeap,
 								int options);
 static void check_concurrent_repack_requirements(Relation rel,
 												 Oid *ident_idx_p);
+static void check_repack_index_requirements(Relation rel);
 static void rebuild_relation(Relation OldHeap, Relation index, bool verbose,
 							 Oid ident_idx);
 static void copy_table_data(Relation NewHeap, Relation OldHeap, Relation OldIndex,
@@ -527,6 +528,14 @@ cluster_rel(RepackCommand cmd, Relation OldHeap, Oid indexOid,
 	if (concurrent)
 		check_concurrent_repack_requirements(OldHeap, &ident_idx);
 
+	/*
+	 * Also check the state of indexes; this can abort the command for REPACK.
+	 * Historically this hasn't affected CLUSTER or VACUUM FULL, so don't do
+	 * it for those commands.
+	 */
+	if (cmd == REPACK_COMMAND_REPACK)
+		check_repack_index_requirements(OldHeap);
+
 	/* Check for user-requested abort. */
 	CHECK_FOR_INTERRUPTS();
 
@@ -874,6 +883,66 @@ mark_index_clustered(Relation rel, Oid indexOid, bool is_internal)
 	table_close(pg_index, RowExclusiveLock);
 }
 
+/*
+ * Verify index state on the table being processed and throw an error if any
+ * indexes are found that are neither valid nor ready for inserts.
+ *
+ * Indexes that are neither valid nor ready for inserts, such as ones left
+ * behind by failed CREATE INDEX CONCURRENTLY, are not maintained by DML,
+ * and if they are constraint indexes, they may fail to build altogether.
+ * Throwing an error here forces the user to fix these indexes separately
+ * from REPACK.
+ */
+static void
+check_repack_index_requirements(Relation rel)
+{
+	Relation	indrel;
+	SysScanDesc indscan;
+	ScanKeyData skey;
+	HeapTuple	htup;
+	int			num_invalid_idxs = 0;
+	StringInfoData dest;
+
+	initStringInfo(&dest);
+
+	/* Prepare to scan pg_index for entries having indrelid = this rel. */
+	ScanKeyInit(&skey,
+				Anum_pg_index_indrelid,
+				BTEqualStrategyNumber, F_OIDEQ,
+				ObjectIdGetDatum(RelationGetRelid(rel)));
+
+	indrel = table_open(IndexRelationId, AccessShareLock);
+	indscan = systable_beginscan(indrel, IndexIndrelidIndexId, true,
+								 NULL, 1, &skey);
+
+	while (HeapTupleIsValid(htup = systable_getnext(indscan)))
+	{
+		Form_pg_index index = (Form_pg_index) GETSTRUCT(htup);
+
+		if (!index->indisvalid && !index->indisready)
+		{
+			if (num_invalid_idxs == 0)
+				appendStringInfo(&dest, _("\"%s\""), get_rel_name(index->indexrelid));
+			else
+				appendStringInfo(&dest, _(", \"%s\""), get_rel_name(index->indexrelid));
+			num_invalid_idxs++;
+		}
+	}
+	systable_endscan(indscan);
+	table_close(indrel, AccessShareLock);
+
+	if (num_invalid_idxs > 0)
+		ereport(ERROR,
+				errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				errmsg("cannot execute %s on relation \"%s\"",
+					   "REPACK", RelationGetRelationName(rel)),
+				errdetail_plural("An invalid index cannot be processed correctly: %s.",
+								 "Some invalid indexes cannot be processed correctly: %s.",
+								 num_invalid_idxs,
+								 dest.data),
+				errhint("Use DROP INDEX or REINDEX."));
+}
+
 /*
  * Check if the CONCURRENTLY option is legal for the relation.
  *
-- 
2.47.3


--ih3nxo6vxmekktdf--