On 21/06/2024 02:12, Tom Lane wrote:
> Heikki Linnakangas <hlinnaka@iki.fi> writes:
>> In commit af0e7deb4a, I removed the call to RelationCloseSmgr() from
>> RelationCacheInvalidate(). I thought it was no longer needed, because we
>> no longer free the underlying SmgrRelation.
>
>> However, it meant that if the relfilenode of the relation was changed,
>> the relation keeps pointing to the SMgrRelation of the old relfilenode.
>> So we still need the RelationCloseSmgr() call, in case the relfilenode
>> has changed.
>
> Ouch. How come we did not see this immediately in testing? I'd have
> thought surely such a bug would be exposed by any command that
> rewrites a heap.
There is a RelationCloseSmgr() call in RelationClearRelation(), which
covers the common cases. This only occurs during
RelationCacheInvalidate(), when pg_class's relfilenumber was changed.
Hmm, looking closer, I think this might be a more appropriate place for
the RelationCloseSmgr() call:
> /*
> * If it's a mapped relation, immediately update its rd_locator in
> * case its relfilenumber changed. We must do this during phase 1
> * in case the relation is consulted during rebuild of other
> * relcache entries in phase 2. It's safe since consulting the
> * map doesn't involve any access to relcache entries.
> */
> if (RelationIsMapped(relation))
> RelationInitPhysicalAddr(relation);
That's where we change the relfilenumber, before the
RelationClearRelation() call.
--
Heikki Linnakangas
Neon (https://neon.tech)
