On 2019-May-09, Michael Paquier wrote: > On Wed, May 08, 2019 at 11:28:35PM -0400, Tom Lane wrote: > > Michael Paquier <mich...@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