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. That causes the refcount to be
decremented when the resource owner is released, which is wrong when the
TupleDesc should live beyond the current query (as it should in the
typcache). We could avoid that by creating a CacheResourceOwner that is
never reset, as discussed earlier, but at present I'm not sure if
there's any point, so I've just used a "dead" field.
Attached is a revised patch. Reference counting is only used for
lookup_rowtype_tupdesc(): as discussed there might be other places that
would benefit from this, but I haven't looked at that yet.
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.
The patch is WIP: a few regression tests fail due to TupleDesc leaks I
haven't fixed yet but that should be easily fixable, and there are a few
other minor issues to address. I'm just posting the patch now to get any
feedback.
(Apologies for not getting this done earlier, I had a touch of the flu
yesterday...)
-Neil
