Fix problems with cached tuple descriptors disappearing while still in use

by creating a reference-count mechanism, similar to what we did a long time
ago for catcache entries.  The back branches have an ugly solution involving
lots of extra copies, but this way is more efficient.  Reference counting is
only applied to tupdescs that are actually in caches --- there seems no need
to use it for tupdescs that are generated in the executor, since they'll go
away during plan shutdown by virtue of being in the per-query memory context.
Neil Conway and Tom Lane

2 files changed, 104 insertions, 31 deletions

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 9d4ffa4b148..82de42020e8 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.241 2006/05/06 15:51:07 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.242 2006/06/16 18:42:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -313,6 +313,8 @@ AllocateRelationDesc(Relation relation, Form_pg_class relp)
 	/* and allocate attribute tuple form storage */
 	relation->rd_att = CreateTemplateTupleDesc(relationForm->relnatts,
 											   relationForm->relhasoids);
+	/* which we mark as a reference-counted tupdesc */
+	relation->rd_att->tdrefcount = 1;
 
 	MemoryContextSwitchTo(oldcxt);
 
@@ -1234,6 +1236,8 @@ formrdesc(const char *relationName, Oid relationReltype,
 	 * defined by macros in src/include/catalog/ headers.
 	 */
 	relation->rd_att = CreateTemplateTupleDesc(natts, hasoids);
+	relation->rd_att->tdrefcount = 1;	/* mark as refcounted */
+
 	relation->rd_att->tdtypeid = relationReltype;
 	relation->rd_att->tdtypmod = -1;	/* unnecessary, but... */
 
@@ -1591,7 +1595,10 @@ RelationClearRelation(Relation relation, bool rebuild)
 	{
 		/* ok to zap remaining substructure */
 		flush_rowtype_cache(old_reltype);
-		FreeTupleDesc(relation->rd_att);
+		/* can't use DecrTupleDescRefCount here */
+		Assert(relation->rd_att->tdrefcount > 0);
+		if (--relation->rd_att->tdrefcount == 0)
+			FreeTupleDesc(relation->rd_att);
 		if (relation->rd_rulescxt)
 			MemoryContextDelete(relation->rd_rulescxt);
 		pfree(relation);
@@ -1601,7 +1608,10 @@ RelationClearRelation(Relation relation, bool rebuild)
 		/*
 		 * When rebuilding an open relcache entry, must preserve ref count and
 		 * rd_createSubid state.  Also attempt to preserve the tupledesc and
-		 * rewrite-rule substructures in place.
+		 * rewrite-rule substructures in place.  (Note: the refcount mechanism
+		 * for tupledescs may eventually ensure that we don't really need to
+		 * preserve the tupledesc in-place, but for now there are still a lot
+		 * of places that assume an open rel's tupledesc won't move.)
 		 *
 		 * Note that this process does not touch CurrentResourceOwner; which
 		 * is good because whatever ref counts the entry may have do not
@@ -1618,7 +1628,9 @@ RelationClearRelation(Relation relation, bool rebuild)
 		{
 			/* Should only get here if relation was deleted */
 			flush_rowtype_cache(old_reltype);
-			FreeTupleDesc(old_att);
+			Assert(old_att->tdrefcount > 0);
+			if (--old_att->tdrefcount == 0)
+				FreeTupleDesc(old_att);
 			if (old_rulescxt)
 				MemoryContextDelete(old_rulescxt);
 			pfree(relation);
@@ -1629,13 +1641,17 @@ RelationClearRelation(Relation relation, bool rebuild)
 		if (equalTupleDescs(old_att, relation->rd_att))
 		{
 			/* needn't flush typcache here */
-			FreeTupleDesc(relation->rd_att);
+			Assert(relation->rd_att->tdrefcount == 1);
+			if (--relation->rd_att->tdrefcount == 0)
+				FreeTupleDesc(relation->rd_att);
 			relation->rd_att = old_att;
 		}
 		else
 		{
 			flush_rowtype_cache(old_reltype);
-			FreeTupleDesc(old_att);
+			Assert(old_att->tdrefcount > 0);
+			if (--old_att->tdrefcount == 0)
+				FreeTupleDesc(old_att);
 		}
 		if (equalRuleLocks(old_rules, relation->rd_rules))
 		{
@@ -2075,6 +2091,7 @@ RelationBuildLocalRelation(const char *relname,
 	 * catalogs.  We can copy attnotnull constraints here, however.
 	 */
 	rel->rd_att = CreateTupleDescCopy(tupDesc);
+	rel->rd_att->tdrefcount = 1;	/* mark as refcounted */
 	has_not_null = false;
 	for (i = 0; i < natts; i++)
 	{
@@ -2996,6 +3013,8 @@ load_relcache_init_file(void)
 		/* initialize attribute tuple forms */
 		rel->rd_att = CreateTemplateTupleDesc(relform->relnatts,
 											  relform->relhasoids);
+		rel->rd_att->tdrefcount = 1;	/* mark as refcounted */
+
 		rel->rd_att->tdtypeid = relform->reltype;
 		rel->rd_att->tdtypmod = -1;		/* unnecessary, but... */
 
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index c3f0afa6853..fbeefad7457 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -36,7 +36,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/cache/typcache.c,v 1.18 2006/03/05 15:58:45 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/cache/typcache.c,v 1.19 2006/06/16 18:42:23 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -270,12 +270,16 @@ lookup_type_cache(Oid type_id, int flags)
 		Assert(rel->rd_rel->reltype == typentry->type_id);
 
 		/*
-		 * Notice that we simply store a link to the relcache's tupdesc. Since
-		 * we are relying on relcache to detect cache flush events, there's
-		 * not a lot of point to maintaining an independent copy.
+		 * Link to the tupdesc and increment its refcount (we assert it's
+		 * a refcounted descriptor).  We don't use IncrTupleDescRefCount()
+		 * for this, because the reference mustn't be entered in the current
+		 * resource owner; it can outlive the current query.
 		 */
 		typentry->tupDesc = RelationGetDescr(rel);
 
+		Assert(typentry->tupDesc->tdrefcount > 0);
+		typentry->tupDesc->tdrefcount++;
+
 		relation_close(rel, AccessShareLock);
 	}
 
@@ -283,29 +287,13 @@ lookup_type_cache(Oid type_id, int flags)
 }
 
 /*
- * 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)
-{
-	return lookup_rowtype_tupdesc_noerror(type_id, typmod, false);
-}
-
-/*
- * lookup_rowtype_tupdesc_noerror
+ * lookup_rowtype_tupdesc_internal --- internal routine to lookup a rowtype
  *
- * As above, but if the type is not a known composite type and noError
- * is true, returns NULL instead of ereport'ing.  (Note that if a bogus
- * type_id is passed, you'll get an ereport anyway.)
+ * Same API as lookup_rowtype_tupdesc_noerror, but the returned tupdesc
+ * hasn't had its refcount bumped.
  */
-TupleDesc
-lookup_rowtype_tupdesc_noerror(Oid type_id, int32 typmod, bool noError)
+static TupleDesc
+lookup_rowtype_tupdesc_internal(Oid type_id, int32 typmod, bool noError)
 {
 	if (type_id != RECORDOID)
 	{
@@ -339,6 +327,59 @@ lookup_rowtype_tupdesc_noerror(Oid type_id, int32 typmod, bool noError)
 	}
 }
 
+/*
+ * 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: 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.
+ */
+TupleDesc
+lookup_rowtype_tupdesc(Oid type_id, int32 typmod)
+{
+	TupleDesc	tupDesc;
+
+	tupDesc = lookup_rowtype_tupdesc_internal(type_id, typmod, false);
+	IncrTupleDescRefCount(tupDesc);
+	return tupDesc;
+}
+
+/*
+ * lookup_rowtype_tupdesc_noerror
+ *
+ * As above, but if the type is not a known composite type and noError
+ * is true, returns NULL instead of ereport'ing.  (Note that if a bogus
+ * type_id is passed, you'll get an ereport anyway.)
+ */
+TupleDesc
+lookup_rowtype_tupdesc_noerror(Oid type_id, int32 typmod, bool noError)
+{
+	TupleDesc	tupDesc;
+
+	tupDesc = lookup_rowtype_tupdesc_internal(type_id, typmod, noError);
+	if (tupDesc != NULL)
+		IncrTupleDescRefCount(tupDesc);
+	return tupDesc;
+}
+
+/*
+ * lookup_rowtype_tupdesc_copy
+ *
+ * Like lookup_rowtype_tupdesc(), but the returned TupleDesc has been
+ * copied into the CurrentMemoryContext and is not reference-counted.
+ */
+TupleDesc
+lookup_rowtype_tupdesc_copy(Oid type_id, int32 typmod)
+{
+	TupleDesc tmp;
+
+	tmp = lookup_rowtype_tupdesc_internal(type_id, typmod, false);
+	return CreateTupleDescCopyConstr(tmp);
+}
+
 
 /*
  * assign_record_type_typmod
@@ -425,6 +466,8 @@ assign_record_type_typmod(TupleDesc tupDesc)
 	/* if fail in subrs, no damage except possibly some wasted memory... */
 	entDesc = CreateTupleDescCopy(tupDesc);
 	recentry->tupdescs = lcons(entDesc, recentry->tupdescs);
+	/* mark it as a reference-counted tupdesc */
+	entDesc->tdrefcount = 1;
 	/* now it's safe to advance NextRecordTypmod */
 	newtypmod = NextRecordTypmod++;
 	entDesc->tdtypmod = newtypmod;
@@ -456,6 +499,17 @@ flush_rowtype_cache(Oid type_id)
 											  HASH_FIND, NULL);
 	if (typentry == NULL)
 		return;					/* no matching entry */
+	if (typentry->tupDesc == NULL)
+		return;					/* tupdesc hasn't been requested */
+
+	/*
+	 * Release our refcount and free the tupdesc if none remain.
+	 * (Can't use DecrTupleDescRefCount because this reference is not
+	 * logged in current resource owner.)
+	 */
+	Assert(typentry->tupDesc->tdrefcount > 0);
+	if (--typentry->tupDesc->tdrefcount == 0)
+		FreeTupleDesc(typentry->tupDesc);
 
 	typentry->tupDesc = NULL;
 }