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