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

On Wed, 18 Feb 2026 at 03:30, Michael Paquier <michael@paquier.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