Stephen Frost <sfrost@snowman.net> writes:
> There's some inconsistency when it comes to if we actually use
> SysCacheGetAttr() when pulling an attribute for a tuple we got via
> SearchSysCache(), or if we use heap_getattr().
> Maybe I'm missing something, but that seems less than ideal.
Well, SysCacheGetAttr just invokes heap_getattr using a tuple descriptor
obtained from the syscache entry. AFAICT the point of it is that callers
need not lay their hands on a tuple descriptor for the relevant system
catalog some other way.
> I've generally been under the belief that using heap_getattr() is 'ok' when
> we've already opened and locked the relation, but there are some other
> checks done through SysCacheGetAttr() that you don't get with
> heap_getattr()...
Basically only that you supplied a valid cacheID, AFAICS.
> In short, should we be fixing these cases to always use
> SysCacheGetAttr() when working with a tuple returned by
> SearchSysCache()?
I can't get excited about it unless the caller is heap_open'ing
the catalog just to get a tupdesc for this purpose. Then it'd
be worth changing so you could remove the heap_open/heap_close.
If the caller has the catalog opened because it's going to do an
insert/update/delete, you could argue about whether it's stylistically
better to use a tupdesc from the syscache or one from the relation.
I think that might be a case-by-case decision, but I'd lean to using
a tupdesc from the relation when preparing tuples to be stored there.
regards, tom lane