On Tue, May 14, 2019 at 11:32:52AM -0400, Alvaro Herrera wrote: > I do :-)
And actually I am happy to see somebody raising that point. The current log messages are quite inconsistent for a couple of years now but I did not bother changing anything other than the new strings per the feedback I got until, well, yesterday. > It seems strange to have some cases say "cannot do foo" and other cases > say "foo is not supported". However, I think having > ReindexMultipleTables say "cannot reindex a system catalog" would be > slightly wrong (since we're not reindexing one but many) -- so it would > have to be "cannot reindex system catalogs". And in order to avoid > having two messages that are essentially identical except in number, I > propose to change the others to use the plural too. So the one you just > committed > > would become "cannot reindex system catalogs concurrently", identical to > the one in ReindexMultipleTables. There are also a couple of similar, much older, error messages in index_create() for concurrent creation. Do you think that these should be changed? I can see benefits for translators to unify things a bit more, but these do not directly apply to REINDEX, and all messages are a bit different depending on the context. One argument to change them is that they don't comply with the project style as they use full sentences. Perhaps something like the attached for the REINDEX portion would make the world a better place? What do you think? -- Michael
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 7e7c03ef12..8501b29b0a 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2485,7 +2485,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("concurrent reindex of system catalogs is not supported")));
+ errmsg("cannot reindex system catalogs concurrently")));
/*
* Get OID of object to reindex, being the database currently being used
@@ -2599,7 +2599,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
if (!concurrent_warning)
ereport(WARNING,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("concurrent reindex is not supported for catalog relations, skipping all")));
+ errmsg("cannot reindex system catalogs concurrently, skipping all")));
concurrent_warning = true;
continue;
}
@@ -2747,7 +2747,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
if (IsCatalogRelationOid(relationOid))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot reindex a system catalog concurrently")));
+ errmsg("cannot reindex system catalogs concurrently")));
/* Open relation to get its indexes */
heapRelation = table_open(relationOid, ShareUpdateExclusiveLock);
@@ -2841,7 +2841,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
if (IsCatalogRelationOid(heapId))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot reindex a system catalog concurrently")));
+ errmsg("cannot reindex system catalogs concurrently")));
/* Save the list of relation OIDs in private context */
oldcontext = MemoryContextSwitchTo(private_context);
@@ -2862,7 +2862,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
/* see reindex_relation() */
ereport(WARNING,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("REINDEX of partitioned tables is not yet implemented, skipping \"%s\"",
+ errmsg("cannot reindex partitioned tables, skipping \"%s\"",
get_rel_name(relationOid))));
return false;
default:
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index c8bc6be061..fda9878f25 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2045,7 +2045,7 @@ REINDEX TABLE concur_reindex_part_10;
WARNING: REINDEX of partitioned tables is not yet implemented, skipping "concur_reindex_part_10"
NOTICE: table "concur_reindex_part_10" has no indexes
REINDEX TABLE CONCURRENTLY concur_reindex_part_10;
-WARNING: REINDEX of partitioned tables is not yet implemented, skipping "concur_reindex_part_10"
+WARNING: cannot reindex partitioned tables, skipping "concur_reindex_part_10"
NOTICE: table "concur_reindex_part_10" has no indexes
SELECT relid, parentrelid, level FROM pg_partition_tree('concur_reindex_part_index')
ORDER BY relid, level;
@@ -2093,19 +2093,19 @@ REINDEX TABLE CONCURRENTLY concur_reindex_tab;
ERROR: REINDEX CONCURRENTLY cannot run inside a transaction block
COMMIT;
REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relation
-ERROR: cannot reindex a system catalog concurrently
+ERROR: cannot reindex system catalogs concurrently
REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index
-ERROR: cannot reindex a system catalog concurrently
+ERROR: cannot reindex system catalogs concurrently
-- These are the toast table and index of pg_authid.
REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table
-ERROR: cannot reindex a system catalog concurrently
+ERROR: cannot reindex system catalogs concurrently
REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index
-ERROR: cannot reindex a system catalog concurrently
+ERROR: cannot reindex system catalogs concurrently
REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
-ERROR: concurrent reindex of system catalogs is not supported
+ERROR: cannot reindex system catalogs concurrently
-- Warns about catalog relations
REINDEX SCHEMA CONCURRENTLY pg_catalog;
-WARNING: concurrent reindex is not supported for catalog relations, skipping all
+WARNING: cannot reindex system catalogs concurrently, skipping all
-- Check the relation status, there should not be invalid indexes
\d concur_reindex_tab
Table "public.concur_reindex_tab"
signature.asc
Description: PGP signature
