Code review for standalone composite types, query-specified composite

types, SRFs.  Not happy with memory management yet, but I'll commit these
other changes.

4 files changed, 60 insertions, 68 deletions

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 7a6e1c69956..6318d79d4eb 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -27,7 +27,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.175 2002/08/28 20:46:22 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.176 2002/08/29 00:17:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -786,10 +786,6 @@ initResultRelInfo(ResultRelInfo *resultRelInfo,
 			elog(ERROR, "You can't change view relation %s",
 				 RelationGetRelationName(resultRelationDesc));
 			break;
-		case RELKIND_COMPOSITE_TYPE:
-			elog(ERROR, "You can't change type relation %s",
-				 RelationGetRelationName(resultRelationDesc));
-			break;
 	}
 
 	MemSet(resultRelInfo, 0, sizeof(ResultRelInfo));
diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c
index 06a784ce26d..afadcd3137b 100644
--- a/src/backend/executor/execTuples.c
+++ b/src/backend/executor/execTuples.c
@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execTuples.c,v 1.56 2002/07/20 05:49:27 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execTuples.c,v 1.57 2002/08/29 00:17:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -109,8 +109,9 @@
 
 #include "funcapi.h"
 #include "access/heapam.h"
-#include "catalog/pg_type.h"
 #include "executor/executor.h"
+#include "utils/lsyscache.h"
+
 
 /* ----------------------------------------------------------------
  *				  tuple table create/delete functions
@@ -676,8 +677,7 @@ ExecTypeFromTL(List *targetList, hasoid_t withoid)
 }
 
 /*
- * TupleDescGetSlot - Initialize a slot based on the supplied
- * tupledesc
+ * TupleDescGetSlot - Initialize a slot based on the supplied tupledesc
  */
 TupleTableSlot *
 TupleDescGetSlot(TupleDesc tupdesc)
@@ -695,40 +695,36 @@ TupleDescGetSlot(TupleDesc tupdesc)
 }
 
 /*
- * TupleDescGetAttInMetadata - Get a pointer to AttInMetadata based on the
+ * TupleDescGetAttInMetadata - Build an AttInMetadata structure based on the
  * supplied TupleDesc. AttInMetadata can be used in conjunction with C strings
  * to produce a properly formed tuple.
  */
 AttInMetadata *
 TupleDescGetAttInMetadata(TupleDesc tupdesc)
 {
-	int				natts;
+	int				natts = tupdesc->natts;
 	int				i;
 	Oid				atttypeid;
 	Oid				attinfuncid;
-	Oid				attelem;
 	FmgrInfo	   *attinfuncinfo;
 	Oid			   *attelems;
-	int4		   *atttypmods;
+	int32		   *atttypmods;
 	AttInMetadata  *attinmeta;
 
 	attinmeta = (AttInMetadata *) palloc(sizeof(AttInMetadata));
-	natts = tupdesc->natts;
 
 	/*
 	 * Gather info needed later to call the "in" function for each attribute
 	 */
 	attinfuncinfo = (FmgrInfo *) palloc(natts * sizeof(FmgrInfo));
 	attelems = (Oid *) palloc(natts * sizeof(Oid));
-	atttypmods = (int4 *) palloc(natts * sizeof(int4));
+	atttypmods = (int32 *) palloc(natts * sizeof(int32));
 
 	for (i = 0; i < natts; i++)
 	{
 		atttypeid = tupdesc->attrs[i]->atttypid;
-		get_type_metadata(atttypeid, &attinfuncid, &attelem);
-
+		getTypeInputInfo(atttypeid, &attinfuncid, &attelems[i]);
 		fmgr_info(attinfuncid, &attinfuncinfo[i]);
-		attelems[i] = attelem;
 		atttypmods[i] = tupdesc->attrs[i]->atttypmod;
 	}
 	attinmeta->tupdesc = tupdesc;
@@ -746,39 +742,35 @@ TupleDescGetAttInMetadata(TupleDesc tupdesc)
 HeapTuple
 BuildTupleFromCStrings(AttInMetadata *attinmeta, char **values)
 {
-	TupleDesc			tupdesc;
-	int					natts;
-	HeapTuple			tuple;
+	TupleDesc			tupdesc = attinmeta->tupdesc;
+	int					natts = tupdesc->natts;
+	Datum			   *dvalues;
 	char			   *nulls;
 	int					i;
-	Datum			   *dvalues;
-	FmgrInfo			attinfuncinfo;
 	Oid					attelem;
-	int4				atttypmod;
-
-	tupdesc = attinmeta->tupdesc;
-	natts = tupdesc->natts;
+	int32				atttypmod;
+	HeapTuple			tuple;
 
 	dvalues = (Datum *) palloc(natts * sizeof(Datum));
 	nulls = (char *) palloc(natts * sizeof(char));
 
-	/* Call the "in" function for each attribute */
+	/* Call the "in" function for each non-null attribute */
 	for (i = 0; i < natts; i++)
 	{
 		if (values[i] != NULL)
 		{
-			attinfuncinfo = attinmeta->attinfuncs[i];
 			attelem = attinmeta->attelems[i];
 			atttypmod = attinmeta->atttypmods[i];
 
-			dvalues[i] = FunctionCall3(&attinfuncinfo, CStringGetDatum(values[i]),
-										ObjectIdGetDatum(attelem),
-										Int32GetDatum(atttypmod));
+			dvalues[i] = FunctionCall3(&attinmeta->attinfuncs[i],
+									   CStringGetDatum(values[i]),
+									   ObjectIdGetDatum(attelem),
+									   Int32GetDatum(atttypmod));
 			nulls[i] = ' ';
 		}
 		else
 		{
-			dvalues[i] = PointerGetDatum(NULL);
+			dvalues[i] = (Datum) 0;
 			nulls[i] = 'n';
 		}
 	}
@@ -788,6 +780,13 @@ BuildTupleFromCStrings(AttInMetadata *attinmeta, char **values)
 	 */
 	tuple = heap_formtuple(tupdesc, dvalues, nulls);
 
+	/*
+	 * Release locally palloc'd space.  XXX would probably be good to
+	 * pfree values of pass-by-reference datums, as well.
+	 */
+	pfree(dvalues);
+	pfree(nulls);
+
 	return tuple;
 }
 
@@ -804,7 +803,7 @@ begin_tup_output_tupdesc(CommandDest dest, TupleDesc tupdesc)
 
 	tstate = (TupOutputState *) palloc(sizeof(TupOutputState));
 
-	tstate->tupdesc = tupdesc;
+	tstate->metadata = TupleDescGetAttInMetadata(tupdesc);
 	tstate->destfunc = DestToFunction(dest);
 
 	(*tstate->destfunc->setup) (tstate->destfunc, (int) CMD_SELECT,
@@ -823,20 +822,22 @@ void
 do_tup_output(TupOutputState *tstate, char **values)
 {
 	/* build a tuple from the input strings using the tupdesc */
-	AttInMetadata *attinmeta = TupleDescGetAttInMetadata(tstate->tupdesc);
-	HeapTuple	tuple = BuildTupleFromCStrings(attinmeta, values);
+	HeapTuple	tuple = BuildTupleFromCStrings(tstate->metadata, values);
 
 	/* send the tuple to the receiver */
 	(*tstate->destfunc->receiveTuple) (tuple,
-									   tstate->tupdesc,
+									   tstate->metadata->tupdesc,
 									   tstate->destfunc);
 	/* clean up */
 	heap_freetuple(tuple);
 }
 
-/* write a chunk of text, breaking at newline characters
+/*
+ * write a chunk of text, breaking at newline characters
+ *
  * NB: scribbles on its input!
- * Should only be used for a single TEXT attribute tupdesc.
+ *
+ * Should only be used with a single-TEXT-attribute tupdesc.
  */
 void
 do_text_output_multiline(TupOutputState *tstate, char *text)
@@ -859,5 +860,6 @@ void
 end_tup_output(TupOutputState *tstate)
 {
 	(*tstate->destfunc->cleanup) (tstate->destfunc);
+	/* XXX worth cleaning up the attinmetadata? */
 	pfree(tstate);
 }
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index eba919b8b66..fe473404b91 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/functions.c,v 1.55 2002/08/23 16:41:37 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/functions.c,v 1.56 2002/08/29 00:17:04 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -193,9 +193,10 @@ init_sql_fcache(FmgrInfo *finfo)
 	 */
 	fcache->typlen = typeStruct->typlen;
 
-	if (typeStruct->typtype != 'c')
+	if (typeStruct->typtype != 'c' &&
+		procedureStruct->prorettype != RECORDOID)
 	{
-		/* The return type is not a relation, so just use byval */
+		/* The return type is not a composite type, so just use byval */
 		fcache->typbyval = typeStruct->typbyval;
 		fcache->returnsTuple = false;
 	}
diff --git a/src/backend/executor/nodeFunctionscan.c b/src/backend/executor/nodeFunctionscan.c
index b8aecc2b825..381b6047bf0 100644
--- a/src/backend/executor/nodeFunctionscan.c
+++ b/src/backend/executor/nodeFunctionscan.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/nodeFunctionscan.c,v 1.5 2002/08/05 02:30:50 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/nodeFunctionscan.c,v 1.6 2002/08/29 00:17:04 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -130,10 +130,10 @@ FunctionNext(FunctionScan *node)
 /* ----------------------------------------------------------------
  *		ExecFunctionScan(node)
  *
- *		Scans the Function sequentially and returns the next qualifying
+ *		Scans the function sequentially and returns the next qualifying
  *		tuple.
  *		It calls the ExecScan() routine and passes it the access method
- *		which retrieve tuples sequentially.
+ *		which retrieves tuples sequentially.
  *
  */
 
@@ -156,7 +156,6 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, Plan *parent)
 	FunctionScanState  *scanstate;
 	RangeTblEntry	   *rte;
 	Oid					funcrettype;
-	Oid					funcrelid;
 	char				functyptype;
 	TupleDesc			tupdesc = NULL;
 
@@ -201,31 +200,26 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, Plan *parent)
 
 	/*
 	 * Now determine if the function returns a simple or composite type,
-	 * and check/add column aliases.
+	 * and build an appropriate tupdesc.
 	 */
 	functyptype = get_typtype(funcrettype);
 
-	/*
-	 * Build a suitable tupledesc representing the output rows
-	 */
 	if (functyptype == 'c')
 	{
-		funcrelid = typeidTypeRelid(funcrettype);
-		if (OidIsValid(funcrelid))
-		{
-			/*
-			 * Composite data type, i.e. a table's row type
-			 * Same as ordinary relation RTE
-			 */
-			Relation	rel;
+		/*
+		 * Composite data type, i.e. a table's row type
+		 */
+		Oid			funcrelid;
+		Relation	rel;
 
-			rel = relation_open(funcrelid, AccessShareLock);
-			tupdesc = CreateTupleDescCopy(RelationGetDescr(rel));
-			relation_close(rel, AccessShareLock);
-			scanstate->returnsTuple = true;
-		}
-		else
-			elog(ERROR, "Invalid return relation specified for function");
+		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);
+		scanstate->returnsTuple = true;
 	}
 	else if (functyptype == 'b' || functyptype == 'd')
 	{
@@ -461,8 +455,7 @@ function_getonetuple(FunctionScanState *scanstate,
 			 */
 			if (fn_typtype == 'p' && fn_typeid == RECORDOID)
 				if (tupledesc_mismatch(tupdesc, slot->ttc_tupleDescriptor))
-					elog(ERROR, "Query-specified return tuple and actual"
-									" function return tuple do not match");
+					elog(ERROR, "Query-specified return tuple and actual function return tuple do not match");
 		}
 		else
 		{
@@ -480,7 +473,7 @@ function_getonetuple(FunctionScanState *scanstate,
 								  slot,				/* slot to store in */
 								  InvalidBuffer,	/* buffer associated with
 													 * this tuple */
-								  true);			/* pfree this pointer */
+								  true);			/* pfree this tuple */
 		}
 	}