From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Inconsistent error message wording for REINDEX CONCURRENTLY
Date: Tue, 14 May 2019 11:32:52 -0400
Message-ID: <20190514153252.GA22168@alvherre.pgsql> (raw)
In-Reply-To: <20190509041154.GC14573@paquier.xyz>
On 2019-May-09, Michael Paquier wrote:
> On Wed, May 08, 2019 at 11:28:35PM -0400, Tom Lane wrote:
> > Michael Paquier <michael@paquier.xyz> writes:
> >> No problem to do that. I'll brush up all that once you commit the
> >> first piece you have come up with, and reuse the new API of catalog.c
> >> you are introducing based on the table OID.
> >
> > Pushed my stuff, have at it.
>
> Thanks. Attached is what I get to after scanning the error messages
> in indexcmds.c and index.c. Perhaps you have more comments about it?
I do :-) There are a couple of "is not supported" messages that are
annoyingly similar but different:
git grep --show-function 'reindex.*supported' -- *.c
src/backend/commands/indexcmds.c=ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
src/backend/commands/indexcmds.c: errmsg("concurrent reindex of system catalogs is not supported")));
src/backend/commands/indexcmds.c: errmsg("concurrent reindex is not supported for catalog relations, skipping all")));
src/backend/commands/indexcmds.c=ReindexRelationConcurrently(Oid relationOid, int options)
src/backend/commands/indexcmds.c: errmsg("concurrent reindex is not supported for shared relations")));
src/backend/commands/indexcmds.c: errmsg("concurrent reindex is not supported for catalog relations")));
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
> + /* A system catalog cannot be reindexed concurrently */
> + if (IsCatalogRelationOid(relationOid))
> + ereport(ERROR,
> + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> + errmsg("cannot reindex a system catalog concurrently")));
would become "cannot reindex system catalogs concurrently", identical to
the one in ReindexMultipleTables.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
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: alvherre@2ndquadrant.com, michael@paquier.xyz, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Subject: Re: Inconsistent error message wording for REINDEX CONCURRENTLY
In-Reply-To: <20190514153252.GA22168@alvherre.pgsql>
* 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