Re: [HACKERS] SearchSysCache, SysCacheGetAttr, and heap_getattr()

[Tom Lane]

Stephen Frost  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

[HACKERS] SearchSysCache, SysCacheGetAttr, and heap_getattr()

[Stephen Frost]

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


signature.asc
Description: Digital signature

[HACKERS] SearchSysCache

[uwcssa]

My program (indirectly) calls the following function twice,

tuple = SearchSysCache(STATRELATT, ObjectIdGetDatum(relid),

Int16GetDatum(colnum),  0, 0);

The first time it assigns NULL to tuple, while the second time it
assigns a valid pointer. Why is it like that?  BTW, my program only
optimize query plan without executing it.

Thanks.

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
  choose an index scan if your joining column's datatypes do not
  match

Re: [HACKERS] SearchSysCache

[Tom Lane]

uwcssa [EMAIL PROTECTED] writes:
 My program (indirectly) calls the following function twice,
 tuple = SearchSysCache(STATRELATT, ObjectIdGetDatum(relid),
 Int16GetDatum(colnum),  0, 0);
 The first time it assigns NULL to tuple, while the second time it
 assigns a valid pointer. Why is it like that?

You did an ANALYZE in between, perhaps?  That's the only operation
that puts new rows in pg_statistic.

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

[HACKERS] SearchSysCache changes committed

[Tom Lane]

I have committed changes to keep reference counts for system cache entries.
This should eliminate the issues we've had with cache entries sometimes
getting dropped while still in use.  Some notes:

1. The routine formerly called SearchSysCacheTuple is now SearchSysCache().
It increments the reference count on the returned cache entry.  You must
drop the reference count when done using the cache entry, so the typical
call scenario is now something like

tuple = SearchSysCache(...);
if (HeapTupleIsValid(tuple))
{
... use tuple ...
ReleaseSysCache(tuple);
}

2. If a cache inval message arrives for a cache entry with refcount  0,
the cache entry will not be dropped until the refcount goes to zero.
However, it will immediately be marked "dead" and so will not be found
by subsequent cache searches.

3. It turned out not to be hard to make the parser drop reference counts
when done with cache entries, so I went over to a hard-and-fast rule
that everyone must drop acquired refcounts.  If you don't, you'll get
an annoying NOTICE at commit time, just like for buffer refcount leaks.

4. There are several convenience routines for common usage patterns:

* SearchSysCacheCopy() --- formerly SearchSysCacheTupleCopy() --- still
exists, although the need for it is less than before.  You do NOT need
this routine just to hang onto a reference to a cache entry for awhile.
You use it if you want to update the tuple and need a modifiable copy
to scribble on.  When you use this routine, you get back a palloc'd
tuple (free it with heap_freetuple), and the original cache entry does
not have its refcount bumped.

* SearchSysCacheExists() just probes for the existence of a tuple via
the cache; it returns true or false without bumping the refcount.

* GetSysCacheOid() returns the cache entry's OID, or InvalidOid if no
entry found, leaving the refcount un-bumped.

* There are some other new convenience routines too in parse_oper.c,
parse_type.c, and lsyscache.c, to reduce the number of places that
have to bother with the full SearchSysCache/ReleaseSysCache protocol.

regards, tom lane