Re: One-off with syscache ID in get_catalog_object_by_oid_extended()

On Wed, 18 Feb 2026 at 03:30, Michael Paquier <michael(at)paquier(dot)xyz> 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