SearchSysCache, SysCacheGetAttr, and heap_getattr()

Started by Stephen Frostalmost 9 years ago2 messages
#1Stephen Frost
sfrost@snowman.net

Greetings,

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. 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()...

In short, should we be fixing these cases to always use
SysCacheGetAttr() when working with a tuple returned by
SearchSysCache()?

Thanks!

Stephen

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Stephen Frost (#1)
Re: SearchSysCache, SysCacheGetAttr, and heap_getattr()

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers