Re: reindexing an invalid index should not use ERRCODE_INDEX_CORRUPTED

On Sat, Nov 18, 2023 at 04:32:36PM -0800, Noah Misch wrote:
> On Sat, Nov 18, 2023 at 03:09:58PM -0800, Andres Freund wrote:
>> 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 same thing can be said a couple of lines above where the code uses
ERRCODE_FEATURE_NOT_SUPPORTED but your suggestion of
ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE would be better.

Would the attached be OK for you?

> 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.

If you would reword that, what would you change?

> 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.

The argument behind this restriction is that repeated interruptions of
a table-level REINDEX CONCURRENTLY would bloat the entire relation in
index entries if invalid entries are rebuilt. This was discussed back
on the original thread back in 2019, around here:
https://www.postgresql.org/message-id/20190411132704.GC30766@paquier.xyz
--
Michael