From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: Michael Paquier <michael@paquier.xyz>,
        Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: REINDEX INDEX results in a crash for an index of pg_class since
 9.6
In-reply-to: <20190425210309.cnofpjnwsrhdnahd@alap3.anarazel.de>
References: <20190423233540.GB1878@paquier.xyz>
 <13930.1556063692@sss.pgh.pa.us> <14271.1556064217@sss.pgh.pa.us>
 <20190425130916.GG8552@paquier.xyz> <22317.1556206341@sss.pgh.pa.us>
 <25143.1556209756@sss.pgh.pa.us>
 <20190425181648.lgffj43btacmxszu@alap3.anarazel.de>
 <26665.1556218209@sss.pgh.pa.us>
 <20190425195130.hhdp5tcpapnupszq@alap3.anarazel.de>
 <29295.1556222523@sss.pgh.pa.us>
 <20190425210309.cnofpjnwsrhdnahd@alap3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Thu, 25 Apr 2019 14:03:09 -0700"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <31932.1556226753.1@sss.pgh.pa.us>
Date: Thu, 25 Apr 2019 17:12:33 -0400
Message-ID: <31933.1556226753@sss.pgh.pa.us>
Precedence: bulk

Andres Freund <andres@anarazel.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