From bbc227e951ecc59a29205be4952a623e7d1dd534 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 15 Dec 2021 18:58:20 -0500 Subject: [PATCH] Always use ReleaseTupleDesc after lookup_rowtype_tupdesc et al. The API spec for lookup_rowtype_tupdesc previously said you could use either ReleaseTupleDesc or DecrTupleDescRefCount. However, the latter choice means the caller must be certain that the returned tupdesc is refcounted. I don't recall right now whether that was always true when this spec was written, but it's certainly not always true since we introduced shared record typcaches for parallel workers. That means that callers using DecrTupleDescRefCount are dependent on typcache behavior details that they probably shouldn't be. Hence, change the API spec to say that you must call ReleaseTupleDesc, and fix the half-dozen callers that weren't. AFAICT this is just future-proofing, there's no live bug here. So no back-patch. Per gripe from Chapman Flack. Discussion: https://postgr.es/m/61B901A4.1050808@anastigmatix.net --- src/backend/commands/tablecmds.c | 2 +- src/backend/executor/execExpr.c | 2 +- src/backend/parser/parse_utilcmd.c | 2 +- src/backend/utils/adt/expandedrecord.c | 4 ++-- src/backend/utils/cache/typcache.c | 7 +++++-- 5 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 47b29001d5..bf42587e38 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -15401,7 +15401,7 @@ ATExecAddOf(Relation rel, const TypeName *ofTypename, LOCKMODE lockmode) errmsg("table \"%s\" has different type for column \"%s\"", RelationGetRelationName(rel), type_attname))); } - DecrTupleDescRefCount(typeTupleDesc); + ReleaseTupleDesc(typeTupleDesc); /* Any remaining columns at the end of the table had better be dropped. */ for (; table_attno <= tableTupleDesc->natts; table_attno++) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 892b4e17e0..7d343f0678 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1465,7 +1465,7 @@ ExecInitExprRec(Expr *node, ExprState *state, /* find out the number of columns in the composite type */ tupDesc = lookup_rowtype_tupdesc(fstore->resulttype, -1); ncolumns = tupDesc->natts; - DecrTupleDescRefCount(tupDesc); + ReleaseTupleDesc(tupDesc); /* create workspace for column values */ values = (Datum *) palloc(sizeof(Datum) * ncolumns); diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 313d7b6ff0..2d857a301b 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -1484,7 +1484,7 @@ transformOfType(CreateStmtContext *cxt, TypeName *ofTypename) n->location = -1; cxt->columns = lappend(cxt->columns, n); } - DecrTupleDescRefCount(tupdesc); + ReleaseTupleDesc(tupdesc); ReleaseSysCache(tuple); } diff --git a/src/backend/utils/adt/expandedrecord.c b/src/backend/utils/adt/expandedrecord.c index e19491ecf7..38d5384c00 100644 --- a/src/backend/utils/adt/expandedrecord.c +++ b/src/backend/utils/adt/expandedrecord.c @@ -171,7 +171,7 @@ make_expanded_record_from_typeid(Oid type_id, int32 typmod, /* If we called lookup_rowtype_tupdesc, release the pin it took */ if (type_id == RECORDOID) - DecrTupleDescRefCount(tupdesc); + ReleaseTupleDesc(tupdesc); } else { @@ -854,7 +854,7 @@ expanded_record_fetch_tupdesc(ExpandedRecordHeader *erh) tupdesc->tdrefcount++; /* Release the pin lookup_rowtype_tupdesc acquired */ - DecrTupleDescRefCount(tupdesc); + ReleaseTupleDesc(tupdesc); } else { diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c index 70e5c51297..d140ef6655 100644 --- a/src/backend/utils/cache/typcache.c +++ b/src/backend/utils/cache/typcache.c @@ -1820,8 +1820,11 @@ lookup_rowtype_tupdesc_internal(Oid type_id, int32 typmod, bool noError) * for example from record_in().) * * Note: on success, we increment the refcount of the returned TupleDesc, - * and log the reference in CurrentResourceOwner. Caller should call - * ReleaseTupleDesc or DecrTupleDescRefCount when done using the tupdesc. + * and log the reference in CurrentResourceOwner. Caller must call + * ReleaseTupleDesc when done using the tupdesc. (There are some + * cases in which the returned tupdesc is not refcounted, in which + * case PinTupleDesc/ReleaseTupleDesc are no-ops; but in these cases + * the tupdesc is guaranteed to live till process exit.) */ TupleDesc lookup_rowtype_tupdesc(Oid type_id, int32 typmod)