From 4bc86d92da636de9d9f9a4acac40fd15ac67d709 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Tue, 8 Sep 2026 15:52:15 +0000 Subject: [PATCH v5 2/2] Use detail messages consistently in REPACK (CONCURRENTLY) checks. check_concurrent_repack_requirements() reported its restrictions with a mix of hint and detail messages. Some messages that only describe why the error happened (system catalogs, TOAST tables, non-permanent relations, and the missing identity index) were written as hints, and the detail messages repeated "REPACK (CONCURRENTLY)", which the main error message already names. Change these so each restriction is reported as a detail message with consistent "This operation ..." wording, matching errdetail_relkind_not_supported(). The only hint left is the useful one that suggests ALTER TABLE ... REPLICA IDENTITY USING INDEX. Author: Bharath Rupireddy Reviewed-by: Alvaro Herrera Discussion: https://postgr.es/m/apBIFWzHYOaG0auN%40nathan Backpatch-through: 19 --- contrib/test_decoding/expected/repack.out | 12 ++++++------ src/backend/commands/repack.c | 22 ++++++++-------------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/contrib/test_decoding/expected/repack.out b/contrib/test_decoding/expected/repack.out index ce2cadfa222..b85cd92aa1a 100644 --- a/contrib/test_decoding/expected/repack.out +++ b/contrib/test_decoding/expected/repack.out @@ -60,7 +60,7 @@ HINT: Consider running the command on individual partitions. -- Disallowed in catalogs REPACK (CONCURRENTLY) pg_class; ERROR: cannot execute REPACK (CONCURRENTLY) on relation "pg_class" -HINT: REPACK (CONCURRENTLY) is not supported for catalog relations. +DETAIL: This operation is not supported for system catalogs. -- Doesn't support tables used as catalog tables CREATE TABLE repack_conc_user_catalog (i int) WITH (user_catalog_table = true); REPACK (CONCURRENTLY) repack_conc_user_catalog; @@ -80,30 +80,30 @@ DROP TABLE repack_conc_toast; CREATE TEMP TABLE repack_conc_temp (i int PRIMARY KEY); REPACK (CONCURRENTLY) repack_conc_temp; ERROR: cannot execute REPACK (CONCURRENTLY) on relation "repack_conc_temp" -HINT: REPACK (CONCURRENTLY) is only allowed for permanent relations. +DETAIL: This operation is only supported for permanent relations. DROP TABLE repack_conc_temp; CREATE UNLOGGED TABLE repack_conc_unlogged (i int PRIMARY KEY); REPACK (CONCURRENTLY) repack_conc_unlogged; ERROR: cannot execute REPACK (CONCURRENTLY) on relation "repack_conc_unlogged" -HINT: REPACK (CONCURRENTLY) is only allowed for permanent relations. +DETAIL: This operation is only supported for permanent relations. DROP TABLE repack_conc_unlogged; -- Doesn't support tables with REPLICA IDENTITY NOTHING, even if they have a primary key CREATE TABLE repack_conc_replident (i int PRIMARY KEY); ALTER TABLE repack_conc_replident REPLICA IDENTITY NOTHING; REPACK (CONCURRENTLY) repack_conc_replident; ERROR: cannot execute REPACK (CONCURRENTLY) on relation "repack_conc_replident" -DETAIL: REPACK (CONCURRENTLY) does not support tables with REPLICA IDENTITY NOTHING. +DETAIL: This operation does not support tables with REPLICA IDENTITY NOTHING. -- Doesn't support tables without a primary key or replica identity index ALTER TABLE repack_conc_replident DROP CONSTRAINT repack_conc_replident_pkey; ALTER TABLE repack_conc_replident REPLICA IDENTITY DEFAULT; REPACK (CONCURRENTLY) repack_conc_replident; ERROR: cannot execute REPACK (CONCURRENTLY) on relation "repack_conc_replident" -HINT: Relation "repack_conc_replident" has no identity index. +DETAIL: Relation "repack_conc_replident" has no identity index. -- Doesn't support tables with deferrable primary keys ALTER TABLE repack_conc_replident ADD PRIMARY KEY (i) DEFERRABLE; REPACK (CONCURRENTLY) repack_conc_replident; ERROR: cannot execute REPACK (CONCURRENTLY) on relation "repack_conc_replident" -DETAIL: REPACK (CONCURRENTLY) does not support deferrable primary keys. +DETAIL: This operation does not support deferrable primary keys. HINT: Use ALTER TABLE ... REPLICA IDENTITY USING INDEX to designate another index as replica identity. -- clean up DROP TABLE repack_conc_replident, clstrpart; diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index 086bef834ec..f6da2b1085d 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -885,8 +885,7 @@ check_concurrent_repack_requirements(Relation rel, Oid *ident_idx_p) errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("cannot execute %s in this configuration", "REPACK (CONCURRENTLY)"), - errdetail("%s requires \"wal_level\" to be set to \"replica\" or higher.", - "REPACK (CONCURRENTLY)")); + errdetail("This operation requires \"wal_level\" to be set to \"replica\" or higher.")); /* Data changes in system relations are not logically decoded. */ if (IsCatalogRelation(rel)) @@ -894,8 +893,7 @@ check_concurrent_repack_requirements(Relation rel, Oid *ident_idx_p) errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot execute %s on relation \"%s\"", "REPACK (CONCURRENTLY)", RelationGetRelationName(rel)), - errhint("%s is not supported for catalog relations.", - "REPACK (CONCURRENTLY)")); + errdetail("This operation is not supported for system catalogs.")); /* * REPACK (CONCURRENTLY) is not MVCC-safe; it doesn't preserve visibility @@ -919,8 +917,7 @@ check_concurrent_repack_requirements(Relation rel, Oid *ident_idx_p) errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot execute %s on relation \"%s\"", "REPACK (CONCURRENTLY)", RelationGetRelationName(rel)), - errhint("%s is not supported for TOAST relations.", - "REPACK (CONCURRENTLY)")); + errdetail("This operation is not supported for TOAST tables.")); relpersistence = rel->rd_rel->relpersistence; if (relpersistence != RELPERSISTENCE_PERMANENT) @@ -928,8 +925,7 @@ check_concurrent_repack_requirements(Relation rel, Oid *ident_idx_p) errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("cannot execute %s on relation \"%s\"", "REPACK (CONCURRENTLY)", RelationGetRelationName(rel)), - errhint("%s is only allowed for permanent relations.", - "REPACK (CONCURRENTLY)")); + errdetail("This operation is only supported for permanent relations.")); /* * With NOTHING, WAL does not contain the old tuple; FULL is not yet @@ -942,8 +938,7 @@ check_concurrent_repack_requirements(Relation rel, Oid *ident_idx_p) errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("cannot execute %s on relation \"%s\"", "REPACK (CONCURRENTLY)", RelationGetRelationName(rel)), - errdetail("%s does not support tables with %s.", - "REPACK (CONCURRENTLY)", + errdetail("This operation does not support tables with %s.", replident == REPLICA_IDENTITY_NOTHING ? "REPLICA IDENTITY NOTHING" : "REPLICA IDENTITY FULL")); @@ -964,16 +959,15 @@ check_concurrent_repack_requirements(Relation rel, Oid *ident_idx_p) errmsg("cannot execute %s on relation \"%s\"", "REPACK (CONCURRENTLY)", RelationGetRelationName(rel)), - errdetail("%s does not support deferrable primary keys.", - "REPACK (CONCURRENTLY)"), + errdetail("This operation does not support deferrable primary keys."), errhint("Use ALTER TABLE ... REPLICA IDENTITY USING INDEX to designate another index as replica identity.")); ereport(ERROR, errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("cannot execute %s on relation \"%s\"", "REPACK (CONCURRENTLY)", RelationGetRelationName(rel)), - errhint("Relation \"%s\" has no identity index.", - RelationGetRelationName(rel))); + errdetail("Relation \"%s\" has no identity index.", + RelationGetRelationName(rel))); } *ident_idx_p = ident_idx; -- 2.47.3