Re: REINDEX INDEX results in a crash for an index of pg_class since 9.6

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2019-05-01 19:41:24 -0400, Tom Lane wrote:
>> OK, so per the other thread, it seems like the error recovery problem
>> isn't going to affect this directly. However, I still don't like this
>> proposal much; the reason being that it's a rather fundamental change
>> in the API of RelationSetNewRelfilenode. This will certainly break
>> any external callers of that function --- and silently, too.

> Couldn't we just address that by adding a new
> RelationSetNewRelfilenodeInternal() that's then wrapped by
> RelationSetNewRelfilenode() which just does
> RelationSetNewRelfilenodeInternal();CCI();?

That's just adding more ugliness ...

>> The solution I'm thinking of should have much more localized effects,
>> basically just in reindex_index and RelationSetNewRelfilenode, which is
>> why I like it better.

> Well, as I said before, I think hiding the to-be-rebuilt index from the
> list of indexes is dangerous too - if somebody added an actual
> CatalogUpdate/Insert (rather than inplace_update) anywhere along the
> index_build() path, we'd not get an assertion failure anymore, but just
> an index without the new entry. And given the fragility with HOT hiding
> that a lot of the time, that seems dangerous to me.

I think that argument is pretty pointless considering that "REINDEX TABLE
pg_class" does it this way, and that code is nearly old enough to vote.
Perhaps there'd be value in rewriting things so that we don't need
RelationSetIndexList at all, but it's not real clear to me what we'd do
instead, and in any case I don't agree with back-patching such a change.
In the near term it seems better to me to make "REINDEX INDEX
some-pg_class-index" handle this problem the same way "REINDEX TABLE
pg_class" has been doing for many years.

Attached is a draft patch for this. It passes check-world with
xxx_FORCE_RELEASE, and gets through reindexing pg_class with
CLOBBER_CACHE_ALWAYS, but I've not completed a full CCA run.

regards, tom lane