Add expected tuple descriptor to ReturnSetInfo information for table

functions, per suggestion from John Gray and Joe Conway.  Also, fix
plpgsql RETURN NEXT to verify that returned values match the expected
tupdesc.

6 files changed, 113 insertions, 74 deletions

diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index b422adc2061..31000ef1f2a 100644
--- a/src/backend/executor/execQual.c
+++ b/src/backend/executor/execQual.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execQual.c,v 1.102 2002/08/30 00:28:41 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execQual.c,v 1.103 2002/08/30 23:59:46 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -707,6 +707,7 @@ ExecMakeFunctionResult(FunctionCachePtr fcache,
 		fcinfo.resultinfo = (Node *) &rsinfo;
 		rsinfo.type = T_ReturnSetInfo;
 		rsinfo.econtext = econtext;
+		rsinfo.expectedDesc = NULL;
 		rsinfo.allowedModes = (int) SFRM_ValuePerCall;
 		rsinfo.returnMode = SFRM_ValuePerCall;
 		/* isDone is filled below */
@@ -851,6 +852,7 @@ ExecMakeFunctionResult(FunctionCachePtr fcache,
 Tuplestorestate *
 ExecMakeTableFunctionResult(Expr *funcexpr,
 							ExprContext *econtext,
+							TupleDesc expectedDesc,
 							TupleDesc *returnDesc)
 {
 	Tuplestorestate *tupstore = NULL;
@@ -859,7 +861,7 @@ ExecMakeTableFunctionResult(Expr *funcexpr,
 	List	   *argList;
 	FunctionCachePtr fcache;
 	FunctionCallInfoData fcinfo;
-	ReturnSetInfo rsinfo;		/* for functions returning sets */
+	ReturnSetInfo rsinfo;
 	ExprDoneCond argDone;
 	MemoryContext callerContext;
 	MemoryContext oldcontext;
@@ -918,17 +920,14 @@ ExecMakeTableFunctionResult(Expr *funcexpr,
 	}
 
 	/*
-	 * If function returns set, prepare a resultinfo node for
-	 * communication
+	 * Prepare a resultinfo node for communication.  We always do this even
+	 * if not expecting a set result, so that we can pass expectedDesc.
 	 */
-	if (fcache->func.fn_retset)
-	{
-		fcinfo.resultinfo = (Node *) &rsinfo;
-		rsinfo.type = T_ReturnSetInfo;
-		rsinfo.econtext = econtext;
-		rsinfo.allowedModes = (int) (SFRM_ValuePerCall | SFRM_Materialize);
-	}
-	/* we set these fields always since we examine them below */
+	fcinfo.resultinfo = (Node *) &rsinfo;
+	rsinfo.type = T_ReturnSetInfo;
+	rsinfo.econtext = econtext;
+	rsinfo.expectedDesc = expectedDesc;
+	rsinfo.allowedModes = (int) (SFRM_ValuePerCall | SFRM_Materialize);
 	rsinfo.returnMode = SFRM_ValuePerCall;
 	/* isDone is filled below */
 	rsinfo.setResult = NULL;
diff --git a/src/backend/executor/nodeFunctionscan.c b/src/backend/executor/nodeFunctionscan.c
index 3d2c160fb4f..e00778f3aa1 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.8 2002/08/30 00:28:41 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/nodeFunctionscan.c,v 1.9 2002/08/30 23:59:46 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -79,6 +79,7 @@ FunctionNext(FunctionScan *node)
 		scanstate->tuplestorestate = tuplestorestate =
 			ExecMakeTableFunctionResult((Expr *) scanstate->funcexpr,
 										econtext,
+										scanstate->tupdesc,
 										&funcTupdesc);
 
 		/*
diff --git a/src/backend/utils/fmgr/README b/src/backend/utils/fmgr/README
index 98d4e7bd51a..3ffc4dddd0e 100644
--- a/src/backend/utils/fmgr/README
+++ b/src/backend/utils/fmgr/README
@@ -423,6 +423,11 @@ function is called in).  The function stores pointers to the Tuplestore and
 TupleDesc into ReturnSetInfo, sets returnMode to indicate materialize mode,
 and returns null.  isDone is not used and should be left at ExprSingleResult.
 
+If the function is being called as a table function (ie, it appears in a
+FROM item), then the expected tuple descriptor is passed in ReturnSetInfo;
+in other contexts the expectedDesc field will be NULL.  The function need
+not pay attention to expectedDesc, but it may be useful in special cases.
+
 There is no support for functions accepting sets; instead, the function will
 be called multiple times, once for each element of the input set.
 
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 31a2b4a2399..3fbf63567ac 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: executor.h,v 1.75 2002/08/30 00:28:41 tgl Exp $
+ * $Id: executor.h,v 1.76 2002/08/30 23:59:46 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -82,6 +82,7 @@ extern Datum ExecMakeFunctionResult(FunctionCachePtr fcache,
 					   ExprDoneCond *isDone);
 extern Tuplestorestate *ExecMakeTableFunctionResult(Expr *funcexpr,
 													ExprContext *econtext,
+													TupleDesc expectedDesc,
 													TupleDesc *returnDesc);
 extern Datum ExecEvalExpr(Node *expression, ExprContext *econtext,
 			 bool *isNull, ExprDoneCond *isDone);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 3081a6842eb..2b18e70e42b 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: execnodes.h,v 1.73 2002/08/30 00:28:41 tgl Exp $
+ * $Id: execnodes.h,v 1.74 2002/08/30 23:59:46 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -147,12 +147,16 @@ typedef enum
 typedef struct ReturnSetInfo
 {
 	NodeTag		type;
+	/* values set by caller: */
 	ExprContext *econtext;		/* context function is being called in */
+	TupleDesc	expectedDesc;	/* tuple descriptor expected by caller */
 	int			allowedModes;	/* bitmask: return modes caller can handle */
-	SetFunctionReturnMode	returnMode;	/* actual return mode */
+	/* result status from function (but pre-initialized by caller): */
+	SetFunctionReturnMode returnMode;	/* actual return mode */
 	ExprDoneCond isDone;		/* status for ValuePerCall mode */
-	Tuplestorestate *setResult;	/* return object for Materialize mode */
-	TupleDesc	setDesc;		/* descriptor for Materialize mode */
+	/* fields filled by function in Materialize return mode: */
+	Tuplestorestate *setResult;	/* holds the complete returned tuple set */
+	TupleDesc	setDesc;		/* actual descriptor for returned tuples */
 } ReturnSetInfo;
 
 /* ----------------
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 49c88f3a090..6f52f2c0480 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -3,7 +3,7 @@
  *			  procedural language
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.60 2002/08/30 00:28:41 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.61 2002/08/30 23:59:46 tgl Exp $
  *
  *	  This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -144,9 +144,9 @@ static Datum exec_cast_value(Datum value, Oid valtype,
 				Oid reqtypelem,
 				int32 reqtypmod,
 				bool *isnull);
-static void exec_set_found(PLpgSQL_execstate * estate, bool state);
 static void exec_init_tuple_store(PLpgSQL_execstate *estate);
-static void exec_set_ret_tupdesc(PLpgSQL_execstate *estate, List *labels);
+static bool compatible_tupdesc(TupleDesc td1, TupleDesc td2);
+static void exec_set_found(PLpgSQL_execstate * estate, bool state);
 
 
 /* ----------
@@ -679,18 +679,9 @@ plpgsql_exec_trigger(PLpgSQL_function * func,
 		rettup = NULL;
 	else
 	{
-		TupleDesc	td1 = trigdata->tg_relation->rd_att;
-		TupleDesc	td2 = estate.rettupdesc;
-		int			i;
-
-		if (td1->natts != td2->natts)
+		if (!compatible_tupdesc(estate.rettupdesc,
+								trigdata->tg_relation->rd_att))
 			elog(ERROR, "returned tuple structure doesn't match table of trigger event");
-		for (i = 1; i <= td1->natts; i++)
-		{
-			if (SPI_gettypeid(td1, i) != SPI_gettypeid(td2, i))
-				elog(ERROR, "returned tuple structure doesn't match table of trigger event");
-		}
-
 		/* Copy tuple to upper executor memory */
 		rettup = SPI_copytuple((HeapTuple) (estate.retval));
 	}
@@ -1597,6 +1588,8 @@ static int
 exec_stmt_return_next(PLpgSQL_execstate *estate,
 					  PLpgSQL_stmt_return_next *stmt)
 {
+	TupleDesc tupdesc;
+	int		 natts;
 	HeapTuple tuple;
 	bool		free_tuple = false;
 
@@ -1606,35 +1599,39 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
 	if (estate->tuple_store == NULL)
 		exec_init_tuple_store(estate);
 
+	/* rettupdesc will be filled by exec_init_tuple_store */
+	tupdesc = estate->rettupdesc;
+	natts	= tupdesc->natts;
+
 	if (stmt->rec)
 	{
 		PLpgSQL_rec *rec = (PLpgSQL_rec *) (estate->datums[stmt->rec->recno]);
+
+		if (!compatible_tupdesc(tupdesc, rec->tupdesc))
+			elog(ERROR, "Wrong record type supplied in RETURN NEXT");
 		tuple = rec->tup;
-		estate->rettupdesc = rec->tupdesc;
 	}
 	else if (stmt->row)
 	{
-		PLpgSQL_var *var;
-		TupleDesc tupdesc;
 		Datum	*dvalues;
 		char	*nulls;
-		int		 natts;
 		int		 i;
 
-		if (!estate->rettupdesc)
-			exec_set_ret_tupdesc(estate, NIL);
+		if (natts != stmt->row->nfields)
+			elog(ERROR, "Wrong record type supplied in RETURN NEXT");
 
-		tupdesc = estate->rettupdesc;
-		natts	= tupdesc->natts;
 		dvalues	= (Datum *) palloc(natts * sizeof(Datum));
 		nulls	= (char *) palloc(natts * sizeof(char));
-
 		MemSet(dvalues, 0, natts * sizeof(Datum));
 		MemSet(nulls, 'n', natts);
 
-		for (i = 0; i < stmt->row->nfields; i++)
+		for (i = 0; i < natts; i++)
 		{
+			PLpgSQL_var *var;
+
 			var = (PLpgSQL_var *) (estate->datums[stmt->row->varnos[i]]);
+			if (var->datatype->typoid != tupdesc->attrs[i]->atttypid)
+				elog(ERROR, "Wrong record type supplied in RETURN NEXT");
 			dvalues[i] = var->value;
 			if (!var->isnull)
 				nulls[i] = ' ';
@@ -1650,19 +1647,40 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
 	{
 		Datum	retval;
 		bool	isNull;
+		Oid		rettype;
 		char	nullflag;
 
-		if (!estate->rettupdesc)
-			exec_set_ret_tupdesc(estate, makeList1(makeString("unused")));
+		if (natts != 1)
+			elog(ERROR, "Wrong result type supplied in RETURN NEXT");
 
 		retval = exec_eval_expr(estate,
 								stmt->expr,
 								&isNull,
-								&(estate->rettype));
+								&rettype);
+
+		/* coerce type if needed */
+		if (!isNull && rettype != tupdesc->attrs[0]->atttypid)
+		{
+			Oid targType = tupdesc->attrs[0]->atttypid;
+			Oid typInput;
+			Oid typElem;
+			FmgrInfo	finfo_input;
+
+			getTypeInputInfo(targType, &typInput, &typElem);
+			fmgr_info(typInput, &finfo_input);
+
+			retval = exec_cast_value(retval,
+									 rettype,
+									 targType,
+									 &finfo_input,
+									 typElem,
+									 tupdesc->attrs[0]->atttypmod,
+									 &isNull);
+		}
 
 		nullflag = isNull ? 'n' : ' ';
 
-		tuple = heap_formtuple(estate->rettupdesc, &retval, &nullflag);
+		tuple = heap_formtuple(tupdesc, &retval, &nullflag);
 
 		free_tuple = true;
 
@@ -1690,24 +1708,17 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
 }
 
 static void
-exec_set_ret_tupdesc(PLpgSQL_execstate *estate, List *labels)
-{
-	estate->rettype = estate->fn_rettype;
-	estate->rettupdesc = TypeGetTupleDesc(estate->rettype, labels);
-
-	if (!estate->rettupdesc)
-		elog(ERROR, "Could not produce descriptor for rowtype");
-}
-
-static void
 exec_init_tuple_store(PLpgSQL_execstate *estate)
 {
 	ReturnSetInfo *rsi = estate->rsi;
 	MemoryContext oldcxt;
 
-	/* Check caller can handle a set result */
+	/*
+	 * Check caller can handle a set result in the way we want
+	 */
 	if (!rsi || !IsA(rsi, ReturnSetInfo) ||
-		(rsi->allowedModes & SFRM_Materialize) == 0)
+		(rsi->allowedModes & SFRM_Materialize) == 0 ||
+		rsi->expectedDesc == NULL)
 		elog(ERROR, "Set-valued function called in context that cannot accept a set");
 
 	estate->tuple_store_cxt = rsi->econtext->ecxt_per_query_memory;
@@ -1715,6 +1726,8 @@ exec_init_tuple_store(PLpgSQL_execstate *estate)
 	oldcxt = MemoryContextSwitchTo(estate->tuple_store_cxt);
 	estate->tuple_store = tuplestore_begin_heap(true, SortMem);
 	MemoryContextSwitchTo(oldcxt);
+
+	estate->rettupdesc = rsi->expectedDesc;
 }
 
 
@@ -2839,9 +2852,9 @@ exec_assign_value(PLpgSQL_execstate * estate,
 	bool		attisnull;
 	Oid			atttype;
 	int32		atttypmod;
-	HeapTuple	typetup;
 	HeapTuple	newtup;
-	Form_pg_type typeStruct;
+	Oid			typInput;
+	Oid			typElem;
 	FmgrInfo	finfo_input;
 
 	switch (target->dtype)
@@ -2942,19 +2955,17 @@ exec_assign_value(PLpgSQL_execstate * estate,
 			 */
 			atttype = SPI_gettypeid(rec->tupdesc, fno + 1);
 			atttypmod = rec->tupdesc->attrs[fno]->atttypmod;
-			typetup = SearchSysCache(TYPEOID,
-									 ObjectIdGetDatum(atttype),
-									 0, 0, 0);
-			if (!HeapTupleIsValid(typetup))
-				elog(ERROR, "cache lookup for type %u failed", atttype);
-			typeStruct = (Form_pg_type) GETSTRUCT(typetup);
-			fmgr_info(typeStruct->typinput, &finfo_input);
+			getTypeInputInfo(atttype, &typInput, &typElem);
+			fmgr_info(typInput, &finfo_input);
 
 			attisnull = *isNull;
-			values[fno] = exec_cast_value(value, valtype,
-										  atttype, &finfo_input,
-										  typeStruct->typelem,
-										  atttypmod, &attisnull);
+			values[fno] = exec_cast_value(value,
+										  valtype,
+										  atttype,
+										  &finfo_input,
+										  typElem,
+										  atttypmod,
+										  &attisnull);
 			if (attisnull)
 				nulls[fno] = 'n';
 			else
@@ -2964,13 +2975,11 @@ exec_assign_value(PLpgSQL_execstate * estate,
 			 * Avoid leaking the result of exec_cast_value, if it
 			 * performed a conversion to a pass-by-ref type.
 			 */
-			if (!typeStruct->typbyval && !attisnull && values[fno] != value)
+			if (!attisnull && values[fno] != value && !get_typbyval(atttype))
 				mustfree = DatumGetPointer(values[fno]);
 			else
 				mustfree = NULL;
 
-			ReleaseSysCache(typetup);
-
 			/*
 			 * Now call heap_formtuple() to create a new tuple that
 			 * replaces the old one in the record.
@@ -3576,6 +3585,26 @@ exec_simple_check_plan(PLpgSQL_expr * expr)
 	expr->plan_simple_type = exprType(tle->expr);
 }
 
+/*
+ * Check two tupledescs have matching number and types of attributes
+ */
+static bool
+compatible_tupdesc(TupleDesc td1, TupleDesc td2)
+{
+	int			i;
+
+	if (td1->natts != td2->natts)
+		return false;
+
+	for (i = 0; i < td1->natts; i++)
+	{
+		if (td1->attrs[i]->atttypid != td2->attrs[i]->atttypid)
+			return false;
+	}
+
+	return true;
+}
+
 /* ----------
  * exec_set_found			Set the global found variable
  *					to true/false