Re: TupleDesc refcounting

Neil Conway <neilc@samurai.com> writes:
> On Tue, 2006-01-17 at 09:36 -0500, Tom Lane wrote:
>> I suspect you'll find that it's convenient to treat typcache's own
>> link to a tupdesc as a reference count too, so it's not strictly
>> "external" references that should be counted.

> The problem with this is that incrementing a TupleDesc's refcount
> informs the CurrentResourceOwner.

Not if it's just "tupdesc->refcount++;" ... ;-)

Seriously, I was imagining a two-level structure where tupdesc itself
just has operations on the order of

    refcount++;

    if (--refcount <= 0)
        FreeTupleDesc();

and then a separate layer on top of that adds ResourceOwner management.
This is precisely because the cache code requires non-resource-owner-
managed links (or at least, has no particular use for the services of a
ResourceOwner).

> There was some call-sites of lookup_rowtype_tupdesc() where it doesn't
> seem to be easy to use reference counting. For example, consider the
> implementation of get_expr_result_type(): some code paths within that
> function call lookup_rowtype_tupdesc() to produce the returned
> TupleDesc, and some do not. The easiest fix seemed to be just making a
> copy of the TupleDesc for the lookup_rowtype_tupdesc() cases.

Yeah, I noticed this while making the back-branch patch.  I agree that
any given routine had better have a consistent return convention: if
the tupdesc needs to be refcount-decremented when done, that must be
the case in all code paths, since the caller can't be expected to know
which case applies.  My recollection though is that the non-lookup cases
return freshly-built-in-local-storage tupdescs, which could certainly be
set up as having refcount 1 so that the decrement would work correctly.
It may or may not be worth the trouble compared to just copying, though.
You should think about whether a routine is a performance hotspot before
going out of your way to avoid a copy step.

> (Apologies for not getting this done earlier, I had a touch of the flu
> yesterday...)

There's no hurry about it.  Hope you're feeling better.

            regards, tom lane