From 5ea70b746281344c877541d65e3d22505c9228c9 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Mon, 20 Jul 2026 21:24:52 +0000 Subject: [PATCH] Don't let invalid indexes break REPACK (CONCURRENTLY) build_new_indexes() copies every index of the table, and it built the copy of an invalid one (e.g. left over from a failed CREATE INDEX CONCURRENTLY) as a full index with its constraints enforced. When such an index is unique and the live data has duplicates, the copy failed to build and aborted the whole REPACK, with an error naming the internal '*_repacknew' index. The non-concurrent path does not have this problem: reindex_relation() rebuilds invalid indexes with constraint enforcement suppressed. Do the same here: build the copy of an invalid index as a plain non-unique index and don't copy its constraints. The old index stays invalid after the swap, as before. --- contrib/test_decoding/expected/repack.out | 35 +++++++++++++++++++++++ contrib/test_decoding/sql/repack.sql | 16 +++++++++++ src/backend/catalog/index.c | 10 +++++-- src/backend/commands/indexcmds.c | 3 +- src/backend/commands/repack.c | 14 +++++++-- src/include/catalog/index.h | 3 +- 6 files changed, 75 insertions(+), 6 deletions(-) diff --git a/contrib/test_decoding/expected/repack.out b/contrib/test_decoding/expected/repack.out index c4ff41be690..af1d10f4dfd 100644 --- a/contrib/test_decoding/expected/repack.out +++ b/contrib/test_decoding/expected/repack.out @@ -99,6 +99,41 @@ REPACK (CONCURRENTLY) repack_conc_replident; ERROR: cannot execute REPACK (CONCURRENTLY) on relation "repack_conc_replident" DETAIL: REPACK (CONCURRENTLY) does not support deferrable primary keys. HINT: Use ALTER TABLE ... REPLICA IDENTITY USING INDEX to designate another index as replica identity. +-- Invalid indexes (e.g. left over from a failed CREATE INDEX CONCURRENTLY) +-- must not prevent the processing; their constraints are not enforced while +-- rebuilding and they stay invalid. +CREATE TABLE repack_conc_invidx (i int PRIMARY KEY, j int); +INSERT INTO repack_conc_invidx VALUES (1, 1), (2, 1); +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)=(1) is duplicated. +SELECT indexrelid::regclass, indisvalid FROM pg_index +WHERE indrelid = 'repack_conc_invidx'::regclass ORDER BY indexrelid::regclass::text; + indexrelid | indisvalid +-------------------------+------------ + repack_conc_invidx_pkey | t + repack_conc_invidx_uq | f +(2 rows) + +REPACK (CONCURRENTLY) repack_conc_invidx; +SELECT indexrelid::regclass, indisvalid FROM pg_index +WHERE indrelid = 'repack_conc_invidx'::regclass ORDER BY indexrelid::regclass::text; + indexrelid | indisvalid +-------------------------+------------ + repack_conc_invidx_pkey | t + repack_conc_invidx_uq | f +(2 rows) + +SELECT * FROM repack_conc_invidx ORDER BY i; + i | j +---+--- + 1 | 1 + 2 | 1 +(2 rows) + +-- the invalid index still must not enforce uniqueness +INSERT INTO repack_conc_invidx VALUES (3, 1); +DROP TABLE repack_conc_invidx; -- clean up DROP TABLE repack_conc_replident, clstrpart; -- verify that the pgrepack plugin cannot be called directly diff --git a/contrib/test_decoding/sql/repack.sql b/contrib/test_decoding/sql/repack.sql index f461f5479f4..db51c600629 100644 --- a/contrib/test_decoding/sql/repack.sql +++ b/contrib/test_decoding/sql/repack.sql @@ -73,6 +73,22 @@ REPACK (CONCURRENTLY) repack_conc_replident; ALTER TABLE repack_conc_replident ADD PRIMARY KEY (i) DEFERRABLE; REPACK (CONCURRENTLY) repack_conc_replident; +-- Invalid indexes (e.g. left over from a failed CREATE INDEX CONCURRENTLY) +-- must not prevent the processing; their constraints are not enforced while +-- rebuilding and they stay invalid. +CREATE TABLE repack_conc_invidx (i int PRIMARY KEY, j int); +INSERT INTO repack_conc_invidx VALUES (1, 1), (2, 1); +CREATE UNIQUE INDEX CONCURRENTLY repack_conc_invidx_uq ON repack_conc_invidx (j); +SELECT indexrelid::regclass, indisvalid FROM pg_index +WHERE indrelid = 'repack_conc_invidx'::regclass ORDER BY indexrelid::regclass::text; +REPACK (CONCURRENTLY) repack_conc_invidx; +SELECT indexrelid::regclass, indisvalid FROM pg_index +WHERE indrelid = 'repack_conc_invidx'::regclass ORDER BY indexrelid::regclass::text; +SELECT * FROM repack_conc_invidx ORDER BY i; +-- the invalid index still must not enforce uniqueness +INSERT INTO repack_conc_invidx VALUES (3, 1); +DROP TABLE repack_conc_invidx; + -- clean up DROP TABLE repack_conc_replident, clstrpart; diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 81bba4beac7..a93f01010c5 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -1301,10 +1301,16 @@ index_create(Relation heapRelation, * index_create. * * "tablespaceOid" is the tablespace to use for this index. + * + * If "skip_constraint_checks" is true, the copy is created as a plain + * non-unique index, so that neither the build nor later insertions enforce + * uniqueness. Useful when the old index is invalid and its constraint + * therefore cannot be assumed to hold. */ Oid index_create_copy(Relation heapRelation, uint16 flags, - Oid oldIndexId, Oid tablespaceOid, const char *newName) + Oid oldIndexId, Oid tablespaceOid, const char *newName, + bool skip_constraint_checks) { Relation indexRelation; IndexInfo *oldInfo, @@ -1397,7 +1403,7 @@ index_create_copy(Relation heapRelation, uint16 flags, oldInfo->ii_Am, indexExprs, indexPreds, - oldInfo->ii_Unique, + oldInfo->ii_Unique && !skip_constraint_checks, oldInfo->ii_NullsNotDistinct, !concurrently, /* isready */ concurrently, /* concurrent */ diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 713bb5d10f1..70da622cb2f 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -4120,7 +4120,8 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein INDEX_CREATE_SUPPRESS_PROGRESS, idx->indexId, tablespaceid, - concurrentName); + concurrentName, + false); /* * Now open the relation of the new index, a session-level lock is diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index dde56fb1e8d..697bc4a3c70 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -3406,9 +3406,18 @@ build_new_indexes(Relation NewHeap, Relation OldHeap, List *OldIndexes) Oid newindex; char *newName; Relation ind; + bool isvalid; ind = index_open(oldindex, ShareUpdateExclusiveLock); + /* + * An invalid index (e.g. left over from a failed CREATE INDEX + * CONCURRENTLY) may contradict its own constraints, so build its copy + * without enforcing them, like reindex_relation() does in the + * non-concurrent case. The old index stays invalid after the swap. + */ + isvalid = ind->rd_index->indisvalid; + newName = ChooseRelationName(get_rel_name(oldindex), NULL, "repacknew", @@ -3416,8 +3425,9 @@ build_new_indexes(Relation NewHeap, Relation OldHeap, List *OldIndexes) false); newindex = index_create_copy(NewHeap, INDEX_CREATE_SUPPRESS_PROGRESS, oldindex, ind->rd_rel->reltablespace, - newName); - copy_index_constraints(ind, newindex, RelationGetRelid(NewHeap)); + newName, !isvalid); + if (isvalid) + copy_index_constraints(ind, newindex, RelationGetRelid(NewHeap)); result = lappend_oid(result, newindex); index_close(ind, NoLock); diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index 9aee8226347..499654769f3 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -104,7 +104,8 @@ extern Oid index_create(Relation heapRelation, extern Oid index_create_copy(Relation heapRelation, uint16 flags, Oid oldIndexId, Oid tablespaceOid, - const char *newName); + const char *newName, + bool skip_constraint_checks); extern void index_concurrently_build(Oid heapRelationId, Oid indexRelationId); -- 2.54.0