Re: Life cycles of tuple descriptors

I wrote:
> Chapman Flack <chap@anastigmatix.net> writes:
>> Oh, hmm, maybe one thing in that API comment ought to be changed. It says
>> I must call ReleaseTupleDesc *or* DecrTupleDescRefCount. Maybe that dates
>> from before the shared registry? ReleaseTupleDesc is safe, but anybody who
>> uses DecrTupleDescRefCount on a lookup_rowtype_tupdesc result could be
>> in for an assertion failure if a non-refcounted tupdesc is returned.

> Yeah, I was just wondering the same.  I think DecrTupleDescRefCount
> is safe if you know you are looking up a named composite type, but
> maybe that's still too much familiarity with typcache innards.

Here's a draft patch for this.  There are several places that are
directly using DecrTupleDescRefCount after lookup_rowtype_tupdesc
or equivalent, which'd now be forbidden.  I think they are all safe
given the assumption that the typcache's tupdescs for named composites
are refcounted.  (The calls in expandedrecord.c could be working
with RECORD, but those code paths just checked that the tupdesc
is refcounted.)  So there's no actual bug here, and no reason to
back-patch, but this seems like a good idea to decouple callers
a bit more from typcache's internal logic.  None of these call
sites are so performance-critical that one extra test will hurt.

            regards, tom lane

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)