Re: Inconsistent error message wording for REINDEX CONCURRENTLY

On Mon, May 27, 2019 at 12:20:58AM -0400, Alvaro Herrera wrote:
> I wonder if we really want to abolish all distinction between "cannot do
> X" and "Y is not supported".  I take the former to mean that the
> operation is impossible to do for some reason, while the latter means we
> just haven't implemented it yet and it seems likely to get implemented
> in a reasonable timeframe.  See some excellent commentary about about
> the "can not" wording at
> https://postgr.es/m/CA+TgmoYS8jKhETyhGYTYMcbvGPwYY=qA6yYp9B47MX7MweE25w@mail.gmail.com

Incorrect URL?

> I notice your patch changes "catalog relations" to "system catalogs".
> I think we predominantly prefer the latter, so that part of your change
> seems OK.  (In passing, I noticed we have a couple of places using
> "system catalog tables", which is weird.)

Good point.  These are not new though, so I would prefer not touch
those parts for this patch.
src/backend/catalog/index.c:                 errmsg("user-defined
indexes on system catalog tables are not supported")));
src/backend/catalog/index.c:                 errmsg("concurrent index
creation on system catalog tables is not supported")));
src/backend/catalog/index.c:                 errmsg("user-defined
indexes on system catalog tables are not supported")));
src/backend/parser/parse_clause.c:               errmsg("ON CONFLICT
is not supported with system catalog tables"),

> I think reindexing system catalogs concurrently is a complex enough
> undertaking that implementing it is far enough in the future that the
> "cannot" wording is okay; but reindexing partitioned tables is not so
> obviously out of the question.

I am not sure that we actually can without much complication, as
technically locks on catalogs may get released before commit if I
recall correctly.

> We do have "is not yet implemented" in a
> couple of other places, so all things considered I'm not so sure about
> changing that one to "cannot".

Okay.  I can live with this difference.  Not changing the string in
ReindexRelationConcurrently() has the merit to be consistent with the
existing ones in reindex_relation() and ReindexPartitionedIndex().
Please find attached an updated version.  What do you think?
--
Michael