From c1c5d28f4a2df9d83ca1bcd6bcd6ed8ca065791b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= Date: Fri, 11 Sep 2026 13:42:45 +0200 Subject: [PATCH] Disallow REPACK (CONCURRENTLY) when replica identity index is dropped The replica identity determination was "flawed": it fell back to the primary key if the marked index was dropped, which is a defensible choice, but logical decoding has a differing opinion and fails to provide usable tuple identity data in this case. "Fix" REPACK by refusing to use the primary key in that case. This is arguably broken behavior in RelationGetIndexList, but I'll refrain from changing that at this stage. Reported-by: Nathan Bossart Author: Matthias van de Meent Author: Ewan Young Backpatch-through: 19 Discussion: https://postgr.es/m/apCBRKCH8jwKiaSY@nathan --- contrib/test_decoding/expected/repack.out | 10 ++++++++++ contrib/test_decoding/sql/repack.sql | 9 +++++++++ src/backend/commands/repack.c | 10 ++++------ src/backend/utils/cache/relcache.c | 7 +++++++ 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/contrib/test_decoding/expected/repack.out b/contrib/test_decoding/expected/repack.out index ac5473137d1..0d229050146 100644 --- a/contrib/test_decoding/expected/repack.out +++ b/contrib/test_decoding/expected/repack.out @@ -111,6 +111,16 @@ REPACK (CONCURRENTLY) repack_conc_replident; ERROR: cannot execute REPACK (CONCURRENTLY) on relation "repack_conc_replident" DETAIL: This operation does not support deferrable primary keys. HINT: Use ALTER TABLE ... REPLICA IDENTITY USING INDEX to designate another index as replica identity. +-- Doesn't support tables whose replica identity indexes were dropped, even +-- if a workable primary key is present. +ALTER TABLE repack_conc_replident DROP CONSTRAINT repack_conc_replident_pkey, + ADD PRIMARY KEY (i); +CREATE UNIQUE INDEX replidx ON repack_conc_replident (i); +ALTER TABLE repack_conc_replident REPLICA IDENTITY USING INDEX replidx; +DROP INDEX replidx; +REPACK (CONCURRENTLY) repack_conc_replident; +ERROR: cannot execute REPACK (CONCURRENTLY) on relation "repack_conc_replident" +DETAIL: Relation "repack_conc_replident" has no identity index. -- 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 e995c72d28d..6e51aedb72d 100644 --- a/contrib/test_decoding/sql/repack.sql +++ b/contrib/test_decoding/sql/repack.sql @@ -83,6 +83,15 @@ REPACK (CONCURRENTLY) repack_conc_replident; ALTER TABLE repack_conc_replident ADD PRIMARY KEY (i) DEFERRABLE; REPACK (CONCURRENTLY) repack_conc_replident; +-- Doesn't support tables whose replica identity indexes were dropped, even +-- if a workable primary key is present. +ALTER TABLE repack_conc_replident DROP CONSTRAINT repack_conc_replident_pkey, + ADD PRIMARY KEY (i); +CREATE UNIQUE INDEX replidx ON repack_conc_replident (i); +ALTER TABLE repack_conc_replident REPLICA IDENTITY USING INDEX replidx; +DROP INDEX replidx; +REPACK (CONCURRENTLY) repack_conc_replident; + -- clean up DROP TABLE repack_conc_replident, clstrpart; diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index 1fa48f8fcc0..83168a4e6f3 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -971,13 +971,11 @@ check_concurrent_repack_requirements(Relation rel, Oid *ident_idx_p) "REPLICA IDENTITY NOTHING" : "REPLICA IDENTITY FULL")); /* - * Obtain the replica identity index -- either one that has been set - * explicitly, or a non-deferrable primary key. If none of these cases - * apply, the table cannot be repacked concurrently. It might be possible - * to have repack work with a FULL replica identity; however that requires - * more work and is not implemented yet. + * Obtain the replica identity index to use. If there isn't one, the + * table cannot be repacked concurrently. (Replica identity FULL is not + * supported yet.) */ - ident_idx = GetRelationIdentityOrPK(rel); + ident_idx = RelationGetReplicaIndex(rel); if (!OidIsValid(ident_idx)) { /* This special case warrants its own error message */ diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index a8b106e36af..541f30f0972 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -5075,6 +5075,13 @@ RelationGetPrimaryKeyIndex(Relation relation, bool deferrable_ok) /* * RelationGetReplicaIndex -- get OID of the relation's replica identity index * + * If replica identity is DEFAULT, then return the OID of the primary key, if + * it's not deferrable; if replica identity is INDEX, return the OID of the + * index with indisreplident, if one exists. + * + * Note that a working PK is not returned if identity is INDEX! This is + * surprising if the replica identity index is dropped. FIXME someday. + * * Returns InvalidOid if there is no such index. */ Oid