Re: [PATCH] Check more invariants during syscache initialization

On Mon, Jul 24, 2023 at 09:58:15PM +0300, Aleksander Alekseev wrote:
> Currently InitCatalogCache() has only one Assert() for cacheinfo[]
> that checks .reloid. The proposed patch adds sanity checks for the
> rest of the fields.

- Assert(cacheinfo[cacheId].reloid != 0);
+ Assert(cacheinfo[cacheId].reloid != InvalidOid);
+ Assert(cacheinfo[cacheId].indoid != InvalidOid);
No objections about checking for the index OID given out to catch
any failures at an early stage before doing an actual lookup? I guess
that you've added an incorrect entry and noticed the problem only when
triggering a syscache lookup for the new incorrect entry?

+ Assert(key[i] != InvalidAttrNumber);

Yeah, same here.

+ Assert((cacheinfo[cacheId].nkeys > 0) && (cacheinfo[cacheId].nkeys <= 4));

Is this addition actually useful? The maximum number of lookup keys
is enforced at API level with the SearchSysCache() family. Or perhaps
you've been able to break this layer in a way I cannot imagine and
were looking at a quick way to spot the inconsistency?
--
Michael