Replace TupleTableSlot convention for whole-row variables and function

results with tuples as ordinary varlena Datums.  This commit does not
in itself do much for us, except eliminate the horrid memory leak
associated with evaluation of whole-row variables.  However, it lays the
groundwork for allowing composite types as table columns, and perhaps
some other useful features as well.  Per my proposal of a few days ago.

5 files changed, 270 insertions, 285 deletions

diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index 4c6f95a9a6f..b27e86122bc 100644
--- a/src/backend/executor/execQual.c
+++ b/src/backend/executor/execQual.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.157 2004/03/24 22:40:28 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.158 2004/04/01 21:28:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -49,6 +49,7 @@
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/lsyscache.h"
+#include "utils/typcache.h"
 
 
 /* static function decls */
@@ -110,7 +111,7 @@ static Datum ExecEvalCoerceToDomain(CoerceToDomainState *cstate,
 static Datum ExecEvalCoerceToDomainValue(ExprState *exprstate,
 										 ExprContext *econtext,
 										 bool *isNull, ExprDoneCond *isDone);
-static Datum ExecEvalFieldSelect(GenericExprState *fstate,
+static Datum ExecEvalFieldSelect(FieldSelectState *fstate,
 					ExprContext *econtext,
 					bool *isNull, ExprDoneCond *isDone);
 static Datum ExecEvalRelabelType(GenericExprState *exprstate,
@@ -420,16 +421,25 @@ ExecEvalVar(ExprState *exprstate, ExprContext *econtext,
 		*isDone = ExprSingleResult;
 
 	/*
-	 * get the slot we want
+	 * Get the slot and attribute number we want
+	 *
+	 * The asserts check that references to system attributes only appear
+	 * at the level of a relation scan; at higher levels, system attributes
+	 * must be treated as ordinary variables (since we no longer have access
+	 * to the original tuple).
 	 */
+	attnum = variable->varattno;
+
 	switch (variable->varno)
 	{
 		case INNER:				/* get the tuple from the inner node */
 			slot = econtext->ecxt_innertuple;
+			Assert(attnum > 0);
 			break;
 
 		case OUTER:				/* get the tuple from the outer node */
 			slot = econtext->ecxt_outertuple;
+			Assert(attnum > 0);
 			break;
 
 		default:				/* get the tuple from the relation being
@@ -444,8 +454,6 @@ ExecEvalVar(ExprState *exprstate, ExprContext *econtext,
 	heapTuple = slot->val;
 	tuple_type = slot->ttc_tupleDescriptor;
 
-	attnum = variable->varattno;
-
 	/*
 	 * Some checks that are only applied for user attribute numbers
 	 * (bogus system attnums will be caught inside heap_getattr).
@@ -481,38 +489,6 @@ ExecEvalVar(ExprState *exprstate, ExprContext *econtext,
 		Assert(variable->vartype == tuple_type->attrs[attnum - 1]->atttypid);
 	}
 
-	/*
-	 * If the attribute number is invalid, then we are supposed to return
-	 * the entire tuple; we give back a whole slot so that callers know
-	 * what the tuple looks like.
-	 *
-	 * XXX this is a horrid crock: since the pointer to the slot might live
-	 * longer than the current evaluation context, we are forced to copy
-	 * the tuple and slot into a long-lived context --- we use the
-	 * econtext's per-query memory which should be safe enough.  This
-	 * represents a serious memory leak if many such tuples are processed
-	 * in one command, however.  We ought to redesign the representation
-	 * of whole-tuple datums so that this is not necessary.
-	 *
-	 * We assume it's OK to point to the existing tupleDescriptor, rather
-	 * than copy that too.
-	 */
-	if (attnum == InvalidAttrNumber)
-	{
-		MemoryContext oldContext;
-		TupleTableSlot *tempSlot;
-		HeapTuple	tup;
-
-		oldContext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
-		tempSlot = MakeTupleTableSlot();
-		tup = heap_copytuple(heapTuple);
-		ExecStoreTuple(tup, tempSlot, InvalidBuffer, true);
-		ExecSetSlotDescriptor(tempSlot, tuple_type, false);
-		MemoryContextSwitchTo(oldContext);
-		*isNull = false;
-		return PointerGetDatum(tempSlot);
-	}
-
 	result = heap_getattr(heapTuple,	/* tuple containing attribute */
 						  attnum,		/* attribute number of desired
 										 * attribute */
@@ -656,17 +632,23 @@ ExecEvalParam(ExprState *exprstate, ExprContext *econtext,
  *		GetAttributeByName
  *		GetAttributeByNum
  *
- *		These are functions which return the value of the
- *		named attribute out of the tuple from the arg slot.  User defined
+ *		These functions return the value of the requested attribute
+ *		out of the given tuple Datum.
  *		C functions which take a tuple as an argument are expected
- *		to use this.  Ex: overpaid(EMP) might call GetAttributeByNum().
+ *		to use these.  Ex: overpaid(EMP) might call GetAttributeByNum().
+ *		Note: these are actually rather slow because they do a typcache
+ *		lookup on each call.
  */
 Datum
-GetAttributeByNum(TupleTableSlot *slot,
+GetAttributeByNum(HeapTupleHeader tuple,
 				  AttrNumber attrno,
 				  bool *isNull)
 {
-	Datum		retval;
+	Datum		result;
+	Oid			tupType;
+	int32		tupTypmod;
+	TupleDesc	tupDesc;
+	HeapTupleData tmptup;
 
 	if (!AttributeNumberIsValid(attrno))
 		elog(ERROR, "invalid attribute number %d", attrno);
@@ -674,29 +656,43 @@ GetAttributeByNum(TupleTableSlot *slot,
 	if (isNull == NULL)
 		elog(ERROR, "a NULL isNull pointer was passed");
 
-	if (TupIsNull(slot))
+	if (tuple == NULL)
 	{
+		/* Kinda bogus but compatible with old behavior... */
 		*isNull = true;
 		return (Datum) 0;
 	}
 
-	retval = heap_getattr(slot->val,
+	tupType = HeapTupleHeaderGetTypeId(tuple);
+	tupTypmod = HeapTupleHeaderGetTypMod(tuple);
+	tupDesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
+
+	/*
+	 * heap_getattr needs a HeapTuple not a bare HeapTupleHeader.  We set
+	 * all the fields in the struct just in case user tries to inspect
+	 * system columns.
+	 */
+	tmptup.t_len = HeapTupleHeaderGetDatumLength(tuple);
+	ItemPointerSetInvalid(&(tmptup.t_self));
+	tmptup.t_tableOid = InvalidOid;
+	tmptup.t_data = tuple;
+
+	result = heap_getattr(&tmptup,
 						  attrno,
-						  slot->ttc_tupleDescriptor,
+						  tupDesc,
 						  isNull);
-	if (*isNull)
-		return (Datum) 0;
-
-	return retval;
+	return result;
 }
 
 Datum
-GetAttributeByName(TupleTableSlot *slot, char *attname, bool *isNull)
+GetAttributeByName(HeapTupleHeader tuple, const char *attname, bool *isNull)
 {
 	AttrNumber	attrno;
-	TupleDesc	tupdesc;
-	Datum		retval;
-	int			natts;
+	Datum		result;
+	Oid			tupType;
+	int32		tupTypmod;
+	TupleDesc	tupDesc;
+	HeapTupleData tmptup;
 	int			i;
 
 	if (attname == NULL)
@@ -705,21 +701,23 @@ GetAttributeByName(TupleTableSlot *slot, char *attname, bool *isNull)
 	if (isNull == NULL)
 		elog(ERROR, "a NULL isNull pointer was passed");
 
-	if (TupIsNull(slot))
+	if (tuple == NULL)
 	{
+		/* Kinda bogus but compatible with old behavior... */
 		*isNull = true;
 		return (Datum) 0;
 	}
 
-	tupdesc = slot->ttc_tupleDescriptor;
-	natts = slot->val->t_data->t_natts;
+	tupType = HeapTupleHeaderGetTypeId(tuple);
+	tupTypmod = HeapTupleHeaderGetTypMod(tuple);
+	tupDesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
 
 	attrno = InvalidAttrNumber;
-	for (i = 0; i < tupdesc->natts; i++)
+	for (i = 0; i < tupDesc->natts; i++)
 	{
-		if (namestrcmp(&(tupdesc->attrs[i]->attname), attname) == 0)
+		if (namestrcmp(&(tupDesc->attrs[i]->attname), attname) == 0)
 		{
-			attrno = tupdesc->attrs[i]->attnum;
+			attrno = tupDesc->attrs[i]->attnum;
 			break;
 		}
 	}
@@ -727,14 +725,21 @@ GetAttributeByName(TupleTableSlot *slot, char *attname, bool *isNull)
 	if (attrno == InvalidAttrNumber)
 		elog(ERROR, "attribute \"%s\" does not exist", attname);
 
-	retval = heap_getattr(slot->val,
+	/*
+	 * heap_getattr needs a HeapTuple not a bare HeapTupleHeader.  We set
+	 * all the fields in the struct just in case user tries to inspect
+	 * system columns.
+	 */
+	tmptup.t_len = HeapTupleHeaderGetDatumLength(tuple);
+	ItemPointerSetInvalid(&(tmptup.t_self));
+	tmptup.t_tableOid = InvalidOid;
+	tmptup.t_data = tuple;
+
+	result = heap_getattr(&tmptup,
 						  attrno,
-						  tupdesc,
+						  tupDesc,
 						  isNull);
-	if (*isNull)
-		return (Datum) 0;
-
-	return retval;
+	return result;
 }
 
 /*
@@ -1133,14 +1138,14 @@ ExecMakeTableFunctionResult(ExprState *funcexpr,
 	Tuplestorestate *tupstore = NULL;
 	TupleDesc	tupdesc = NULL;
 	Oid			funcrettype;
+	bool		returnsTuple;
 	FunctionCallInfoData fcinfo;
 	ReturnSetInfo rsinfo;
+	HeapTupleData tmptup;
 	MemoryContext callerContext;
 	MemoryContext oldcontext;
-	TupleTableSlot *slot;
 	bool		direct_function_call;
 	bool		first_time = true;
-	bool		returnsTuple = false;
 
 	/*
 	 * Normally the passed expression tree will be a FuncExprState, since
@@ -1216,6 +1221,9 @@ ExecMakeTableFunctionResult(ExprState *funcexpr,
 
 	funcrettype = exprType((Node *) funcexpr->expr);
 
+	returnsTuple = (funcrettype == RECORDOID ||
+					get_typtype(funcrettype) == 'c');
+
 	/*
 	 * Prepare a resultinfo node for communication.  We always do this
 	 * even if not expecting a set result, so that we can pass
@@ -1282,31 +1290,34 @@ ExecMakeTableFunctionResult(ExprState *funcexpr,
 				break;
 
 			/*
+			 * Can't do anything useful with NULL rowtype values.  Currently
+			 * we raise an error, but another alternative is to just ignore
+			 * the result and "continue" to get another row.
+			 */
+			if (returnsTuple && fcinfo.isnull)
+				ereport(ERROR,
+						(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+						 errmsg("function returning row cannot return null value")));
+
+			/*
 			 * If first time through, build tupdesc and tuplestore for
 			 * result
 			 */
 			if (first_time)
 			{
 				oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
-				if (funcrettype == RECORDOID ||
-					get_typtype(funcrettype) == 'c')
+				if (returnsTuple)
 				{
 					/*
-					 * Composite type, so function should have returned a
-					 * TupleTableSlot; use its descriptor
+					 * Use the type info embedded in the rowtype Datum to
+					 * look up the needed tupdesc.  Make a copy for the query.
 					 */
-					slot = (TupleTableSlot *) DatumGetPointer(result);
-					if (fcinfo.isnull || !slot)
-						ereport(ERROR,
-								(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
-								 errmsg("function returning row cannot return null value")));
-					if (!IsA(slot, TupleTableSlot) ||
-						!slot->ttc_tupleDescriptor)
-						ereport(ERROR,
-								(errcode(ERRCODE_DATATYPE_MISMATCH),
-								 errmsg("function returning row did not return a valid tuple slot")));
-					tupdesc = CreateTupleDescCopy(slot->ttc_tupleDescriptor);
-					returnsTuple = true;
+					HeapTupleHeader	td;
+
+					td = DatumGetHeapTupleHeader(result);
+					tupdesc = lookup_rowtype_tupdesc(HeapTupleHeaderGetTypeId(td),
+													 HeapTupleHeaderGetTypMod(td));
+					tupdesc = CreateTupleDescCopy(tupdesc);
 				}
 				else
 				{
@@ -1319,8 +1330,7 @@ ExecMakeTableFunctionResult(ExprState *funcexpr,
 									   "column",
 									   funcrettype,
 									   -1,
-									   0,
-									   false);
+									   0);
 				}
 				tupstore = tuplestore_begin_heap(true, false, work_mem);
 				MemoryContextSwitchTo(oldcontext);
@@ -1333,15 +1343,17 @@ ExecMakeTableFunctionResult(ExprState *funcexpr,
 			 */
 			if (returnsTuple)
 			{
-				slot = (TupleTableSlot *) DatumGetPointer(result);
-				if (fcinfo.isnull ||
-					!slot ||
-					!IsA(slot, TupleTableSlot) ||
-					TupIsNull(slot))
-					ereport(ERROR,
-							(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
-							 errmsg("function returning row cannot return null value")));
-				tuple = slot->val;
+				HeapTupleHeader	td;
+
+				td = DatumGetHeapTupleHeader(result);
+
+				/*
+				 * tuplestore_puttuple needs a HeapTuple not a bare
+				 * HeapTupleHeader, but it doesn't need all the fields.
+				 */
+				tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
+				tmptup.t_data = td;
+				tuple = &tmptup;
 			}
 			else
 			{
@@ -2415,26 +2427,62 @@ ExecEvalCoerceToDomainValue(ExprState *exprstate,
  * ----------------------------------------------------------------
  */
 static Datum
-ExecEvalFieldSelect(GenericExprState *fstate,
+ExecEvalFieldSelect(FieldSelectState *fstate,
 					ExprContext *econtext,
 					bool *isNull,
 					ExprDoneCond *isDone)
 {
 	FieldSelect *fselect = (FieldSelect *) fstate->xprstate.expr;
 	Datum		result;
-	TupleTableSlot *resSlot;
+	Datum		tupDatum;
+	HeapTupleHeader tuple;
+	Oid			tupType;
+	int32		tupTypmod;
+	TupleDesc	tupDesc;
+	HeapTupleData tmptup;
 
-	result = ExecEvalExpr(fstate->arg, econtext, isNull, isDone);
+	tupDatum = ExecEvalExpr(fstate->arg, econtext, isNull, isDone);
 
 	/* this test covers the isDone exception too: */
 	if (*isNull)
-		return result;
+		return tupDatum;
+
+	tuple = DatumGetHeapTupleHeader(tupDatum);
+
+	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);
+	}
 
-	resSlot = (TupleTableSlot *) DatumGetPointer(result);
-	Assert(resSlot != NULL && IsA(resSlot, TupleTableSlot));
-	result = heap_getattr(resSlot->val,
+	/*
+	 * heap_getattr needs a HeapTuple not a bare HeapTupleHeader.  We set
+	 * all the fields in the struct just in case user tries to inspect
+	 * system columns.
+	 */
+	tmptup.t_len = HeapTupleHeaderGetDatumLength(tuple);
+	ItemPointerSetInvalid(&(tmptup.t_self));
+	tmptup.t_tableOid = InvalidOid;
+	tmptup.t_data = tuple;
+
+	result = heap_getattr(&tmptup,
 						  fselect->fieldnum,
-						  resSlot->ttc_tupleDescriptor,
+						  tupDesc,
 						  isNull);
 	return result;
 }
@@ -2703,11 +2751,12 @@ ExecInitExpr(Expr *node, PlanState *parent)
 		case T_FieldSelect:
 			{
 				FieldSelect *fselect = (FieldSelect *) node;
-				GenericExprState *gstate = makeNode(GenericExprState);
+				FieldSelectState *fstate = makeNode(FieldSelectState);
 
-				gstate->xprstate.evalfunc = (ExprStateEvalFunc) ExecEvalFieldSelect;
-				gstate->arg = ExecInitExpr(fselect->arg, parent);
-				state = (ExprState *) gstate;
+				fstate->xprstate.evalfunc = (ExprStateEvalFunc) ExecEvalFieldSelect;
+				fstate->arg = ExecInitExpr(fselect->arg, parent);
+				fstate->argdesc = NULL;
+				state = (ExprState *) fstate;
 			}
 			break;
 		case T_RelabelType:
@@ -3088,8 +3137,6 @@ ExecTargetList(List *targetlist,
 	List	   *tl;
 	bool		isNull;
 	bool		haveDoneSets;
-	static struct tupleDesc NullTupleDesc;		/* we assume this inits to
-												 * zeroes */
 
 	/*
 	 * debugging stuff
@@ -3106,13 +3153,8 @@ ExecTargetList(List *targetlist,
 	/*
 	 * There used to be some klugy and demonstrably broken code here that
 	 * special-cased the situation where targetlist == NIL.  Now we just
-	 * fall through and return an empty-but-valid tuple.  We do, however,
-	 * have to cope with the possibility that targettype is NULL ---
-	 * heap_formtuple won't like that, so pass a dummy descriptor with
-	 * natts = 0 to deal with it.
+	 * fall through and return an empty-but-valid tuple.
 	 */
-	if (targettype == NULL)
-		targettype = &NullTupleDesc;
 
 	/*
 	 * evaluate all the expressions in the target list
@@ -3285,8 +3327,8 @@ ExecProject(ProjectionInfo *projInfo, ExprDoneCond *isDone)
 	/*
 	 * store the tuple in the projection slot and return the slot.
 	 */
-	return ExecStoreTuple(newTuple,		/* tuple to store */
-						  slot, /* slot to store in */
-						  InvalidBuffer,		/* tuple has no buffer */
+	return ExecStoreTuple(newTuple,			/* tuple to store */
+						  slot,				/* slot to store in */
+						  InvalidBuffer,	/* tuple has no buffer */
 						  true);
 }
diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c
index 0bb59d26b11..faf910b736f 100644
--- a/src/backend/executor/execTuples.c
+++ b/src/backend/executor/execTuples.c
@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/execTuples.c,v 1.75 2004/01/07 18:56:26 neilc Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/execTuples.c,v 1.76 2004/04/01 21:28:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -109,8 +109,11 @@
 
 #include "funcapi.h"
 #include "access/heapam.h"
+#include "catalog/pg_type.h"
 #include "executor/executor.h"
 #include "utils/lsyscache.h"
+#include "utils/typcache.h"
+
 
 static TupleDesc ExecTypeFromTLInternal(List *targetList,
 										bool hasoid, bool skipjunk);
@@ -144,16 +147,11 @@ ExecCreateTupleTable(int initialSize)	/* initial number of slots in
 
 	/*
 	 * Now allocate our new table along with space for the pointers to the
-	 * tuples.
+	 * tuples.  Zero out the slots.
 	 */
 
 	newtable = (TupleTable) palloc(sizeof(TupleTableData));
-	array = (TupleTableSlot *) palloc(initialSize * sizeof(TupleTableSlot));
-
-	/*
-	 * clean out the slots we just allocated
-	 */
-	MemSet(array, 0, initialSize * sizeof(TupleTableSlot));
+	array = (TupleTableSlot *) palloc0(initialSize * sizeof(TupleTableSlot));
 
 	/*
 	 * initialize the new table and return it to the caller.
@@ -514,6 +512,10 @@ TupleTableSlot *
 ExecInitNullTupleSlot(EState *estate, TupleDesc tupType)
 {
 	TupleTableSlot *slot = ExecInitExtraTupleSlot(estate);
+	struct tupleDesc nullTupleDesc;
+	HeapTuple	nullTuple;
+	Datum		values[1];
+	char		nulls[1];
 
 	/*
 	 * Since heap_getattr() will treat attributes beyond a tuple's t_natts
@@ -521,15 +523,12 @@ ExecInitNullTupleSlot(EState *estate, TupleDesc tupType)
 	 * of zero length.	However, the slot descriptor must match the real
 	 * tupType.
 	 */
-	HeapTuple	nullTuple;
-	Datum		values[1];
-	char		nulls[1];
-	static struct tupleDesc NullTupleDesc;		/* we assume this inits to
-												 * zeroes */
+	nullTupleDesc = *tupType;
+	nullTupleDesc.natts = 0;
 
-	ExecSetSlotDescriptor(slot, tupType, false);
+	nullTuple = heap_formtuple(&nullTupleDesc, values, nulls);
 
-	nullTuple = heap_formtuple(&NullTupleDesc, values, nulls);
+	ExecSetSlotDescriptor(slot, tupType, false);
 
 	return ExecStoreTuple(nullTuple, slot, InvalidBuffer, true);
 }
@@ -590,21 +589,45 @@ ExecTypeFromTLInternal(List *targetList, bool hasoid, bool skipjunk)
 						   resdom->resname,
 						   resdom->restype,
 						   resdom->restypmod,
-						   0,
-						   false);
+						   0);
 	}
 
 	return typeInfo;
 }
 
 /*
+ * BlessTupleDesc - make a completed tuple descriptor useful for SRFs
+ *
+ * Rowtype Datums returned by a function must contain valid type information.
+ * This happens "for free" if the tupdesc came from a relcache entry, but
+ * not if we have manufactured a tupdesc for a transient RECORD datatype.
+ * In that case we have to notify typcache.c of the existence of the type.
+ */
+TupleDesc
+BlessTupleDesc(TupleDesc tupdesc)
+{
+	if (tupdesc->tdtypeid == RECORDOID &&
+		tupdesc->tdtypmod < 0)
+		assign_record_type_typmod(tupdesc);
+
+	return tupdesc;				/* just for notational convenience */
+}
+
+/*
  * TupleDescGetSlot - Initialize a slot based on the supplied tupledesc
+ *
+ * Note: this is obsolete; it is sufficient to call BlessTupleDesc on
+ * the tupdesc.  We keep it around just for backwards compatibility with
+ * existing user-written SRFs.
  */
 TupleTableSlot *
 TupleDescGetSlot(TupleDesc tupdesc)
 {
 	TupleTableSlot *slot;
 
+	/* The useful work is here */
+	BlessTupleDesc(tupdesc);
+
 	/* Make a standalone slot */
 	slot = MakeTupleTableSlot();
 
@@ -634,6 +657,9 @@ TupleDescGetAttInMetadata(TupleDesc tupdesc)
 
 	attinmeta = (AttInMetadata *) palloc(sizeof(AttInMetadata));
 
+	/* "Bless" the tupledesc so that we can make rowtype datums with it */
+	attinmeta->tupdesc = BlessTupleDesc(tupdesc);
+
 	/*
 	 * Gather info needed later to call the "in" function for each
 	 * attribute
@@ -653,7 +679,6 @@ TupleDescGetAttInMetadata(TupleDesc tupdesc)
 			atttypmods[i] = tupdesc->attrs[i]->atttypmod;
 		}
 	}
-	attinmeta->tupdesc = tupdesc;
 	attinmeta->attinfuncs = attinfuncinfo;
 	attinmeta->attelems = attelems;
 	attinmeta->atttypmods = atttypmods;
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 8dec6131fb4..aa7652e07ef 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.78 2004/03/21 22:29:11 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.79 2004/04/01 21:28:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -24,8 +24,10 @@
 #include "tcop/tcopprot.h"
 #include "tcop/utility.h"
 #include "utils/builtins.h"
+#include "utils/datum.h"
 #include "utils/lsyscache.h"
 #include "utils/syscache.h"
+#include "utils/typcache.h"
 
 
 /*
@@ -61,10 +63,6 @@ typedef struct
 	bool		returnsTuple;	/* true if return type is a tuple */
 	bool		shutdown_reg;	/* true if registered shutdown callback */
 
-	TupleTableSlot *funcSlot;	/* if one result we need to copy it before
-								 * we end execution of the function and
-								 * free stuff */
-
 	ParamListInfo paramLI;		/* Param list representing current args */
 
 	/* head of linked list of execution_state records */
@@ -196,34 +194,9 @@ init_sql_fcache(FmgrInfo *finfo)
 	 * get the type length and by-value flag from the type tuple
 	 */
 	fcache->typlen = typeStruct->typlen;
-
-	if (typeStruct->typtype != 'c' && rettype != RECORDOID)
-	{
-		/* The return type is not a composite type, so just use byval */
-		fcache->typbyval = typeStruct->typbyval;
-		fcache->returnsTuple = false;
-	}
-	else
-	{
-		/*
-		 * This is a hack.	We assume here that any function returning a
-		 * tuple returns it by reference.  This needs to be fixed, since
-		 * actually the mechanism isn't quite like return-by-reference.
-		 */
-		fcache->typbyval = false;
-		fcache->returnsTuple = true;
-	}
-
-	/*
-	 * If we are returning exactly one result then we have to copy tuples
-	 * and by reference results because we have to end the execution
-	 * before we return the results.  When you do this everything
-	 * allocated by the executor (i.e. slots and tuples) is freed.
-	 */
-	if (!finfo->fn_retset && !fcache->typbyval)
-		fcache->funcSlot = MakeTupleTableSlot();
-	else
-		fcache->funcSlot = NULL;
+	fcache->typbyval = typeStruct->typbyval;
+	fcache->returnsTuple = (typeStruct->typtype == 'c' ||
+							rettype == RECORDOID);
 
 	/*
 	 * Parse and plan the queries.	We need the argument type info to pass
@@ -366,39 +339,6 @@ postquel_sub_params(SQLFunctionCachePtr fcache,
 	fcache->paramLI = paramLI;
 }
 
-static TupleTableSlot *
-copy_function_result(SQLFunctionCachePtr fcache,
-					 TupleTableSlot *resultSlot)
-{
-	TupleTableSlot *funcSlot;
-	TupleDesc	resultTd;
-	HeapTuple	resultTuple;
-	HeapTuple	newTuple;
-
-	Assert(!TupIsNull(resultSlot));
-	resultTuple = resultSlot->val;
-
-	funcSlot = fcache->funcSlot;
-
-	if (funcSlot == NULL)
-		return resultSlot;		/* no need to copy result */
-
-	/*
-	 * If first time through, we have to initialize the funcSlot's tuple
-	 * descriptor.
-	 */
-	if (funcSlot->ttc_tupleDescriptor == NULL)
-	{
-		resultTd = CreateTupleDescCopy(resultSlot->ttc_tupleDescriptor);
-		ExecSetSlotDescriptor(funcSlot, resultTd, true);
-		ExecSetSlotDescriptorIsNew(funcSlot, true);
-	}
-
-	newTuple = heap_copytuple(resultTuple);
-
-	return ExecStoreTuple(newTuple, funcSlot, InvalidBuffer, true);
-}
-
 static Datum
 postquel_execute(execution_state *es,
 				 FunctionCallInfo fcinfo,
@@ -429,43 +369,51 @@ postquel_execute(execution_state *es,
 
 	if (LAST_POSTQUEL_COMMAND(es))
 	{
-		TupleTableSlot *resSlot;
-
 		/*
-		 * Copy the result.  copy_function_result is smart enough to do
-		 * nothing when no action is called for.  This helps reduce the
-		 * logic and code redundancy here.
+		 * Set up to return the function value.
 		 */
-		resSlot = copy_function_result(fcache, slot);
+		HeapTuple	tup = slot->val;
+		TupleDesc	tupDesc = slot->ttc_tupleDescriptor;
 
-		/*
-		 * If we are supposed to return a tuple, we return the tuple slot
-		 * pointer converted to Datum.	If we are supposed to return a
-		 * simple value, then project out the first attribute of the
-		 * result tuple (ie, take the first result column of the final
-		 * SELECT).
-		 */
 		if (fcache->returnsTuple)
 		{
 			/*
+			 * We are returning the whole tuple, so copy it into current
+			 * execution context and make sure it is a valid Datum.
+			 *
 			 * XXX do we need to remove junk attrs from the result tuple?
 			 * Probably OK to leave them, as long as they are at the end.
 			 */
-			value = PointerGetDatum(resSlot);
+			HeapTupleHeader	dtup;
+
+			dtup = (HeapTupleHeader) palloc(tup->t_len);
+			memcpy((char *) dtup, (char *) tup->t_data, tup->t_len);
+
+			/*
+			 * For RECORD results, make sure a typmod has been assigned.
+			 */
+			if (tupDesc->tdtypeid == RECORDOID &&
+				tupDesc->tdtypmod < 0)
+				assign_record_type_typmod(tupDesc);
+
+			HeapTupleHeaderSetDatumLength(dtup, tup->t_len);
+			HeapTupleHeaderSetTypeId(dtup, tupDesc->tdtypeid);
+			HeapTupleHeaderSetTypMod(dtup, tupDesc->tdtypmod);
+
+			value = PointerGetDatum(dtup);
 			fcinfo->isnull = false;
 		}
 		else
 		{
-			value = heap_getattr(resSlot->val,
-								 1,
-								 resSlot->ttc_tupleDescriptor,
-								 &(fcinfo->isnull));
-
 			/*
-			 * Note: if result type is pass-by-reference then we are
-			 * returning a pointer into the tuple copied by
-			 * copy_function_result.  This is OK.
+			 * Returning a scalar, which we have to extract from the
+			 * first column of the SELECT result, and then copy into current
+			 * execution context if needed.
 			 */
+			value = heap_getattr(tup, 1, tupDesc, &(fcinfo->isnull));
+
+			if (!fcinfo->isnull)
+				value = datumCopy(value, fcache->typbyval, fcache->typlen);
 		}
 
 		/*
diff --git a/src/backend/executor/nodeFunctionscan.c b/src/backend/executor/nodeFunctionscan.c
index f3fa17c8881..7847b24ffe2 100644
--- a/src/backend/executor/nodeFunctionscan.c
+++ b/src/backend/executor/nodeFunctionscan.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/nodeFunctionscan.c,v 1.23 2003/11/29 19:51:48 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeFunctionscan.c,v 1.24 2004/04/01 21:28:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -32,6 +32,7 @@
 #include "parser/parse_expr.h"
 #include "parser/parse_type.h"
 #include "utils/lsyscache.h"
+#include "utils/typcache.h"
 
 
 static TupleTableSlot *FunctionNext(FunctionScanState *node);
@@ -194,25 +195,12 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate)
 
 	if (functyptype == 'c')
 	{
-		/*
-		 * Composite data type, i.e. a table's row type
-		 */
-		Oid			funcrelid;
-		Relation	rel;
-
-		funcrelid = typeidTypeRelid(funcrettype);
-		if (!OidIsValid(funcrelid))
-			elog(ERROR, "invalid typrelid for complex type %u",
-				 funcrettype);
-		rel = relation_open(funcrelid, AccessShareLock);
-		tupdesc = CreateTupleDescCopy(RelationGetDescr(rel));
-		relation_close(rel, AccessShareLock);
+		/* Composite data type, e.g. a table's row type */
+		tupdesc = CreateTupleDescCopy(lookup_rowtype_tupdesc(funcrettype, -1));
 	}
 	else if (functyptype == 'b' || functyptype == 'd')
 	{
-		/*
-		 * Must be a base data type, i.e. scalar
-		 */
+		/* Must be a base data type, i.e. scalar */
 		char	   *attname = strVal(lfirst(rte->eref->colnames));
 
 		tupdesc = CreateTemplateTupleDesc(1, false);
@@ -221,14 +209,11 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate)
 						   attname,
 						   funcrettype,
 						   -1,
-						   0,
-						   false);
+						   0);
 	}
-	else if (functyptype == 'p' && funcrettype == RECORDOID)
+	else if (funcrettype == RECORDOID)
 	{
-		/*
-		 * Must be a pseudo type, i.e. record
-		 */
+		/* Must be a pseudo type, i.e. record */
 		tupdesc = BuildDescForRelation(rte->coldeflist);
 	}
 	else
@@ -237,6 +222,14 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate)
 		elog(ERROR, "function in FROM has unsupported return type");
 	}
 
+	/*
+	 * For RECORD results, make sure a typmod has been assigned.  (The
+	 * function should do this for itself, but let's cover things in case
+	 * it doesn't.)
+	 */
+	if (tupdesc->tdtypeid == RECORDOID && tupdesc->tdtypmod < 0)
+		assign_record_type_typmod(tupdesc);
+
 	scanstate->tupdesc = tupdesc;
 	ExecSetSlotDescriptor(scanstate->ss.ss_ScanTupleSlot,
 						  tupdesc, false);
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 1fb60fff02d..bcf7f8d52b6 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.112 2004/03/21 22:29:11 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.113 2004/04/01 21:28:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -19,6 +19,7 @@
 #include "executor/spi_priv.h"
 #include "tcop/tcopprot.h"
 #include "utils/lsyscache.h"
+#include "utils/typcache.h"
 
 
 uint32		SPI_processed = 0;
@@ -380,40 +381,11 @@ SPI_copytuple(HeapTuple tuple)
 	return ctuple;
 }
 
-TupleDesc
-SPI_copytupledesc(TupleDesc tupdesc)
+HeapTupleHeader
+SPI_returntuple(HeapTuple tuple, TupleDesc tupdesc)
 {
 	MemoryContext oldcxt = NULL;
-	TupleDesc	ctupdesc;
-
-	if (tupdesc == NULL)
-	{
-		SPI_result = SPI_ERROR_ARGUMENT;
-		return NULL;
-	}
-
-	if (_SPI_curid + 1 == _SPI_connected)		/* connected */
-	{
-		if (_SPI_current != &(_SPI_stack[_SPI_curid + 1]))
-			elog(ERROR, "SPI stack corrupted");
-		oldcxt = MemoryContextSwitchTo(_SPI_current->savedcxt);
-	}
-
-	ctupdesc = CreateTupleDescCopy(tupdesc);
-
-	if (oldcxt)
-		MemoryContextSwitchTo(oldcxt);
-
-	return ctupdesc;
-}
-
-TupleTableSlot *
-SPI_copytupleintoslot(HeapTuple tuple, TupleDesc tupdesc)
-{
-	MemoryContext oldcxt = NULL;
-	TupleTableSlot *cslot;
-	HeapTuple	ctuple;
-	TupleDesc	ctupdesc;
+	HeapTupleHeader	dtup;
 
 	if (tuple == NULL || tupdesc == NULL)
 	{
@@ -421,6 +393,11 @@ SPI_copytupleintoslot(HeapTuple tuple, TupleDesc tupdesc)
 		return NULL;
 	}
 
+	/* For RECORD results, make sure a typmod has been assigned */
+	if (tupdesc->tdtypeid == RECORDOID &&
+		tupdesc->tdtypmod < 0)
+		assign_record_type_typmod(tupdesc);
+
 	if (_SPI_curid + 1 == _SPI_connected)		/* connected */
 	{
 		if (_SPI_current != &(_SPI_stack[_SPI_curid + 1]))
@@ -428,17 +405,17 @@ SPI_copytupleintoslot(HeapTuple tuple, TupleDesc tupdesc)
 		oldcxt = MemoryContextSwitchTo(_SPI_current->savedcxt);
 	}
 
-	ctuple = heap_copytuple(tuple);
-	ctupdesc = CreateTupleDescCopy(tupdesc);
+	dtup = (HeapTupleHeader) palloc(tuple->t_len);
+	memcpy((char *) dtup, (char *) tuple->t_data, tuple->t_len);
 
-	cslot = MakeTupleTableSlot();
-	ExecSetSlotDescriptor(cslot, ctupdesc, true);
-	cslot = ExecStoreTuple(ctuple, cslot, InvalidBuffer, true);
+	HeapTupleHeaderSetDatumLength(dtup, tuple->t_len);
+	HeapTupleHeaderSetTypeId(dtup, tupdesc->tdtypeid);
+	HeapTupleHeaderSetTypMod(dtup, tupdesc->tdtypmod);
 
 	if (oldcxt)
 		MemoryContextSwitchTo(oldcxt);
 
-	return cslot;
+	return dtup;
 }
 
 HeapTuple