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

============================================================
*** src/backend/access/common/tupdesc.c	83ca807d4fdd572c409bc9214922b6ba9da7ce18
--- src/backend/access/common/tupdesc.c	c423df65341ed283d20ff03c4437ad67876ef0ba
***************
*** 23,28 ****
--- 23,29 ----
  #include "catalog/pg_type.h"
  #include "parser/parse_type.h"
  #include "utils/builtins.h"
+ #include "utils/resowner.h"
  #include "utils/syscache.h"
  
  
***************
*** 84,89 ****
--- 85,92 ----
  	desc->tdtypeid = RECORDOID;
  	desc->tdtypmod = -1;
  	desc->tdhasoid = hasoid;
+ 	desc->refcount = 0;
+ 	desc->dead = false;
  
  	return desc;
  }
***************
*** 116,121 ****
--- 119,126 ----
  	desc->tdtypeid = RECORDOID;
  	desc->tdtypmod = -1;
  	desc->tdhasoid = hasoid;
+ 	desc->refcount = 0;
+ 	desc->dead = false;
  
  	return desc;
  }
***************
*** 214,219 ****
--- 219,226 ----
  {
  	int			i;
  
+ 	Assert(tupdesc->refcount == 0);
+ 
  	if (tupdesc->constr)
  	{
  		if (tupdesc->constr->num_defval > 0)
***************
*** 246,251 ****
--- 253,279 ----
  	pfree(tupdesc);
  }
  
+ void
+ IncrTupleDescRefCount(TupleDesc tupdesc)
+ {
+ 	Assert(tupdesc->refcount >= 0);
+ 
+ 	ResourceOwnerEnlargeTupleDescs(CurrentResourceOwner);
+ 	tupdesc->refcount++;
+ 	ResourceOwnerRememberTupleDesc(CurrentResourceOwner, tupdesc);
+ }
+ 
+ void
+ DecrTupleDescRefCount(TupleDesc tupdesc)
+ {
+ 	Assert(tupdesc->refcount > 0);
+ 
+ 	tupdesc->refcount--;
+ 	ResourceOwnerForgetTupleDesc(CurrentResourceOwner, tupdesc);
+ 	if (tupdesc->refcount == 0 && tupdesc->dead)
+ 		FreeTupleDesc(tupdesc);
+ }
+ 
  /*
   * Compare two TupleDesc structures for logical equality
   *
============================================================
*** src/backend/access/heap/tuptoaster.c	f3ec822c0e6b6328bc0026716fcf4b133f89b092
--- src/backend/access/heap/tuptoaster.c	bbdea6bdd98d250326fad68899d07a8b153fb263
***************
*** 892,898 ****
--- 892,901 ----
  	 * If nothing to untoast, just return the original tuple.
  	 */
  	if (!need_change)
+ 	{
+ 		DecrTupleDescRefCount(tupleDesc);
  		return value;
+ 	}
  
  	/*
  	 * Calculate the new size of the tuple.  Header size should not change,
***************
*** 930,935 ****
--- 933,939 ----
  		if (toast_free[i])
  			pfree(DatumGetPointer(toast_values[i]));
  
+ 	DecrTupleDescRefCount(tupleDesc);
  	return PointerGetDatum(new_data);
  }
  
============================================================
*** src/backend/executor/execQual.c	eb1daef85341439578a3f6bb73549c4420292bc3
--- src/backend/executor/execQual.c	73a3d813603ce497b0a2c45a6f21fc001445d1b8
***************
*** 707,712 ****
--- 707,714 ----
  						  attrno,
  						  tupDesc,
  						  isNull);
+ 
+ 	DecrTupleDescRefCount(tupDesc);
  	return result;
  }
  
***************
*** 765,770 ****
--- 767,774 ----
  						  attrno,
  						  tupDesc,
  						  isNull);
+ 
+ 	DecrTupleDescRefCount(tupDesc);
  	return result;
  }
  
***************
*** 1149,1157 ****
  /*
   *		ExecMakeTableFunctionResult
   *
!  * Evaluate a table function, producing a materialized result in a Tuplestore
!  * object.	*returnDesc is set to the tupledesc actually returned by the
!  * function, or NULL if it didn't provide one.
   */
  Tuplestorestate *
  ExecMakeTableFunctionResult(ExprState *funcexpr,
--- 1153,1163 ----
  /*
   *		ExecMakeTableFunctionResult
   *
!  * Evaluate a table function, producing a materialized result in a
!  * Tuplestore object. *returnDesc is set to the tupledesc actually
!  * returned by the function, or NULL if it didn't provide one. If
!  * non-NULL, the caller should decrement the reference count on the
!  * TupleDesc when finished with it.
   */
  Tuplestorestate *
  ExecMakeTableFunctionResult(ExprState *funcexpr,
***************
*** 1336,1342 ****
  				{
  					/*
  					 * Use the type info embedded in the rowtype Datum to look
! 					 * up the needed tupdesc.  Make a copy for the query.
  					 */
  					HeapTupleHeader td;
  
--- 1342,1348 ----
  				{
  					/*
  					 * Use the type info embedded in the rowtype Datum to look
! 					 * up the needed tupdesc.
  					 */
  					HeapTupleHeader td;
  
***************
*** 1343,1356 ****
  					td = DatumGetHeapTupleHeader(result);
  					tupdesc = lookup_rowtype_tupdesc(HeapTupleHeaderGetTypeId(td),
  											   HeapTupleHeaderGetTypMod(td));
- 					tupdesc = CreateTupleDescCopy(tupdesc);
  				}
  				else
  				{
  					/*
! 					 * Scalar type, so make a single-column descriptor
  					 */
  					tupdesc = CreateTemplateTupleDesc(1, false);
  					TupleDescInitEntry(tupdesc,
  									   (AttrNumber) 1,
  									   "column",
--- 1349,1364 ----
  					td = DatumGetHeapTupleHeader(result);
  					tupdesc = lookup_rowtype_tupdesc(HeapTupleHeaderGetTypeId(td),
  											   HeapTupleHeaderGetTypMod(td));
  				}
  				else
  				{
  					/*
! 					 * Scalar type, so make a single-column
! 					 * descriptor. The caller expects the refcount on
! 					 * the TupleDesc to be non-zero, so bump it.
  					 */
  					tupdesc = CreateTemplateTupleDesc(1, false);
+ 					IncrTupleDescRefCount(tupdesc);
  					TupleDescInitEntry(tupdesc,
  									   (AttrNumber) 1,
  									   "column",
***************
*** 2754,2759 ****
--- 2762,2776 ----
  	return econtext->domainValue_datum;
  }
  
+ static void
+ CleanupFieldSelectCallback(Datum arg)
+ {
+ 	FieldSelectState *fstate = (FieldSelectState *) DatumGetPointer(arg);
+ 
+ 	if (fstate->argdesc)
+ 		DecrTupleDescRefCount(fstate->argdesc);
+ }
+ 
  /* ----------------------------------------------------------------
   *		ExecEvalFieldSelect
   *
***************
*** 2786,2807 ****
  	tupType = HeapTupleHeaderGetTypeId(tuple);
  	tupTypmod = HeapTupleHeaderGetTypMod(tuple);
  
  	/* Lookup tupdesc if first time through or if type changes */
  	tupDesc = fstate->argdesc;
  	if (tupDesc == NULL ||
  		tupType != tupDesc->tdtypeid ||
  		tupTypmod != tupDesc->tdtypmod)
  	{
! 		MemoryContext oldcontext;
  
  		tupDesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
- 		/* Copy the tupdesc into query storage for safety */
- 		oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
- 		tupDesc = CreateTupleDescCopy(tupDesc);
- 		if (fstate->argdesc)
- 			FreeTupleDesc(fstate->argdesc);
  		fstate->argdesc = tupDesc;
- 		MemoryContextSwitchTo(oldcontext);
  	}
  
  	/*
--- 2803,2827 ----
  	tupType = HeapTupleHeaderGetTypeId(tuple);
  	tupTypmod = HeapTupleHeaderGetTypMod(tuple);
  
+ 	/*
+ 	 * If this is the first time through, setup a callback to ensure
+ 	 * we release the reference count we hold on the TupleDesc
+ 	 */
+ 	if (fstate->argdesc == NULL)
+ 		RegisterExprContextCallback(econtext, CleanupFieldSelectCallback,
+ 									PointerGetDatum(fstate));
+ 
  	/* Lookup tupdesc if first time through or if type changes */
  	tupDesc = fstate->argdesc;
  	if (tupDesc == NULL ||
  		tupType != tupDesc->tdtypeid ||
  		tupTypmod != tupDesc->tdtypmod)
  	{
! 		if (fstate->argdesc)
! 			DecrTupleDescRefCount(fstate->argdesc);
  
  		tupDesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
  		fstate->argdesc = tupDesc;
  	}
  
  	/*
***************
*** 2821,2826 ****
--- 2841,2855 ----
  	return result;
  }
  
+ static void
+ CleanupFieldStoreCallback(Datum arg)
+ {
+ 	FieldStoreState *fstate = (FieldStoreState *) DatumGetPointer(arg);
+ 
+ 	if (fstate->argdesc)
+ 		DecrTupleDescRefCount(fstate->argdesc);
+ }
+ 
  /* ----------------------------------------------------------------
   *		ExecEvalFieldStore
   *
***************
*** 2849,2869 ****
  	if (isDone && *isDone == ExprEndResult)
  		return tupDatum;
  
  	/* Lookup tupdesc if first time through or if type changes */
  	tupDesc = fstate->argdesc;
  	if (tupDesc == NULL ||
  		fstore->resulttype != tupDesc->tdtypeid)
  	{
! 		MemoryContext oldcontext;
  
  		tupDesc = lookup_rowtype_tupdesc(fstore->resulttype, -1);
- 		/* Copy the tupdesc into query storage for safety */
- 		oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
- 		tupDesc = CreateTupleDescCopy(tupDesc);
- 		if (fstate->argdesc)
- 			FreeTupleDesc(fstate->argdesc);
  		fstate->argdesc = tupDesc;
- 		MemoryContextSwitchTo(oldcontext);
  	}
  
  	/* Allocate workspace */
--- 2878,2901 ----
  	if (isDone && *isDone == ExprEndResult)
  		return tupDatum;
  
+ 	/*
+ 	 * If this is the first time through, setup a callback to ensure
+ 	 * we release the reference count we hold on the TupleDesc
+ 	 */
+ 	if (fstate->argdesc == NULL)
+ 		RegisterExprContextCallback(econtext, CleanupFieldStoreCallback,
+ 									PointerGetDatum(fstate));
+ 
  	/* Lookup tupdesc if first time through or if type changes */
  	tupDesc = fstate->argdesc;
  	if (tupDesc == NULL ||
  		fstore->resulttype != tupDesc->tdtypeid)
  	{
! 		if (fstate->argdesc)
! 			DecrTupleDescRefCount(fstate->argdesc);
  
  		tupDesc = lookup_rowtype_tupdesc(fstore->resulttype, -1);
  		fstate->argdesc = tupDesc;
  	}
  
  	/* Allocate workspace */
***************
*** 3242,3252 ****
  
  				cstate->xprstate.evalfunc = (ExprStateEvalFunc) ExecEvalConvertRowtype;
  				cstate->arg = ExecInitExpr(convert->arg, parent);
! 				/* save copies of needed tuple descriptors */
  				cstate->indesc = lookup_rowtype_tupdesc(exprType((Node *) convert->arg), -1);
- 				cstate->indesc = CreateTupleDescCopy(cstate->indesc);
  				cstate->outdesc = lookup_rowtype_tupdesc(convert->resulttype, -1);
- 				cstate->outdesc = CreateTupleDescCopy(cstate->outdesc);
  				/* prepare map from old to new attribute numbers */
  				n = cstate->outdesc->natts;
  				cstate->attrMap = (AttrNumber *) palloc0(n * sizeof(AttrNumber));
--- 3274,3282 ----
  
  				cstate->xprstate.evalfunc = (ExprStateEvalFunc) ExecEvalConvertRowtype;
  				cstate->arg = ExecInitExpr(convert->arg, parent);
! 				/* lookup needed tuple descriptors */
  				cstate->indesc = lookup_rowtype_tupdesc(exprType((Node *) convert->arg), -1);
  				cstate->outdesc = lookup_rowtype_tupdesc(convert->resulttype, -1);
  				/* prepare map from old to new attribute numbers */
  				n = cstate->outdesc->natts;
  				cstate->attrMap = (AttrNumber *) palloc0(n * sizeof(AttrNumber));
***************
*** 3368,3373 ****
--- 3398,3404 ----
  					/* it's been cast to a named type, use that */
  					rstate->tupdesc = lookup_rowtype_tupdesc(rowexpr->row_typeid, -1);
  					rstate->tupdesc = CreateTupleDescCopy(rstate->tupdesc);
+ 					/* XXX: remove this copy? */
  				}
  				/* Set up evaluation, skipping any deleted columns */
  				Assert(list_length(rowexpr->args) <= rstate->tupdesc->natts);
============================================================
*** src/backend/executor/nodeFunctionscan.c	aaa361c427457b027312c88f8cbd9dff4713b4e1
--- src/backend/executor/nodeFunctionscan.c	276225550a66664d415b5d236e51f81e46ddfb57
***************
*** 80,86 ****
--- 80,90 ----
  		 * do it always.
  		 */
  		if (funcTupdesc)
+ 		{
  			tupledesc_match(node->tupdesc, funcTupdesc);
+ 			/* Decrement its refcount now that we're finished with it */
+ 			DecrTupleDescRefCount(funcTupdesc);
+ 		}
  	}
  
  	/*
============================================================
*** src/backend/optimizer/util/clauses.c	bbe724038d294e1730ac04e4ef5a4a445186a690
--- src/backend/optimizer/util/clauses.c	8bfea1ba4129ae9583ffd892674cbe892a5ebc9b
***************
*** 1289,1309 ****
  rowtype_field_matches(Oid rowtypeid, int fieldnum,
  					  Oid expectedtype, int32 expectedtypmod)
  {
  	TupleDesc	tupdesc;
- 	Form_pg_attribute attr;
  
  	/* No issue for RECORD, since there is no way to ALTER such a type */
  	if (rowtypeid == RECORDOID)
  		return true;
  	tupdesc = lookup_rowtype_tupdesc(rowtypeid, -1);
  	if (fieldnum <= 0 || fieldnum > tupdesc->natts)
! 		return false;
! 	attr = tupdesc->attrs[fieldnum - 1];
! 	if (attr->attisdropped ||
! 		attr->atttypid != expectedtype ||
! 		attr->atttypmod != expectedtypmod)
! 		return false;
! 	return true;
  }
  
  
--- 1289,1317 ----
  rowtype_field_matches(Oid rowtypeid, int fieldnum,
  					  Oid expectedtype, int32 expectedtypmod)
  {
+ 	bool		result;
  	TupleDesc	tupdesc;
  
  	/* No issue for RECORD, since there is no way to ALTER such a type */
  	if (rowtypeid == RECORDOID)
  		return true;
+ 
  	tupdesc = lookup_rowtype_tupdesc(rowtypeid, -1);
  	if (fieldnum <= 0 || fieldnum > tupdesc->natts)
! 		result = false;
! 	else
! 	{
! 		Form_pg_attribute attr = tupdesc->attrs[fieldnum - 1];
! 		if (attr->attisdropped ||
! 			attr->atttypid != expectedtype ||
! 			attr->atttypmod != expectedtypmod)
! 			result = false;
! 		else
! 			result = true;
! 	}
! 
! 	DecrTupleDescRefCount(tupdesc);
! 	return result;
  }
  
  
============================================================
*** src/backend/parser/parse_coerce.c	3737c5680e258c68490479e5f12fec046aaebf86
--- src/backend/parser/parse_coerce.c	6e158f35e8953bd62a64f145b041987ac203ef90
***************
*** 763,768 ****
--- 763,771 ----
  						format_type_be(targetTypeId)),
  				 errdetail("Input has too many columns.")));
  
+ 
+ 	DecrTupleDescRefCount(tupdesc);
+ 
  	rowexpr = makeNode(RowExpr);
  	rowexpr->args = newargs;
  	rowexpr->row_typeid = targetTypeId;
============================================================
*** src/backend/parser/parse_target.c	bc7dd09c579281b54dd356e999c3b17d8fa23310
--- src/backend/parser/parse_target.c	fa7ebfe84615c82aabd457e20898bdec56c9b036
***************
*** 803,812 ****
  
  	/*
  	 * Verify it's a composite type, and get the tupdesc.  We use
! 	 * get_expr_result_type() because that can handle references to functions
! 	 * returning anonymous record types.  If that fails, use
! 	 * lookup_rowtype_tupdesc(), which will almost certainly fail as well, but
! 	 * it will give an appropriate error message.
  	 *
  	 * If it's a Var of type RECORD, we have to work even harder: we have to
  	 * find what the Var refers to, and pass that to get_expr_result_type.
--- 803,812 ----
  
  	/*
  	 * Verify it's a composite type, and get the tupdesc.  We use
! 	 * get_expr_result_type() because that can handle references to
! 	 * functions returning anonymous record types.  If that fails, use
! 	 * lookup_rowtype_tupdesc_copy(), which will almost certainly fail
! 	 * as well, but it will give an appropriate error message.
  	 *
  	 * If it's a Var of type RECORD, we have to work even harder: we have to
  	 * find what the Var refers to, and pass that to get_expr_result_type.
***************
*** 816,822 ****
  		((Var *) expr)->vartype == RECORDOID)
  		tupleDesc = expandRecordVariable(pstate, (Var *) expr, 0);
  	else if (get_expr_result_type(expr, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
! 		tupleDesc = lookup_rowtype_tupdesc(exprType(expr), exprTypmod(expr));
  	Assert(tupleDesc);
  
  	/* Generate a list of references to the individual fields */
--- 816,823 ----
  		((Var *) expr)->vartype == RECORDOID)
  		tupleDesc = expandRecordVariable(pstate, (Var *) expr, 0);
  	else if (get_expr_result_type(expr, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
! 		tupleDesc = lookup_rowtype_tupdesc_copy(exprType(expr),
! 												exprTypmod(expr));
  	Assert(tupleDesc);
  
  	/* Generate a list of references to the individual fields */
***************
*** 993,999 ****
  	 * appropriate error message while failing.
  	 */
  	if (get_expr_result_type(expr, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
! 		tupleDesc = lookup_rowtype_tupdesc(exprType(expr), exprTypmod(expr));
  
  	return tupleDesc;
  }
--- 994,1001 ----
  	 * appropriate error message while failing.
  	 */
  	if (get_expr_result_type(expr, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
! 		tupleDesc = lookup_rowtype_tupdesc_copy(exprType(expr),
! 												exprTypmod(expr));
  
  	return tupleDesc;
  }
============================================================
*** src/backend/utils/adt/rowtypes.c	07d7136a68bc720c43b726e3c9cf9e85fc09b807
--- src/backend/utils/adt/rowtypes.c	28c870e40e6df794c511938063b11c8826b021b9
***************
*** 257,262 ****
--- 257,263 ----
  	result = (HeapTupleHeader) palloc(tuple->t_len);
  	memcpy(result, tuple->t_data, tuple->t_len);
  
+ 	DecrTupleDescRefCount(tupdesc);
  	heap_freetuple(tuple);
  	pfree(buf.data);
  	pfree(values);
***************
*** 407,412 ****
--- 408,414 ----
  
  	appendStringInfoChar(&buf, ')');
  
+ 	DecrTupleDescRefCount(tupdesc);
  	pfree(values);
  	pfree(nulls);
  
***************
*** 594,599 ****
--- 596,602 ----
  	result = (HeapTupleHeader) palloc(tuple->t_len);
  	memcpy(result, tuple->t_data, tuple->t_len);
  
+ 	DecrTupleDescRefCount(tupdesc);
  	heap_freetuple(tuple);
  	pfree(values);
  	pfree(nulls);
***************
*** 722,727 ****
--- 725,731 ----
  		pfree(outputbytes);
  	}
  
+ 	DecrTupleDescRefCount(tupdesc);
  	pfree(values);
  	pfree(nulls);
  
============================================================
*** src/backend/utils/adt/ruleutils.c	ecb75470bb827232a0cdeeee9ae3ec4f6d511782
--- src/backend/utils/adt/ruleutils.c	b67411eea2fd387bbc9881125a81d9fa4efd9271
***************
*** 2689,2700 ****
  
  	/*
  	 * We now have an expression we can't expand any more, so see if
! 	 * get_expr_result_type() can do anything with it.	If not, pass to
! 	 * lookup_rowtype_tupdesc() which will probably fail, but will give an
! 	 * appropriate error message while failing.
  	 */
  	if (get_expr_result_type(expr, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
! 		tupleDesc = lookup_rowtype_tupdesc(exprType(expr), exprTypmod(expr));
  
  	/* Got the tupdesc, so we can extract the field name */
  	Assert(fieldno >= 1 && fieldno <= tupleDesc->natts);
--- 2689,2701 ----
  
  	/*
  	 * We now have an expression we can't expand any more, so see if
! 	 * get_expr_result_type() can do anything with it. If not, pass to
! 	 * lookup_rowtype_tupdesc_copy() which will probably fail, but
! 	 * will give an appropriate error message while failing.
  	 */
  	if (get_expr_result_type(expr, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
! 		tupleDesc = lookup_rowtype_tupdesc_copy(exprType(expr),
! 												exprTypmod(expr));
  
  	/* Got the tupdesc, so we can extract the field name */
  	Assert(fieldno >= 1 && fieldno <= tupleDesc->natts);
***************
*** 3326,3333 ****
  					TupleDesc	tupdesc;
  
  					if (get_expr_result_type(arg, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
! 						tupdesc = lookup_rowtype_tupdesc(exprType(arg),
! 														 exprTypmod(arg));
  					Assert(tupdesc);
  					/* Got the tupdesc, so we can extract the field name */
  					Assert(fno >= 1 && fno <= tupdesc->natts);
--- 3327,3334 ----
  					TupleDesc	tupdesc;
  
  					if (get_expr_result_type(arg, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
! 						tupdesc = lookup_rowtype_tupdesc_copy(exprType(arg),
! 															  exprTypmod(arg));
  					Assert(tupdesc);
  					/* Got the tupdesc, so we can extract the field name */
  					Assert(fno >= 1 && fno <= tupdesc->natts);
***************
*** 3528,3533 ****
--- 3529,3536 ----
  						}
  						i++;
  					}
+ 
+ 					DecrTupleDescRefCount(tupdesc);
  				}
  				appendStringInfo(buf, ")");
  				if (rowexpr->row_format == COERCE_EXPLICIT_CAST)
============================================================
*** src/backend/utils/cache/relcache.c	6d26b0a1bc7e27a51e8c1c49ebdec70959a5c322
--- src/backend/utils/cache/relcache.c	6b4dc21e525ee187e745554027ee6265a50de5dc
***************
*** 1571,1577 ****
  	{
  		/* ok to zap remaining substructure */
  		flush_rowtype_cache(old_reltype);
- 		FreeTupleDesc(relation->rd_att);
  		if (relation->rd_rulescxt)
  			MemoryContextDelete(relation->rd_rulescxt);
  		pfree(relation);
--- 1571,1576 ----
***************
*** 1598,1604 ****
  		{
  			/* Should only get here if relation was deleted */
  			flush_rowtype_cache(old_reltype);
- 			FreeTupleDesc(old_att);
  			if (old_rulescxt)
  				MemoryContextDelete(old_rulescxt);
  			pfree(relation);
--- 1597,1602 ----
***************
*** 1615,1621 ****
  		else
  		{
  			flush_rowtype_cache(old_reltype);
- 			FreeTupleDesc(old_att);
  		}
  		if (equalRuleLocks(old_rules, relation->rd_rules))
  		{
--- 1613,1618 ----
============================================================
*** src/backend/utils/cache/typcache.c	85eb6ae0e0048cd300ef5142aa3926b2f5907b2f
--- src/backend/utils/cache/typcache.c	0473ff9b4edeb8d0416438f8ceea0a71d235d02f
***************
*** 373,382 ****
   * lookup_rowtype_tupdesc
   *
   * Given a typeid/typmod that should describe a known composite type,
!  * return the tuple descriptor for the type.  Will ereport on failure.
!  *
!  * Note: returned TupleDesc points to cached copy; caller must copy it
!  * if intending to scribble on it or keep a reference for a long time.
   */
  TupleDesc
  lookup_rowtype_tupdesc(Oid type_id, int32 typmod)
--- 373,381 ----
   * lookup_rowtype_tupdesc
   *
   * Given a typeid/typmod that should describe a known composite type,
!  * return the tuple descriptor for the type.  Will ereport on
!  * failure. The returned TupleDesc has non-zero reference count; the
!  * caller should decrement its reference count when finished with it.
   */
  TupleDesc
  lookup_rowtype_tupdesc(Oid type_id, int32 typmod)
***************
*** 385,390 ****
--- 384,408 ----
  }
  
  /*
+  * lookup_rowtype_tupdesc_copy
+  *
+  * Like lookup_rowtype_tupdesc(), but the returned TupleDesc has been
+  * copied into the CurrentMemoryContext and has a zero reference count.
+  */
+ TupleDesc
+ lookup_rowtype_tupdesc_copy(Oid type_id, int32 typmod)
+ {
+ 	TupleDesc tmp;
+ 	TupleDesc result;
+ 
+ 	tmp = lookup_rowtype_tupdesc_noerror(type_id, typmod, false);
+ 	result = CreateTupleDescCopyConstr(tmp);
+ 	DecrTupleDescRefCount(tmp);
+ 
+ 	return result;
+ }
+ 
+ /*
   * lookup_rowtype_tupdesc_noerror
   *
   * As above, but if the type is not a known composite type and noError
***************
*** 394,399 ****
--- 412,419 ----
  TupleDesc
  lookup_rowtype_tupdesc_noerror(Oid type_id, int32 typmod, bool noError)
  {
+ 	TupleDesc result;
+ 
  	if (type_id != RECORDOID)
  	{
  		/*
***************
*** 407,413 ****
  					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
  					 errmsg("type %s is not composite",
  							format_type_be(type_id))));
! 		return typentry->tupDesc;
  	}
  	else
  	{
--- 427,433 ----
  					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
  					 errmsg("type %s is not composite",
  							format_type_be(type_id))));
! 		result = typentry->tupDesc;
  	}
  	else
  	{
***************
*** 422,429 ****
  						 errmsg("record type has not been registered")));
  			return NULL;
  		}
! 		return RecordCacheArray[typmod];
  	}
  }
  
  
--- 442,453 ----
  						 errmsg("record type has not been registered")));
  			return NULL;
  		}
! 		result = RecordCacheArray[typmod];
  	}
+ 
+ 	if (result)
+ 		IncrTupleDescRefCount(result);
+ 	return result;
  }
  
  
***************
*** 534,539 ****
--- 558,564 ----
  flush_rowtype_cache(Oid type_id)
  {
  	TypeCacheEntry *typentry;
+ 	TupleDesc		tupdesc;
  
  	if (TypeCacheHash == NULL)
  		return;					/* no table, so certainly no entry */
***************
*** 544,548 ****
--- 569,587 ----
  	if (typentry == NULL)
  		return;					/* no matching entry */
  
+ 	/*
+ 	 * Clear out the tuple descriptor. If there are no references to
+ 	 * the TupleDesc, we can just free it now. Otherwise, mark it dead
+ 	 * -- it will be freed when the reference count reaches zero.
+ 	 */
+ 	tupdesc = typentry->tupDesc;
  	typentry->tupDesc = NULL;
+ 
+ 	if (tupdesc)
+ 	{
+ 		if (tupdesc->refcount == 0)
+ 			FreeTupleDesc(tupdesc);
+ 		else
+ 			tupdesc->dead = true;
+ 	}
  }
============================================================
*** src/backend/utils/fmgr/funcapi.c	1e22765271d66af893b77287b1907c075fe5a866
--- src/backend/utils/fmgr/funcapi.c	114647ae1f1c708a71d206de6c7a679497bee208
***************
*** 180,192 ****
  
  /*
   * get_call_result_type
!  *		Given a function's call info record, determine the kind of datatype
!  *		it is supposed to return.  If resultTypeId isn't NULL, *resultTypeId
!  *		receives the actual datatype OID (this is mainly useful for scalar
!  *		result types).	If resultTupleDesc isn't NULL, *resultTupleDesc
!  *		receives a pointer to a TupleDesc when the result is of a composite
!  *		type, or NULL when it's a scalar result.  NB: the tupledesc should
!  *		be copied if it is to be accessed over a long period.
   *
   * One hard case that this handles is resolution of actual rowtypes for
   * functions returning RECORD (from either the function's OUT parameter
--- 180,192 ----
  
  /*
   * get_call_result_type
!  *		Given a function's call info record, determine the kind of
!  *		datatype it is supposed to return.  If resultTypeId isn't
!  *		NULL, *resultTypeId receives the actual datatype OID (this is
!  *		mainly useful for scalar result types). If resultTupleDesc
!  *		isn't NULL, *resultTupleDesc receives a pointer to a TupleDesc
!  *		when the result is of a composite type, or NULL when it's a
!  *		scalar result.
   *
   * One hard case that this handles is resolution of actual rowtypes for
   * functions returning RECORD (from either the function's OUT parameter
***************
*** 246,252 ****
  			*resultTupleDesc = NULL;
  		result = get_type_func_class(typid);
  		if (result == TYPEFUNC_COMPOSITE && resultTupleDesc)
! 			*resultTupleDesc = lookup_rowtype_tupdesc(typid, -1);
  	}
  
  	return result;
--- 246,252 ----
  			*resultTupleDesc = NULL;
  		result = get_type_func_class(typid);
  		if (result == TYPEFUNC_COMPOSITE && resultTupleDesc)
! 			*resultTupleDesc = lookup_rowtype_tupdesc_copy(typid, -1);
  	}
  
  	return result;
***************
*** 363,369 ****
  	{
  		case TYPEFUNC_COMPOSITE:
  			if (resultTupleDesc)
! 				*resultTupleDesc = lookup_rowtype_tupdesc(rettype, -1);
  			/* Named composite types can't have any polymorphic columns */
  			break;
  		case TYPEFUNC_SCALAR:
--- 363,369 ----
  	{
  		case TYPEFUNC_COMPOSITE:
  			if (resultTupleDesc)
! 				*resultTupleDesc = lookup_rowtype_tupdesc_copy(rettype, -1);
  			/* Named composite types can't have any polymorphic columns */
  			break;
  		case TYPEFUNC_SCALAR:
***************
*** 1053,1059 ****
  	if (functypclass == TYPEFUNC_COMPOSITE)
  	{
  		/* Composite data type, e.g. a table's row type */
! 		tupdesc = CreateTupleDescCopy(lookup_rowtype_tupdesc(typeoid, -1));
  
  		if (colaliases != NIL)
  		{
--- 1053,1059 ----
  	if (functypclass == TYPEFUNC_COMPOSITE)
  	{
  		/* Composite data type, e.g. a table's row type */
! 		tupdesc = lookup_rowtype_tupdesc_copy(typeoid, -1);
  
  		if (colaliases != NIL)
  		{
============================================================
*** src/backend/utils/resowner/resowner.c	8e55e3ae813bd6f162ef5de540740cedbe7d56a3
--- src/backend/utils/resowner/resowner.c	bdb3c3b7e32e7103828eb66dd8b8c5d4edad4368
***************
*** 39,50 ****
  	ResourceOwner nextchild;	/* next child of same parent */
  	const char *name;			/* name (just for debugging) */
  
! 	/* We have built-in support for remembering owned buffers */
  	int			nbuffers;		/* number of owned buffer pins */
  	Buffer	   *buffers;		/* dynamically allocated array */
  	int			maxbuffers;		/* currently allocated array size */
  
- 	/* We have built-in support for remembering catcache references */
  	int			ncatrefs;		/* number of owned catcache pins */
  	HeapTuple  *catrefs;		/* dynamically allocated array */
  	int			maxcatrefs;		/* currently allocated array size */
--- 39,54 ----
  	ResourceOwner nextchild;	/* next child of same parent */
  	const char *name;			/* name (just for debugging) */
  
! 	/*
! 	 * We have built-in support for remembering buffer pins, catcache
! 	 * references, catcache-list pins, relcache references, and
! 	 * tupledescs.
! 	 */
! 
  	int			nbuffers;		/* number of owned buffer pins */
  	Buffer	   *buffers;		/* dynamically allocated array */
  	int			maxbuffers;		/* currently allocated array size */
  
  	int			ncatrefs;		/* number of owned catcache pins */
  	HeapTuple  *catrefs;		/* dynamically allocated array */
  	int			maxcatrefs;		/* currently allocated array size */
***************
*** 53,62 ****
  	CatCList  **catlistrefs;	/* dynamically allocated array */
  	int			maxcatlistrefs; /* currently allocated array size */
  
- 	/* We have built-in support for remembering relcache references */
  	int			nrelrefs;		/* number of owned relcache pins */
  	Relation   *relrefs;		/* dynamically allocated array */
  	int			maxrelrefs;		/* currently allocated array size */
  } ResourceOwnerData;
  
  
--- 57,69 ----
  	CatCList  **catlistrefs;	/* dynamically allocated array */
  	int			maxcatlistrefs; /* currently allocated array size */
  
  	int			nrelrefs;		/* number of owned relcache pins */
  	Relation   *relrefs;		/* dynamically allocated array */
  	int			maxrelrefs;		/* currently allocated array size */
+ 
+ 	int			ntupdescs;		/* number of owned tupledescs */
+ 	TupleDesc  *tupdescs;		/* dynamically allocated array */
+ 	int			maxtupdescs;	/* currently allocated array size */
  } ResourceOwnerData;
  
  
***************
*** 87,92 ****
--- 94,100 ----
  							 bool isCommit,
  							 bool isTopLevel);
  static void PrintRelCacheLeakWarning(Relation rel);
+ static void PrintTupleDescLeakWarning(TupleDesc tupdesc);
  
  
  /*****************************************************************************
***************
*** 276,281 ****
--- 284,297 ----
  			ReleaseCatCacheList(owner->catlistrefs[owner->ncatlistrefs - 1]);
  		}
  
+ 		/* Clean up tupledescs */
+ 		while (owner->ntupdescs > 0)
+ 		{
+ 			if (isCommit)
+ 				PrintTupleDescLeakWarning(owner->tupdescs[owner->ntupdescs - 1]);
+ 			DecrTupleDescRefCount(owner->tupdescs[owner->ntupdescs - 1]);
+ 		}
+ 
  		/* Clean up index scans too */
  		ReleaseResources_gist();
  		ReleaseResources_hash();
***************
*** 305,310 ****
--- 321,327 ----
  	Assert(owner->ncatrefs == 0);
  	Assert(owner->ncatlistrefs == 0);
  	Assert(owner->nrelrefs == 0);
+ 	Assert(owner->ntupdescs == 0);
  
  	/*
  	 * Delete children.  The recursive call will delink the child from me, so
***************
*** 329,334 ****
--- 346,353 ----
  		pfree(owner->catlistrefs);
  	if (owner->relrefs)
  		pfree(owner->relrefs);
+ 	if (owner->tupdescs)
+ 		pfree(owner->tupdescs);
  
  	pfree(owner);
  }
***************
*** 735,745 ****
  }
  
  /*
!  * Debugging subroutine
   */
  static void
  PrintRelCacheLeakWarning(Relation rel)
  {
  	elog(WARNING, "relcache reference leak: relation \"%s\" not closed",
  		 RelationGetRelationName(rel));
  }
--- 754,842 ----
  }
  
  /*
!  * Make sure there is room for at least one more entry in a ResourceOwner's
!  * TupleDesc array.
!  *
!  * This is separate from actually inserting an entry because if we run out
!  * of memory, it's critical to do so *before* acquiring the resource.
   */
+ void
+ ResourceOwnerEnlargeTupleDescs(ResourceOwner owner)
+ {
+ 	int			newmax;
+ 
+ 	if (owner->ntupdescs < owner->maxtupdescs)
+ 		return;					/* nothing to do */
+ 	if (owner->tupdescs == NULL)
+ 	{
+ 		newmax = 16;
+ 		owner->tupdescs = (TupleDesc *)
+ 			MemoryContextAlloc(TopMemoryContext, newmax * sizeof(TupleDesc));
+ 		owner->maxtupdescs = newmax;
+ 	}
+ 	else
+ 	{
+ 		newmax = owner->maxtupdescs * 2;
+ 		owner->tupdescs = (TupleDesc *)
+ 			repalloc(owner->tupdescs, newmax * sizeof(TupleDesc));
+ 		owner->maxtupdescs = newmax;
+ 	}
+ }
+ 
+ /*
+  * Remember that a TupleDesc is owned by a ResourceOwner
+  *
+  * Caller must have previously done ResourceOwnerEnlargeTupleDescs()
+  */
+ void
+ ResourceOwnerRememberTupleDesc(ResourceOwner owner, TupleDesc tupdesc)
+ {
+ 	Assert(owner->ntupdescs < owner->maxtupdescs);
+ 	owner->tupdescs[owner->ntupdescs] = tupdesc;
+ 	owner->ntupdescs++;
+ }
+ 
+ /*
+  * Forget that a TupleDesc is owned by a ResourceOwner
+  */
+ void
+ ResourceOwnerForgetTupleDesc(ResourceOwner owner, TupleDesc tupdesc)
+ {
+ 	TupleDesc  *tupdescs = owner->tupdescs;
+ 	int			nt1 = owner->ntupdescs - 1;
+ 	int			i;
+ 
+ 	for (i = nt1; i >= 0; i--)
+ 	{
+ 		if (tupdescs[i] == tupdesc)
+ 		{
+ 			while (i < nt1)
+ 			{
+ 				tupdescs[i] = tupdescs[i + 1];
+ 				i++;
+ 			}
+ 			owner->ntupdescs = nt1;
+ 			return;
+ 		}
+ 	}
+ 	elog(ERROR, "TupleDesc %p is not owned by resource owner %s",
+ 		 tupdesc, owner->name);
+ }
+ 
+ 
+ /*
+  * Debugging subroutines
+  */
  static void
  PrintRelCacheLeakWarning(Relation rel)
  {
  	elog(WARNING, "relcache reference leak: relation \"%s\" not closed",
  		 RelationGetRelationName(rel));
  }
+ 
+ static void
+ PrintTupleDescLeakWarning(TupleDesc tupdesc)
+ {
+ 	elog(WARNING, "TupleDesc reference leak: TupleDesc %p is still referenced",
+ 		 tupdesc);
+ }
============================================================
*** src/include/access/tupdesc.h	838f14152479f66e6f98d9055886ab655da40f70
--- src/include/access/tupdesc.h	98872368403e528c3d61051a02729d2bbd876b81
***************
*** 57,62 ****
--- 57,65 ----
   * tdtypeid is RECORDOID, and tdtypmod can be either -1 for a fully anonymous
   * row type, or a value >= 0 to allow the rowtype to be looked up in the
   * typcache.c type cache.
+  *
+  * If a TupleDesc has non-zero refcount, we assume it should be
+  * automatically freed when the refcount reaches zero.
   */
  typedef struct tupleDesc
  {
***************
*** 67,72 ****
--- 70,77 ----
  	Oid			tdtypeid;		/* composite type ID for tuple type */
  	int32		tdtypmod;		/* typmod for tuple type */
  	bool		tdhasoid;		/* tuple has oid attribute in its header */
+ 	int			refcount;		/* if > 0, the # of references */
+ 	bool		dead;
  }	*TupleDesc;
  
  
***************
*** 81,86 ****
--- 86,94 ----
  
  extern void FreeTupleDesc(TupleDesc tupdesc);
  
+ extern void IncrTupleDescRefCount(TupleDesc tupdesc);
+ extern void DecrTupleDescRefCount(TupleDesc tupdesc);
+ 
  extern bool equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2);
  
  extern void TupleDescInitEntry(TupleDesc desc,
============================================================
*** src/include/utils/resowner.h	ba8a6dae7162d1f5026daf7a9552db29f9615e91
--- src/include/utils/resowner.h	7322c8b8cd6206c6090099eb837516a866049d50
***************
*** 19,24 ****
--- 19,25 ----
  #ifndef RESOWNER_H
  #define RESOWNER_H
  
+ #include "access/tupdesc.h"
  #include "storage/buf.h"
  #include "utils/catcache.h"
  #include "utils/rel.h"
***************
*** 107,110 ****
--- 108,118 ----
  extern void ResourceOwnerForgetRelationRef(ResourceOwner owner,
  							   Relation rel);
  
+ /* support for tupledesc refcount management */
+ extern void ResourceOwnerEnlargeTupleDescs(ResourceOwner owner);
+ extern void ResourceOwnerRememberTupleDesc(ResourceOwner owner,
+ 										   TupleDesc tupdesc);
+ extern void ResourceOwnerForgetTupleDesc(ResourceOwner owner,
+ 										 TupleDesc tupdesc);
+ 
  #endif   /* RESOWNER_H */
============================================================
*** src/include/utils/typcache.h	60ff2e4a19fc06afb5c41d440c20aff03efd79ae
--- src/include/utils/typcache.h	9ad569c8067bc41063bac65866783065a9a16cf7
***************
*** 75,81 ****
  extern TypeCacheEntry *lookup_type_cache(Oid type_id, int flags);
  
  extern TupleDesc lookup_rowtype_tupdesc(Oid type_id, int32 typmod);
! 
  extern TupleDesc lookup_rowtype_tupdesc_noerror(Oid type_id, int32 typmod,
  							   bool noError);
  
--- 75,81 ----
  extern TypeCacheEntry *lookup_type_cache(Oid type_id, int flags);
  
  extern TupleDesc lookup_rowtype_tupdesc(Oid type_id, int32 typmod);
! extern TupleDesc lookup_rowtype_tupdesc_copy(Oid type_id, int32 typmod);
  extern TupleDesc lookup_rowtype_tupdesc_noerror(Oid type_id, int32 typmod,
  							   bool noError);
  
============================================================
*** src/pl/plperl/plperl.c	a4ba9c99e0d5d19ee7fcf9fb6762594a43fd2fae
--- src/pl/plperl/plperl.c	298226f8719779be83482cbe35d3037759bd4092
***************
*** 828,833 ****
--- 828,834 ----
  
  			hashref = plperl_hash_from_tuple(&tmptup, tupdesc);
  			XPUSHs(sv_2mortal(hashref));
+ 			DecrTupleDescRefCount(tupdesc);
  		}
  		else
  		{
============================================================
*** src/pl/plpgsql/src/pl_exec.c	0f3dff3cfb05fda66b508752d7583ca187aa567a
--- src/pl/plpgsql/src/pl_exec.c	6a0d816314313208c62d62c31495506e16b37e12
***************
*** 259,264 ****
--- 259,265 ----
  						tmptup.t_tableOid = InvalidOid;
  						tmptup.t_data = td;
  						exec_move_row(&estate, NULL, row, &tmptup, tupdesc);
+ 						DecrTupleDescRefCount(tupdesc);
  					}
  					else
  					{
***************
*** 3114,3119 ****
--- 3115,3121 ----
  					tmptup.t_tableOid = InvalidOid;
  					tmptup.t_data = td;
  					exec_move_row(estate, NULL, row, &tmptup, tupdesc);
+ 					DecrTupleDescRefCount(tupdesc);
  				}
  				break;
  			}
***************
*** 3156,3161 ****
--- 3158,3164 ----
  					tmptup.t_tableOid = InvalidOid;
  					tmptup.t_data = td;
  					exec_move_row(estate, rec, NULL, &tmptup, tupdesc);
+ 					DecrTupleDescRefCount(tupdesc);
  				}
  				break;
  			}
============================================================
*** src/pl/plpython/plpython.c	103ec857e125a1ca082966ee70f7a2d7dbc19928
--- src/pl/plpython/plpython.c	a8a59377ad8334e910cdaae7845d5a2a20796714
***************
*** 864,869 ****
--- 864,870 ----
  					tmptup.t_data = td;
  
  					arg = PLyDict_FromTuple(&(proc->args[i]), &tmptup, tupdesc);
+ 					DecrTupleDescRefCount(tupdesc);
  				}
  			}
  			else
============================================================
*** src/pl/tcl/pltcl.c	e9000795907af26a6a066ad43a6b7749423df32b
--- src/pl/tcl/pltcl.c	7e30fd2195d7a7a99eee6a35cf7eaec5877c6035
***************
*** 539,544 ****
--- 539,545 ----
  					pltcl_build_tuple_argument(&tmptup, tupdesc, &list_tmp);
  					Tcl_DStringAppendElement(&tcl_cmd,
  											 Tcl_DStringValue(&list_tmp));
+ 					DecrTupleDescRefCount(tupdesc);
  				}
  			}
  			else
---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
       subscribe-nomail command to [EMAIL PROTECTED] so that your
       message can get through to the mailing list cleanly

Reply via email to