From: Michael Paquier <michael@paquier.xyz>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Inconsistent error message wording for REINDEX CONCURRENTLY
Date: Wed, 15 May 2019 11:17:51 +0900
Message-ID: <20190515021609.GD1440@paquier.xyz> (raw)
In-Reply-To: <20190514153252.GA22168@alvherre.pgsql>
References: <20190509041154.GC14573@paquier.xyz>
<20190514153252.GA22168@alvherre.pgsql>
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
Attachments:reindex-error-strings.patch (text/x-diff, 4.5K) downloaddiff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.cindex 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.outindex 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 (application/pgp-signature, 832B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: michael@paquier.xyz, alvherre@2ndquadrant.com, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Subject: Re: Inconsistent error message wording for REINDEX CONCURRENTLY
In-Reply-To: <20190515021609.GD1440@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox