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-04-25 16:02:03 -0400, Tom Lane wrote:
>> That could work. The important API spec is then that the relcache entry
>> reflects the *previous* state of the relation, and is not to be modified
>> by the tableam call.

> Right.

> I was wondering if we should just pass in the pg_class tuple as an "out"
> argument, instead of pointers to relfilnode/relfrozenxid/relminmxid.

Yeah, possibly. The whole business with xids is perhaps heapam specific,
so decoupling this function's signature from them would be good.

> My point was that given the current coding the code in
> ATExecSetTableSpace() would make changes to the *old* relfilenode, after
> having already copied the contents to the new relfilenode. Which means
> that if ATExecSetTableSpace() is ever used on pg_class or one of it's
> indexes, it'd just loose those changes, afaict.

Hmm.

There's another reason why we'd like the relcache contents to only change
at CCI, which is that if we get a relcache invalidation somewhere before
we get to the CCI, relcache.c would proceed to reload it based on the
*current* catalog contents (ie, pre-update, thanks to the magic of MVCC),
so that the entry would revert back to its previous state until we did get
to CCI. I wonder whether there's any demonstrable bug there. Though
you'd think the CLOBBER_CACHE_ALWAYS animals would've found it if so.

regards, tom lane