Re: reindexing an invalid index should not use ERRCODE_INDEX_CORRUPTED

On Sat, Nov 18, 2023 at 03:09:58PM -0800, Andres Freund wrote:
> We currently provide no way to learn about a postgres instance having
> corruption than searching the logs for corruption events than matching by
> sqlstate, for ERRCODE_DATA_CORRUPTED and ERRCODE_INDEX_CORRUPTED.
> 
> Unfortunately, there is a case of such an sqlstate that's not at all indicating
> corruption, namely REINDEX CONCURRENTLY when the index is invalid:
> 
>                         if (!indexRelation->rd_index->indisvalid)
>                             ereport(WARNING,
>                                     (errcode(ERRCODE_INDEX_CORRUPTED),
>                                      errmsg("cannot reindex invalid index \"%s.%s\" concurrently, skipping",
>                                             get_namespace_name(get_rel_namespace(cellOid)),
>                                             get_rel_name(cellOid))));
> 
> The only thing required to get to this is an interrupted CREATE INDEX
> CONCURRENTLY, which I don't think can be fairly characterized as "corruption".
> 
> ISTM something like ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE would be more
> appropriate?

+1, that's a clear improvement.

The "cannot" part of the message is also inaccurate, and it's not clear to me
why we have this specific restriction at all.  REINDEX INDEX CONCURRENTLY
accepts such indexes, so I doubt it's an implementation gap.  Since an INVALID
index often duplicates some valid index, I could see an argument that
reindexing INVALID indexes as part of a table-level REINDEX is wanted less
often than not.  But that argument would be just as pertinent to REINDEX TABLE
(w/o CONCURRENTLY), which doesn't impose this restriction.  Hmmm.