Extend ExecMakeFunctionResult() to support set-returning functions that return

via a tuplestore instead of value-per-call.  Refactor a few things to reduce
ensuing code duplication with nodeFunctionscan.c.  This represents the
reasonably noncontroversial part of my proposed patch to switch SQL functions
over to returning tuplestores.  For the moment, SQL functions still do things
the old way.  However, this change enables PL SRFs to be called in targetlists
(observe changes in plperl regression results).

contrib/tablefunc/tablefunc.c

 /*
- * $PostgreSQL: pgsql/contrib/tablefunc/tablefunc.c,v 1.53 2008/05/17 01:28:22 adunstan Exp $ 
+ * $PostgreSQL: pgsql/contrib/tablefunc/tablefunc.c,v 1.54 2008/10/28 22:02:05 tgl Exp $ 
  *
  *
  * tablefunc
@@ -709,7 +709,8 @@ crosstab_hash(PG_FUNCTION_ARGS)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("set-valued function called in context that cannot accept a set")));
-	if (!(rsinfo->allowedModes & SFRM_Materialize))
+	if (!(rsinfo->allowedModes & SFRM_Materialize) ||
+		rsinfo->expectedDesc == NULL)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("materialize mode required, but it is not " \
@@ -1072,7 +1073,8 @@ connectby_text(PG_FUNCTION_ARGS)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("set-valued function called in context that cannot accept a set")));
-	if (!(rsinfo->allowedModes & SFRM_Materialize))
+	if (!(rsinfo->allowedModes & SFRM_Materialize) ||
+		rsinfo->expectedDesc == NULL)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("materialize mode required, but it is not " \
@@ -1139,7 +1141,6 @@ connectby_text_serial(PG_FUNCTION_ARGS)
 	char	   *branch_delim = NULL;
 	bool		show_branch = false;
 	bool		show_serial = true;
-
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
 	TupleDesc	tupdesc;
 	AttInMetadata *attinmeta;
@@ -1151,7 +1152,8 @@ connectby_text_serial(PG_FUNCTION_ARGS)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("set-valued function called in context that cannot accept a set")));
-	if (!(rsinfo->allowedModes & SFRM_Materialize))
+	if (!(rsinfo->allowedModes & SFRM_Materialize) ||
+		rsinfo->expectedDesc == NULL)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("materialize mode required, but it is not " \

doc/src/sgml/plpgsql.sgml

-<!-- $PostgreSQL: pgsql/doc/src/sgml/plpgsql.sgml,v 1.134 2008/09/24 19:51:22 momjian Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/plpgsql.sgml,v 1.135 2008/10/28 22:02:05 tgl Exp $ -->
 
 <chapter id="plpgsql">
   <title><application>PL/pgSQL</application> - <acronym>SQL</acronym> Procedural Language</title>
@@ -1575,10 +1575,6 @@ LANGUAGE 'plpgsql' ;
 
 SELECT * FROM getallfoo();
 </programlisting>
-
-      Note that functions using <command>RETURN NEXT</command> or
-      <command>RETURN QUERY</command> must be called as a table source in
-      a <literal>FROM</literal> clause.
      </para>
 
      <note>

src/backend/executor/execTuples.c

@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/execTuples.c,v 1.102 2008/08/25 22:42:32 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/execTuples.c,v 1.103 2008/10/28 22:02:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -766,6 +766,33 @@ ExecFetchSlotMinimalTuple(TupleTableSlot *slot)
 	return slot->tts_mintuple;
 }
 
+/* --------------------------------
+ *		ExecFetchSlotTupleDatum
+ *			Fetch the slot's tuple as a composite-type Datum.
+ *
+ *		We convert the slot's contents to local physical-tuple form,
+ *		and fill in the Datum header fields.  Note that the result
+ *		always points to storage owned by the slot.
+ * --------------------------------
+ */
+Datum
+ExecFetchSlotTupleDatum(TupleTableSlot *slot)
+{
+	HeapTuple	tup;
+	HeapTupleHeader td;
+	TupleDesc	tupdesc;
+
+	/* Make sure we can scribble on the slot contents ... */
+	tup = ExecMaterializeSlot(slot);
+	/* ... and set up the composite-Datum header fields, in case not done */
+	td = tup->t_data;
+	tupdesc = slot->tts_tupleDescriptor;
+	HeapTupleHeaderSetDatumLength(td, tup->t_len);
+	HeapTupleHeaderSetTypeId(td, tupdesc->tdtypeid);
+	HeapTupleHeaderSetTypMod(td, tupdesc->tdtypmod);
+	return PointerGetDatum(td);
+}
+
 /* --------------------------------
  *		ExecMaterializeSlot
  *			Force a slot into the "materialized" state.

src/backend/executor/nodeFunctionscan.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/nodeFunctionscan.c,v 1.47 2008/10/01 19:51:49 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeFunctionscan.c,v 1.48 2008/10/28 22:02:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -28,7 +28,6 @@
 
 
 static TupleTableSlot *FunctionNext(FunctionScanState *node);
-static void tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc);
 
 /* ----------------------------------------------------------------
  *						Scan Support
@@ -62,32 +61,10 @@ FunctionNext(FunctionScanState *node)
 	 */
 	if (tuplestorestate == NULL)
 	{
-		ExprContext *econtext = node->ss.ps.ps_ExprContext;
-		TupleDesc	funcTupdesc;
-
 		node->tuplestorestate = tuplestorestate =
 			ExecMakeTableFunctionResult(node->funcexpr,
-										econtext,
-										node->tupdesc,
-										&funcTupdesc);
-
-		/*
-		 * If function provided a tupdesc, cross-check it.	We only really
-		 * need to do this for functions returning RECORD, but might as well
-		 * do it always.
-		 */
-		if (funcTupdesc)
-		{
-			tupledesc_match(node->tupdesc, funcTupdesc);
-
-			/*
-			 * If it is a dynamically-allocated TupleDesc, free it: it is
-			 * typically allocated in the EState's per-query context, so we
-			 * must avoid leaking it on rescan.
-			 */
-			if (funcTupdesc->tdrefcount == -1)
-				FreeTupleDesc(funcTupdesc);
-		}
+										node->ss.ps.ps_ExprContext,
+										node->tupdesc);
 	}
 
 	/*
@@ -296,9 +273,9 @@ ExecFunctionReScan(FunctionScanState *node, ExprContext *exprCtxt)
 
 	/*
 	 * Here we have a choice whether to drop the tuplestore (and recompute the
-	 * function outputs) or just rescan it.  This should depend on whether the
-	 * function expression contains parameters and/or is marked volatile.
-	 * FIXME soon.
+	 * function outputs) or just rescan it.  We must recompute if the
+	 * expression contains parameters, else we rescan.  XXX maybe we should
+	 * recompute if the function is volatile?
 	 */
 	if (node->ss.ps.chgParam != NULL)
 	{
@@ -308,51 +285,3 @@ ExecFunctionReScan(FunctionScanState *node, ExprContext *exprCtxt)
 	else
 		tuplestore_rescan(node->tuplestorestate);
 }
-
-/*
- * Check that function result tuple type (src_tupdesc) matches or can
- * be considered to match what the query expects (dst_tupdesc). If
- * they don't match, ereport.
- *
- * We really only care about number of attributes and data type.
- * Also, we can ignore type mismatch on columns that are dropped in the
- * destination type, so long as the physical storage matches.  This is
- * helpful in some cases involving out-of-date cached plans.
- */
-static void
-tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc)
-{
-	int			i;
-
-	if (dst_tupdesc->natts != src_tupdesc->natts)
-		ereport(ERROR,
-				(errcode(ERRCODE_DATATYPE_MISMATCH),
-				 errmsg("function return row and query-specified return row do not match"),
-				 errdetail("Returned row contains %d attributes, but query expects %d.",
-						   src_tupdesc->natts, dst_tupdesc->natts)));
-
-	for (i = 0; i < dst_tupdesc->natts; i++)
-	{
-		Form_pg_attribute dattr = dst_tupdesc->attrs[i];
-		Form_pg_attribute sattr = src_tupdesc->attrs[i];
-
-		if (dattr->atttypid == sattr->atttypid)
-			continue;			/* no worries */
-		if (!dattr->attisdropped)
-			ereport(ERROR,
-					(errcode(ERRCODE_DATATYPE_MISMATCH),
-					 errmsg("function return row and query-specified return row do not match"),
-					 errdetail("Returned type %s at ordinal position %d, but query expects %s.",
-							   format_type_be(sattr->atttypid),
-							   i + 1,
-							   format_type_be(dattr->atttypid))));
-
-		if (dattr->attlen != sattr->attlen ||
-			dattr->attalign != sattr->attalign)
-			ereport(ERROR,
-					(errcode(ERRCODE_DATATYPE_MISMATCH),
-					 errmsg("function return row and query-specified return row do not match"),
-					 errdetail("Physical storage mismatch on dropped attribute at ordinal position %d.",
-							   i + 1)));
-	}
-}

src/backend/utils/fmgr/README

-$PostgreSQL: pgsql/src/backend/utils/fmgr/README,v 1.13 2008/05/15 00:17:40 tgl Exp $
+$PostgreSQL: pgsql/src/backend/utils/fmgr/README,v 1.14 2008/10/28 22:02:05 tgl Exp $
 
 Function Manager
 ================
@@ -432,8 +432,7 @@ 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;
+If available, 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.
 

src/include/executor/executor.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.149 2008/07/26 19:15:35 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.150 2008/10/28 22:02:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -176,16 +176,9 @@ extern Datum GetAttributeByNum(HeapTupleHeader tuple, AttrNumber attrno,
 				  bool *isNull);
 extern Datum GetAttributeByName(HeapTupleHeader tuple, const char *attname,
 				   bool *isNull);
-extern void init_fcache(Oid foid, FuncExprState *fcache,
-			MemoryContext fcacheCxt);
-extern Datum ExecMakeFunctionResult(FuncExprState *fcache,
-					   ExprContext *econtext,
-					   bool *isNull,
-					   ExprDoneCond *isDone);
 extern Tuplestorestate *ExecMakeTableFunctionResult(ExprState *funcexpr,
 							ExprContext *econtext,
-							TupleDesc expectedDesc,
-							TupleDesc *returnDesc);
+							TupleDesc expectedDesc);
 extern Datum ExecEvalExprSwitchContext(ExprState *expression, ExprContext *econtext,
 						  bool *isNull, ExprDoneCond *isDone);
 extern ExprState *ExecInitExpr(Expr *node, PlanState *parent);

src/include/executor/tuptable.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/executor/tuptable.h,v 1.38 2008/01/01 19:45:57 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/executor/tuptable.h,v 1.39 2008/10/28 22:02:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -160,6 +160,7 @@ extern HeapTuple ExecCopySlotTuple(TupleTableSlot *slot);
 extern MinimalTuple ExecCopySlotMinimalTuple(TupleTableSlot *slot);
 extern HeapTuple ExecFetchSlotTuple(TupleTableSlot *slot);
 extern MinimalTuple ExecFetchSlotMinimalTuple(TupleTableSlot *slot);
+extern Datum ExecFetchSlotTupleDatum(TupleTableSlot *slot);
 extern HeapTuple ExecMaterializeSlot(TupleTableSlot *slot);
 extern TupleTableSlot *ExecCopySlot(TupleTableSlot *dstslot,
 			 TupleTableSlot *srcslot);

src/include/nodes/execnodes.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.191 2008/10/23 14:34:34 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.192 2008/10/28 22:02:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -538,13 +538,29 @@ typedef struct FuncExprState
 
 	/*
 	 * Function manager's lookup info for the target function.  If func.fn_oid
-	 * is InvalidOid, we haven't initialized it yet.
+	 * is InvalidOid, we haven't initialized it yet (nor any of the following
+	 * fields).
 	 */
 	FmgrInfo	func;
 
 	/*
-	 * We also need to store argument values across calls when evaluating a
-	 * function-returning-set.
+	 * For a set-returning function (SRF) that returns a tuplestore, we
+	 * keep the tuplestore here and dole out the result rows one at a time.
+	 * The slot holds the row currently being returned.
+	 */
+	Tuplestorestate *funcResultStore;
+	TupleTableSlot *funcResultSlot;
+
+	/*
+	 * In some cases we need to compute a tuple descriptor for the function's
+	 * output.  If so, it's stored here.
+	 */
+	TupleDesc	funcResultDesc;
+	bool		funcReturnsTuple;	/* valid when funcResultDesc isn't NULL */
+
+	/*
+	 * We need to store argument values across calls when evaluating a SRF
+	 * that uses value-per-call mode.
 	 *
 	 * setArgsValid is true when we are evaluating a set-valued function and
 	 * we are in the middle of a call series; we want to pass the same
@@ -556,14 +572,15 @@ typedef struct FuncExprState
 	/*
 	 * Flag to remember whether we found a set-valued argument to the
 	 * function. This causes the function result to be a set as well. Valid
-	 * only when setArgsValid is true.
+	 * only when setArgsValid is true or funcResultStore isn't NULL.
 	 */
 	bool		setHasSetArg;	/* some argument returns a set */
 
 	/*
 	 * Flag to remember whether we have registered a shutdown callback for
-	 * this FuncExprState.	We do so only if setArgsValid has been true at
-	 * least once (since all the callback is for is to clear setArgsValid).
+	 * this FuncExprState.	We do so only if funcResultStore or setArgsValid
+	 * has been set at least once (since all the callback is for is to release
+	 * the tuplestore or clear setArgsValid).
 	 */
 	bool		shutdown_reg;	/* a shutdown callback is registered */
 

src/pl/plperl/expected/plperl.out

@@ -35,7 +35,10 @@ CREATE OR REPLACE FUNCTION perl_set_int(int) RETURNS SETOF INTEGER AS $$
 return undef;
 $$ LANGUAGE plperl;
 SELECT perl_set_int(5);
-ERROR:  set-valued function called in context that cannot accept a set
+ perl_set_int 
+--------------
+(0 rows)
+
 SELECT * FROM perl_set_int(5);
  perl_set_int 
 --------------
@@ -45,7 +48,16 @@ CREATE OR REPLACE FUNCTION perl_set_int(int) RETURNS SETOF INTEGER AS $$
 return [0..$_[0]];
 $$ LANGUAGE plperl;
 SELECT perl_set_int(5);
-ERROR:  set-valued function called in context that cannot accept a set
+ perl_set_int 
+--------------
+            0
+            1
+            2
+            3
+            4
+            5
+(6 rows)
+
 SELECT * FROM perl_set_int(5);
  perl_set_int 
 --------------
@@ -92,7 +104,10 @@ CREATE OR REPLACE FUNCTION perl_set() RETURNS SETOF testrowperl AS $$
     return undef;
 $$  LANGUAGE plperl;
 SELECT perl_set();
-ERROR:  set-valued function called in context that cannot accept a set
+ perl_set 
+----------
+(0 rows)
+
 SELECT * FROM perl_set();
  f1 | f2 | f3 
 ----+----+----
@@ -106,7 +121,7 @@ CREATE OR REPLACE FUNCTION perl_set() RETURNS SETOF testrowperl AS $$
     ];
 $$  LANGUAGE plperl;
 SELECT perl_set();
-ERROR:  set-valued function called in context that cannot accept a set
+ERROR:  setof-composite-returning Perl function must call return_next with reference to hash
 SELECT * FROM perl_set();
 ERROR:  setof-composite-returning Perl function must call return_next with reference to hash
 CREATE OR REPLACE FUNCTION perl_set() RETURNS SETOF testrowperl AS $$
@@ -117,7 +132,13 @@ CREATE OR REPLACE FUNCTION perl_set() RETURNS SETOF testrowperl AS $$
     ];
 $$  LANGUAGE plperl;
 SELECT perl_set();
-ERROR:  set-valued function called in context that cannot accept a set
+       perl_set       
+----------------------
+ (1,Hello,World)
+ (2,Hello,PostgreSQL)
+ (3,Hello,PL/Perl)
+(3 rows)
+
 SELECT * FROM perl_set();
  f1 |  f2   |     f3     
 ----+-------+------------
@@ -242,7 +263,13 @@ RETURNS SETOF record AS $$
     ];
 $$  LANGUAGE plperl;
 SELECT perl_out_params_set();
-ERROR:  set-valued function called in context that cannot accept a set
+ perl_out_params_set  
+----------------------
+ (1,Hello,World)
+ (2,Hello,PostgreSQL)
+ (3,Hello,PL/Perl)
+(3 rows)
+
 SELECT * FROM perl_out_params_set();
  f1 |  f2   |     f3     
 ----+-------+------------
@@ -252,7 +279,13 @@ SELECT * FROM perl_out_params_set();
 (3 rows)
 
 SELECT (perl_out_params_set()).f3;
-ERROR:  set-valued function called in context that cannot accept a set
+     f3     
+------------
+ World
+ PostgreSQL
+ PL/Perl
+(3 rows)
+
 --
 -- Check behavior with erroneous return values
 --