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

Hi,

On 2019-05-02 11:41:28 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > ISTM that if we go down this path, we should split (not now, but either
> > still in v12, or *early* in v13), the sets of indexes that are intended
> > to a) not being used for catalog queries b) may be skipped for index
> > insertions. It seems pretty likely that somebody will otherwise soon
> > introduce an heap_update() somewhere into the index build process, and
> > it'll work just fine in testing due to HOT.
>
> Given the assertions you added in CatalogIndexInsert, I'm not sure
> why that's a big hazard?

Afaict the new RelationSetIndexList() trickery would prevent that
assertion from being reached, because RelationGetIndexList() will not
see the current index, and therefore CatalogIndexInsert() won't know to
assert it either. It kinda works today for reindex_relation(), because
we'll "un-hide" the already rebuilt indexes - i.e. we'd not notice the
bug on pg_class' first index, but for later ones it'd trigger. I guess
you could argue that we'll just have to rely on REINDEX TABLE pg_class
regression tests to make sure REINDEX INDEX pg_class_* ain't broken :/.

> > I kinda wonder if there's not a third approach hiding somewhere here. We
> > could just stop updating pg_class in RelationSetNewRelfilenode() in
> > pg_class, when it's an index on pg_class.
>
> Hmm ... are all those indexes mapped? I guess so.

They are:

postgres[13357][1]=# SELECT oid::regclass, relfilenode FROM pg_class WHERE oid IN (SELECT indexrelid FROM pg_index WHERE indrelid = 'pg_class'::regclass);
┌───────────────────────────────────┬─────────────┐
│ oid │ relfilenode │
├───────────────────────────────────┼─────────────┤
│ pg_class_oid_index │ 0 │
│ pg_class_relname_nsp_index │ 0 │
│ pg_class_tblspc_relfilenode_index │ 0 │
└───────────────────────────────────┴─────────────┘
(3 rows)

I guess that doesn't stricly have to be the case for at least some of
them, but it seems unlikely that we'd want to change that.

> But don't we need to worry about resetting relfrozenxid?

Indexes don't have that though? We couldn't do it for pg_class itself,
but that's not a problem here.

Greetings,

Andres Freund