On Wed, 18 Feb 2026 at 03:30, Michael Paquier <[email protected]> wrote: > > Hi all, > > While reviewing the syscache code, I have bumped into the following > funny bit in objectaddress.c: > if (oidCacheId > 0) > { > if (locktup) > tuple = SearchSysCacheLockedCopy1(oidCacheId, > ObjectIdGetDatum(objectId)); > else > tuple = SearchSysCacheCopy1(oidCacheId, > ObjectIdGetDatum(objectId)); > if (!HeapTupleIsValid(tuple)) /* should not happen */ > return NULL; > } > > This is wrong, because SysCacheIdentifier starts at 0. This has no > consequence currently, because the first value in the enum is > AGGFNOID, something that we don't rely on for object type lookups. > Even if this code had the idea to use an ID of 0, the logic would just > get back to a systable lookup, that would still work, that's just less > efficient.
As I can see, this if-statement was introduced in 994c36e, at which point it was already wrong (AGGFNOID was present and equal to 0 back then). I agree with analysis, SearchSysCache function does this check: ``` if (cacheId < 0 || cacheId >= SysCacheSize || !PointerIsValid(SysCache[cacheId])) elog(ERROR, "invalid cache ID: %d", cacheId); ``` > Simple patch attached, planned for a backpatch quickly as I am playing > with a different patch that reworks a bit this code. LGTM > Regards, > -- > Michael -- Best regards, Kirill Reshke
