Re: lookup_rowtype_tupdesc considered harmful

Neil Conway <neilc@samurai.com> writes:
> Hmm, okay. There's the additional complication that we need to handle
> record types (see RecordCacheArray in typcache.c). Since I don't think
> we need reference counting for those,

Yeah, you do.  See record_out for instance, and reflect on the fact that
it can have no idea what the called I/O functions are liable to do.

> where lookup_rowtype_tupdesc() returns a pointer to this struct:

> typedef struct
> {
>     struct tupleDesc tdesc; /* must be first field */

>     TypeCacheEntry *tentry; /* pointer to owning TypeCacheEntry,
>                                or NULL if this is a record type */
> } MagicTupleDesc;

> and where TypeCacheEntry has been modified to contain a reference count
> and an "is dead?" flag.

No, the refcount and isdead flags should be in *this* struct, and
there's really no need for a back-link to TypeCacheEntry.  Think harder
about the situation where there are both old and new tupdescs
outstanding for a single typcache entry.

> Is there actually a need for the (ugly) "magic
> value" hackery used by catcache?

It's a safety check to make sure that what got passed to
release_rowtype_tupdesc is really one of these animals and not just any
old TupleDesc.  It's not *necessary*, granted, but given that the compiler
is not going to help people avoid such errors, I think it's prudent.
        regards, tom lane