>From 7f46602167b591943fe35a85739a3a1d901aa3f6 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Thu, 25 Aug 2016 11:10:05 -0700
Subject: [PATCH 3/6] Avoid materializing SRFs in the FROM list.

So far SFRM_ValuePerCall (and ones optionally using ValuePerCall) set
returning functions have been eagerly materialized when in the from
list (i.e. when nodeFunctionscan.c is used).  In contrast to that SRFs
in the target list support ValuePerCall.  The materialization has a
significant overhead, in a number of simple example cases more than 2x
have been measured.
While that's an annoying performance difference on its own, it becomes
particularly problematic with the upcoming work to implement targetlist
SRFs via ROWS FROM.

Thus implement support for querying SFRM_ValuePerCall SRFs without
materialization.

As supporting backward scans isn't possible without materialization for
ValuePerCall, and the complication for conditionally materializing don't
seem worthwhile, instead drop backward scanning support for FunctionScan
nodes (which will, when necessary, be implemented by the planner by
using a Material node).

This moves the required support code from execQual.c to
nodeFunctionscan.c. While not a clear win now (including requiring some
duplication), it becomes more advantageous lateron, when SRF support is
removed from execQual.c entirely.

It's worthwhile to call out that the removed materialization support
implies a behavioural change in cases were some functions in ROWS FROM
are dependant on a changed PARAM_EXEC parameter, other's not. Previously
only dependent functions were recomputed, now all.

TODO:
- add some more tests
- remove ugly double gotos
- add some function header comments
---
 src/backend/executor/execAmi.c           |   2 -
 src/backend/executor/execQual.c          | 381 +----------------
 src/backend/executor/nodeFunctionscan.c  | 685 +++++++++++++++++++++++++------
 src/include/executor/executor.h          |   9 +-
 src/test/regress/expected/pg_lsn.out     |  13 +-
 src/test/regress/expected/plpgsql.out    |   2 +-
 src/test/regress/expected/rangefuncs.out |   8 +-
 src/test/regress/sql/plpgsql.sql         |   2 +-
 8 files changed, 584 insertions(+), 518 deletions(-)

diff --git a/src/backend/executor/execAmi.c b/src/backend/executor/execAmi.c
index 2587ef7..ea2f09e 100644
--- a/src/backend/executor/execAmi.c
+++ b/src/backend/executor/execAmi.c
@@ -475,7 +475,6 @@ ExecSupportsBackwardScan(Plan *node)
 
 		case T_SeqScan:
 		case T_TidScan:
-		case T_FunctionScan:
 		case T_ValuesScan:
 		case T_CteScan:
 			return TargetListSupportsBackwardScan(node->targetlist);
@@ -579,7 +578,6 @@ ExecMaterializesOutput(NodeTag plantype)
 	switch (plantype)
 	{
 		case T_Material:
-		case T_FunctionScan:
 		case T_CteScan:
 		case T_WorkTableScan:
 		case T_Sort:
diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index 743e7d6..79589d0 100644
--- a/src/backend/executor/execQual.c
+++ b/src/backend/executor/execQual.c
@@ -91,14 +91,10 @@ static Datum ExecEvalParamExec(ExprState *exprstate, ExprContext *econtext,
 				  bool *isNull, ExprDoneCond *isDone);
 static Datum ExecEvalParamExtern(ExprState *exprstate, ExprContext *econtext,
 					bool *isNull, ExprDoneCond *isDone);
-static void init_fcache(Oid foid, Oid input_collation, FuncExprState *fcache,
-			MemoryContext fcacheCxt, bool needDescForSets);
 static void ShutdownFuncExpr(Datum arg);
 static TupleDesc get_cached_rowtype(Oid type_id, int32 typmod,
 				   TupleDesc *cache_field, ExprContext *econtext);
 static void ShutdownTupleDescRef(Datum arg);
-static ExprDoneCond ExecEvalFuncArgs(FunctionCallInfo fcinfo,
-				 List *argList, ExprContext *econtext);
 static void ExecPrepareTuplestoreResult(FuncExprState *fcache,
 							ExprContext *econtext,
 							Tuplestorestate *resultStore,
@@ -1323,11 +1319,11 @@ GetAttributeByName(HeapTupleHeader tuple, const char *attname, bool *isNull)
 }
 
 /*
- * init_fcache - initialize a FuncExprState node during first use
+ * ExecInitFcache - initialize a FuncExprState node during first use
  */
-static void
-init_fcache(Oid foid, Oid input_collation, FuncExprState *fcache,
-			MemoryContext fcacheCxt, bool needDescForSets)
+void
+ExecInitFcache(Oid foid, Oid input_collation, FuncExprState *fcache,
+			   MemoryContext fcacheCxt, bool needDescForSets)
 {
 	AclResult	aclresult;
 
@@ -1503,7 +1499,7 @@ ShutdownTupleDescRef(Datum arg)
 /*
  * Evaluate arguments for a function.
  */
-static ExprDoneCond
+ExprDoneCond
 ExecEvalFuncArgs(FunctionCallInfo fcinfo,
 				 List *argList,
 				 ExprContext *econtext)
@@ -2052,353 +2048,6 @@ ExecMakeFunctionResultNoSets(FuncExprState *fcache,
 }
 
 
-/*
- *		ExecMakeTableFunctionResult
- *
- * Evaluate a table function, producing a materialized result in a Tuplestore
- * object.
- */
-Tuplestorestate *
-ExecMakeTableFunctionResult(ExprState *funcexpr,
-							ExprContext *econtext,
-							MemoryContext argContext,
-							TupleDesc expectedDesc,
-							bool randomAccess)
-{
-	Tuplestorestate *tupstore = NULL;
-	TupleDesc	tupdesc = NULL;
-	Oid			funcrettype;
-	bool		returnsTuple;
-	bool		returnsSet = false;
-	FunctionCallInfoData fcinfo;
-	PgStat_FunctionCallUsage fcusage;
-	ReturnSetInfo rsinfo;
-	HeapTupleData tmptup;
-	MemoryContext callerContext;
-	MemoryContext oldcontext;
-	bool		direct_function_call;
-	bool		first_time = true;
-
-	callerContext = CurrentMemoryContext;
-
-	funcrettype = exprType((Node *) funcexpr->expr);
-
-	returnsTuple = type_is_rowtype(funcrettype);
-
-	/*
-	 * Prepare a resultinfo node for communication.  We always do this even if
-	 * not expecting a set result, so that we can pass expectedDesc.  In the
-	 * generic-expression case, the expression doesn't actually get to see the
-	 * resultinfo, but set it up anyway because we use some of the fields as
-	 * our own state variables.
-	 */
-	rsinfo.type = T_ReturnSetInfo;
-	rsinfo.econtext = econtext;
-	rsinfo.expectedDesc = expectedDesc;
-	rsinfo.allowedModes = (int) (SFRM_ValuePerCall | SFRM_Materialize | SFRM_Materialize_Preferred);
-	if (randomAccess)
-		rsinfo.allowedModes |= (int) SFRM_Materialize_Random;
-	rsinfo.returnMode = SFRM_ValuePerCall;
-	/* isDone is filled below */
-	rsinfo.setResult = NULL;
-	rsinfo.setDesc = NULL;
-
-	/*
-	 * Normally the passed expression tree will be a FuncExprState, since the
-	 * grammar only allows a function call at the top level of a table
-	 * function reference.  However, if the function doesn't return set then
-	 * the planner might have replaced the function call via constant-folding
-	 * or inlining.  So if we see any other kind of expression node, execute
-	 * it via the general ExecEvalExpr() code; the only difference is that we
-	 * don't get a chance to pass a special ReturnSetInfo to any functions
-	 * buried in the expression.
-	 */
-	if (funcexpr && IsA(funcexpr, FuncExprState) &&
-		IsA(funcexpr->expr, FuncExpr))
-	{
-		FuncExprState *fcache = (FuncExprState *) funcexpr;
-		ExprDoneCond argDone;
-
-		/*
-		 * This path is similar to ExecMakeFunctionResult.
-		 */
-		direct_function_call = true;
-
-		/*
-		 * Initialize function cache if first time through
-		 */
-		if (fcache->func.fn_oid == InvalidOid)
-		{
-			FuncExpr   *func = (FuncExpr *) fcache->xprstate.expr;
-
-			init_fcache(func->funcid, func->inputcollid, fcache,
-						econtext->ecxt_per_query_memory, false);
-		}
-		returnsSet = fcache->func.fn_retset;
-		InitFunctionCallInfoData(fcinfo, &(fcache->func),
-								 list_length(fcache->args),
-								 fcache->fcinfo_data.fncollation,
-								 NULL, (Node *) &rsinfo);
-
-		/*
-		 * Evaluate the function's argument list.
-		 *
-		 * We can't do this in the per-tuple context: the argument values
-		 * would disappear when we reset that context in the inner loop.  And
-		 * the caller's CurrentMemoryContext is typically a query-lifespan
-		 * context, so we don't want to leak memory there.  We require the
-		 * caller to pass a separate memory context that can be used for this,
-		 * and can be reset each time through to avoid bloat.
-		 */
-		MemoryContextReset(argContext);
-		oldcontext = MemoryContextSwitchTo(argContext);
-		argDone = ExecEvalFuncArgs(&fcinfo, fcache->args, econtext);
-		MemoryContextSwitchTo(oldcontext);
-
-		/* We don't allow sets in the arguments of the table function */
-		if (argDone != ExprSingleResult)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("set-valued function called in context that cannot accept a set")));
-
-		/*
-		 * If function is strict, and there are any NULL arguments, skip
-		 * calling the function and act like it returned NULL (or an empty
-		 * set, in the returns-set case).
-		 */
-		if (fcache->func.fn_strict)
-		{
-			int			i;
-
-			for (i = 0; i < fcinfo.nargs; i++)
-			{
-				if (fcinfo.argnull[i])
-					goto no_function_result;
-			}
-		}
-	}
-	else
-	{
-		/* Treat funcexpr as a generic expression */
-		direct_function_call = false;
-		InitFunctionCallInfoData(fcinfo, NULL, 0, InvalidOid, NULL, NULL);
-	}
-
-	/*
-	 * Switch to short-lived context for calling the function or expression.
-	 */
-	MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
-
-	/*
-	 * Loop to handle the ValuePerCall protocol (which is also the same
-	 * behavior needed in the generic ExecEvalExpr path).
-	 */
-	for (;;)
-	{
-		Datum		result;
-
-		CHECK_FOR_INTERRUPTS();
-
-		/*
-		 * reset per-tuple memory context before each call of the function or
-		 * expression. This cleans up any local memory the function may leak
-		 * when called.
-		 */
-		ResetExprContext(econtext);
-
-		/* Call the function or expression one time */
-		if (direct_function_call)
-		{
-			pgstat_init_function_usage(&fcinfo, &fcusage);
-
-			fcinfo.isnull = false;
-			rsinfo.isDone = ExprSingleResult;
-			result = FunctionCallInvoke(&fcinfo);
-
-			pgstat_end_function_usage(&fcusage,
-									  rsinfo.isDone != ExprMultipleResult);
-		}
-		else
-		{
-			result = ExecEvalExpr(funcexpr, econtext,
-								  &fcinfo.isnull, &rsinfo.isDone);
-		}
-
-		/* Which protocol does function want to use? */
-		if (rsinfo.returnMode == SFRM_ValuePerCall)
-		{
-			/*
-			 * Check for end of result set.
-			 */
-			if (rsinfo.isDone == ExprEndResult)
-				break;
-
-			/*
-			 * If first time through, build tuplestore for result.  For a
-			 * scalar function result type, also make a suitable tupdesc.
-			 */
-			if (first_time)
-			{
-				oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
-				tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
-				rsinfo.setResult = tupstore;
-				if (!returnsTuple)
-				{
-					tupdesc = CreateTemplateTupleDesc(1, false);
-					TupleDescInitEntry(tupdesc,
-									   (AttrNumber) 1,
-									   "column",
-									   funcrettype,
-									   -1,
-									   0);
-					rsinfo.setDesc = tupdesc;
-				}
-				MemoryContextSwitchTo(oldcontext);
-			}
-
-			/*
-			 * Store current resultset item.
-			 */
-			if (returnsTuple)
-			{
-				if (!fcinfo.isnull)
-				{
-					HeapTupleHeader td = DatumGetHeapTupleHeader(result);
-
-					if (tupdesc == NULL)
-					{
-						/*
-						 * This is the first non-NULL result from the
-						 * function.  Use the type info embedded in the
-						 * rowtype Datum to look up the needed tupdesc.  Make
-						 * a copy for the query.
-						 */
-						oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
-						tupdesc = lookup_rowtype_tupdesc_copy(HeapTupleHeaderGetTypeId(td),
-											   HeapTupleHeaderGetTypMod(td));
-						rsinfo.setDesc = tupdesc;
-						MemoryContextSwitchTo(oldcontext);
-					}
-					else
-					{
-						/*
-						 * Verify all later returned rows have same subtype;
-						 * necessary in case the type is RECORD.
-						 */
-						if (HeapTupleHeaderGetTypeId(td) != tupdesc->tdtypeid ||
-							HeapTupleHeaderGetTypMod(td) != tupdesc->tdtypmod)
-							ereport(ERROR,
-									(errcode(ERRCODE_DATATYPE_MISMATCH),
-									 errmsg("rows returned by function are not all of the same row type")));
-					}
-
-					/*
-					 * 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;
-
-					tuplestore_puttuple(tupstore, &tmptup);
-				}
-				else
-				{
-					/*
-					 * NULL result from a tuple-returning function; expand it
-					 * to a row of all nulls.  We rely on the expectedDesc to
-					 * form such rows.  (Note: this would be problematic if
-					 * tuplestore_putvalues saved the tdtypeid/tdtypmod from
-					 * the provided descriptor, since that might not match
-					 * what we get from the function itself.  But it doesn't.)
-					 */
-					int			natts = expectedDesc->natts;
-					bool	   *nullflags;
-
-					nullflags = (bool *) palloc(natts * sizeof(bool));
-					memset(nullflags, true, natts * sizeof(bool));
-					tuplestore_putvalues(tupstore, expectedDesc, NULL, nullflags);
-				}
-			}
-			else
-			{
-				/* Scalar-type case: just store the function result */
-				tuplestore_putvalues(tupstore, tupdesc, &result, &fcinfo.isnull);
-			}
-
-			/*
-			 * Are we done?
-			 */
-			if (rsinfo.isDone != ExprMultipleResult)
-				break;
-		}
-		else if (rsinfo.returnMode == SFRM_Materialize)
-		{
-			/* check we're on the same page as the function author */
-			if (!first_time || rsinfo.isDone != ExprSingleResult)
-				ereport(ERROR,
-						(errcode(ERRCODE_E_R_I_E_SRF_PROTOCOL_VIOLATED),
-						 errmsg("table-function protocol for materialize mode was not followed")));
-			/* Done evaluating the set result */
-			break;
-		}
-		else
-			ereport(ERROR,
-					(errcode(ERRCODE_E_R_I_E_SRF_PROTOCOL_VIOLATED),
-					 errmsg("unrecognized table-function returnMode: %d",
-							(int) rsinfo.returnMode)));
-
-		first_time = false;
-	}
-
-no_function_result:
-
-	/*
-	 * If we got nothing from the function (ie, an empty-set or NULL result),
-	 * we have to create the tuplestore to return, and if it's a
-	 * non-set-returning function then insert a single all-nulls row.  As
-	 * above, we depend on the expectedDesc to manufacture the dummy row.
-	 */
-	if (rsinfo.setResult == NULL)
-	{
-		MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
-		tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
-		rsinfo.setResult = tupstore;
-		if (!returnsSet)
-		{
-			int			natts = expectedDesc->natts;
-			bool	   *nullflags;
-
-			MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
-			nullflags = (bool *) palloc(natts * sizeof(bool));
-			memset(nullflags, true, natts * sizeof(bool));
-			tuplestore_putvalues(tupstore, expectedDesc, NULL, nullflags);
-		}
-	}
-
-	/*
-	 * 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 (rsinfo.setDesc)
-	{
-		tupledesc_match(expectedDesc, rsinfo.setDesc);
-
-		/*
-		 * If it is a dynamically-allocated TupleDesc, free it: it is
-		 * typically allocated in a per-query context, so we must avoid
-		 * leaking it across multiple usages.
-		 */
-		if (rsinfo.setDesc->tdrefcount == -1)
-			FreeTupleDesc(rsinfo.setDesc);
-	}
-
-	MemoryContextSwitchTo(callerContext);
-
-	/* All done, pass back the tuplestore */
-	return rsinfo.setResult;
-}
-
-
 /* ----------------------------------------------------------------
  *		ExecEvalFunc
  *		ExecEvalOper
@@ -2422,8 +2071,8 @@ ExecEvalFunc(FuncExprState *fcache,
 	FuncExpr   *func = (FuncExpr *) fcache->xprstate.expr;
 
 	/* Initialize function lookup info */
-	init_fcache(func->funcid, func->inputcollid, fcache,
-				econtext->ecxt_per_query_memory, true);
+	ExecInitFcache(func->funcid, func->inputcollid, fcache,
+				   econtext->ecxt_per_query_memory, true);
 
 	/*
 	 * We need to invoke ExecMakeFunctionResult if either the function itself
@@ -2457,8 +2106,8 @@ ExecEvalOper(FuncExprState *fcache,
 	OpExpr	   *op = (OpExpr *) fcache->xprstate.expr;
 
 	/* Initialize function lookup info */
-	init_fcache(op->opfuncid, op->inputcollid, fcache,
-				econtext->ecxt_per_query_memory, true);
+	ExecInitFcache(op->opfuncid, op->inputcollid, fcache,
+				   econtext->ecxt_per_query_memory, true);
 
 	/*
 	 * We need to invoke ExecMakeFunctionResult if either the function itself
@@ -2511,8 +2160,8 @@ ExecEvalDistinct(FuncExprState *fcache,
 	{
 		DistinctExpr *op = (DistinctExpr *) fcache->xprstate.expr;
 
-		init_fcache(op->opfuncid, op->inputcollid, fcache,
-					econtext->ecxt_per_query_memory, true);
+		ExecInitFcache(op->opfuncid, op->inputcollid, fcache,
+					   econtext->ecxt_per_query_memory, true);
 		Assert(!fcache->func.fn_retset);
 	}
 
@@ -2588,8 +2237,8 @@ ExecEvalScalarArrayOp(ScalarArrayOpExprState *sstate,
 	 */
 	if (sstate->fxprstate.func.fn_oid == InvalidOid)
 	{
-		init_fcache(opexpr->opfuncid, opexpr->inputcollid, &sstate->fxprstate,
-					econtext->ecxt_per_query_memory, true);
+		ExecInitFcache(opexpr->opfuncid, opexpr->inputcollid, &sstate->fxprstate,
+					   econtext->ecxt_per_query_memory, true);
 		Assert(!sstate->fxprstate.func.fn_retset);
 	}
 
@@ -3850,8 +3499,8 @@ ExecEvalNullIf(FuncExprState *nullIfExpr,
 	{
 		NullIfExpr *op = (NullIfExpr *) nullIfExpr->xprstate.expr;
 
-		init_fcache(op->opfuncid, op->inputcollid, nullIfExpr,
-					econtext->ecxt_per_query_memory, true);
+		ExecInitFcache(op->opfuncid, op->inputcollid, nullIfExpr,
+					   econtext->ecxt_per_query_memory, true);
 		Assert(!nullIfExpr->func.fn_retset);
 	}
 
diff --git a/src/backend/executor/nodeFunctionscan.c b/src/backend/executor/nodeFunctionscan.c
index a03f6e7..4885f75 100644
--- a/src/backend/executor/nodeFunctionscan.c
+++ b/src/backend/executor/nodeFunctionscan.c
@@ -22,12 +22,20 @@
  */
 #include "postgres.h"
 
+#include "access/htup_details.h"
 #include "catalog/pg_type.h"
 #include "executor/nodeFunctionscan.h"
 #include "funcapi.h"
+#include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
+#include "parser/parse_coerce.h"
+#include "pgstat.h"
 #include "utils/builtins.h"
+#include "utils/expandeddatum.h"
+#include "utils/lsyscache.h"
+#include "utils/typcache.h"
 #include "utils/memutils.h"
+#include "utils/tuplestore.h"
 
 
 /*
@@ -39,12 +47,19 @@ typedef struct FunctionScanPerFuncState
 	TupleDesc	tupdesc;		/* desc of the function result type */
 	int			colcount;		/* expected number of result columns */
 	Tuplestorestate *tstore;	/* holds the function result set */
-	int64		rowcount;		/* # of rows in result set, -1 if not known */
 	TupleTableSlot *func_slot;	/* function result slot (or NULL) */
+	bool		started;
+	bool		returnsTuple;
+	FunctionCallInfoData fcinfo;
+	ReturnSetInfo rsinfo;
 } FunctionScanPerFuncState;
 
 static TupleTableSlot *FunctionNext(FunctionScanState *node);
-
+static void ExecBeginFunctionResult(FunctionScanState *node,
+												FunctionScanPerFuncState *perfunc);
+static void ExecNextFunctionResult(FunctionScanState *node,
+											   FunctionScanPerFuncState *perfunc);
+static void tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc);
 
 /* ----------------------------------------------------------------
  *						Scan Support
@@ -63,7 +78,6 @@ FunctionNext(FunctionScanState *node)
 	ScanDirection direction;
 	TupleTableSlot *scanslot;
 	bool		alldone;
-	int64		oldpos;
 	int			funcno;
 	int			att;
 
@@ -74,59 +88,39 @@ FunctionNext(FunctionScanState *node)
 	direction = estate->es_direction;
 	scanslot = node->ss.ss_ScanTupleSlot;
 
+	Assert(ScanDirectionIsForward(direction));
+
 	if (node->simple)
 	{
 		/*
 		 * Fast path for the trivial case: the function return type and scan
 		 * result type are the same, so we fetch the function result straight
-		 * into the scan result slot. No need to update ordinality or
-		 * rowcounts either.
+		 * into the scan result slot. No need to update ordinality either.
 		 */
-		Tuplestorestate *tstore = node->funcstates[0].tstore;
+		FunctionScanPerFuncState *fs = &node->funcstates[0];
 
 		/*
-		 * If first time through, read all tuples from function and put them
-		 * in a tuplestore. Subsequent calls just fetch tuples from
-		 * tuplestore.
+		 * If first time through, call the SRF. Subsequent calls read from a
+		 * tuplestore (for SFRM_Materialize) or call the function again (if
+		 * SFRM_ValuePerCall).
 		 */
-		if (tstore == NULL)
-		{
-			node->funcstates[0].tstore = tstore =
-				ExecMakeTableFunctionResult(node->funcstates[0].funcexpr,
-											node->ss.ps.ps_ExprContext,
-											node->argcontext,
-											node->funcstates[0].tupdesc,
-										  node->eflags & EXEC_FLAG_BACKWARD);
+		if (!fs->started)
+			ExecBeginFunctionResult(node, fs);
+		else
+			ExecNextFunctionResult(node, fs);
 
-			/*
-			 * paranoia - cope if the function, which may have constructed the
-			 * tuplestore itself, didn't leave it pointing at the start. This
-			 * call is fast, so the overhead shouldn't be an issue.
-			 */
-			tuplestore_rescan(tstore);
-		}
+		scanslot = fs->func_slot;
 
-		/*
-		 * Get the next tuple from tuplestore.
-		 */
-		(void) tuplestore_gettupleslot(tstore,
-									   ScanDirectionIsForward(direction),
-									   false,
-									   scanslot);
 		return scanslot;
 	}
 
 	/*
-	 * Increment or decrement ordinal counter before checking for end-of-data,
-	 * so that we can move off either end of the result by 1 (and no more than
-	 * 1) without losing correct count.  See PortalRunSelect for why we can
-	 * assume that we won't be called repeatedly in the end-of-data state.
+	 * Increment ordinal counter before checking for end-of-data, so that we
+	 * can move off the end of the result by 1 (and no more than 1) without
+	 * losing correct count.  See PortalRunSelect for why we can assume that
+	 * we won't be called repeatedly in the end-of-data state.
 	 */
-	oldpos = node->ordinal;
-	if (ScanDirectionIsForward(direction))
-		node->ordinal++;
-	else
-		node->ordinal--;
+	node->ordinal++;
 
 	/*
 	 * Main loop over functions.
@@ -144,55 +138,18 @@ FunctionNext(FunctionScanState *node)
 		int			i;
 
 		/*
-		 * If first time through, read all tuples from function and put them
-		 * in a tuplestore. Subsequent calls just fetch tuples from
-		 * tuplestore.
+		 * If first time through, call the SRF. Subsequent calls read from a
+		 * tuplestore (for SFRM_Materialize) or call the function again (if
+		 * SFRM_ValuePerCall).
 		 */
-		if (fs->tstore == NULL)
-		{
-			fs->tstore =
-				ExecMakeTableFunctionResult(fs->funcexpr,
-											node->ss.ps.ps_ExprContext,
-											node->argcontext,
-											fs->tupdesc,
-										  node->eflags & EXEC_FLAG_BACKWARD);
-
-			/*
-			 * paranoia - cope if the function, which may have constructed the
-			 * tuplestore itself, didn't leave it pointing at the start. This
-			 * call is fast, so the overhead shouldn't be an issue.
-			 */
-			tuplestore_rescan(fs->tstore);
-		}
-
-		/*
-		 * Get the next tuple from tuplestore.
-		 *
-		 * If we have a rowcount for the function, and we know the previous
-		 * read position was out of bounds, don't try the read. This allows
-		 * backward scan to work when there are mixed row counts present.
-		 */
-		if (fs->rowcount != -1 && fs->rowcount < oldpos)
-			ExecClearTuple(fs->func_slot);
+		if (!fs->started)
+			ExecBeginFunctionResult(node, fs);
 		else
-			(void) tuplestore_gettupleslot(fs->tstore,
-										   ScanDirectionIsForward(direction),
-										   false,
-										   fs->func_slot);
+			ExecNextFunctionResult(node, fs);
 
 		if (TupIsNull(fs->func_slot))
 		{
 			/*
-			 * If we ran out of data for this function in the forward
-			 * direction then we now know how many rows it returned. We need
-			 * to know this in order to handle backwards scans. The row count
-			 * we store is actually 1+ the actual number, because we have to
-			 * position the tuplestore 1 off its end sometimes.
-			 */
-			if (ScanDirectionIsForward(direction) && fs->rowcount == -1)
-				fs->rowcount = node->ordinal;
-
-			/*
 			 * populate the result cols with nulls
 			 */
 			for (i = 0; i < fs->colcount; i++)
@@ -307,21 +264,12 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, int eflags)
 	scanstate->ordinality = node->funcordinality;
 
 	scanstate->nfuncs = nfuncs;
-	if (nfuncs == 1 && !node->funcordinality)
-		scanstate->simple = true;
-	else
+	if (nfuncs > 1 || node->funcordinality)
 		scanstate->simple = false;
+	else
+		scanstate->simple = true;
 
-	/*
-	 * Ordinal 0 represents the "before the first row" position.
-	 *
-	 * We need to track ordinal position even when not adding an ordinality
-	 * column to the result, in order to handle backwards scanning properly
-	 * with multiple functions with different result sizes. (We can't position
-	 * any individual function's tuplestore any more than 1 place beyond its
-	 * end, so when scanning backwards, we need to know when to start
-	 * including the function in the scan again.)
-	 */
+	/* ordinal 0 represents the "before the first row" position */
 	scanstate->ordinal = 0;
 
 	/*
@@ -367,11 +315,12 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, int eflags)
 
 		/*
 		 * Don't allocate the tuplestores; the actual calls to the functions
-		 * do that.  NULL means that we have not called the function yet (or
-		 * need to call it again after a rescan).
+		 * do that if necessary.  started = false means that we have not
+		 * called the function yet (or need to call it again after a rescan).
 		 */
 		fs->tstore = NULL;
-		fs->rowcount = -1;
+		fs->started = false;
+		fs->rsinfo.setDesc = NULL;
 
 		/*
 		 * Now determine if the function returns a simple or composite type,
@@ -390,6 +339,7 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, int eflags)
 			Assert(tupdesc->natts >= colcount);
 			/* Must copy it out of typcache for safety */
 			tupdesc = CreateTupleDescCopy(tupdesc);
+			fs->returnsTuple = true;
 		}
 		else if (functypclass == TYPEFUNC_SCALAR)
 		{
@@ -404,6 +354,7 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, int eflags)
 			TupleDescInitEntryCollation(tupdesc,
 										(AttrNumber) 1,
 										exprCollation(funcexpr));
+			fs->returnsTuple = false;
 		}
 		else if (functypclass == TYPEFUNC_RECORD)
 		{
@@ -418,6 +369,7 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, int eflags)
 			 * case it doesn't.)
 			 */
 			BlessTupleDesc(tupdesc);
+			fs->returnsTuple = true;
 		}
 		else
 		{
@@ -439,7 +391,7 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, int eflags)
 			ExecSetSlotDescriptor(fs->func_slot, fs->tupdesc);
 		}
 		else
-			fs->func_slot = NULL;
+			fs->func_slot = scanstate->ss.ss_ScanTupleSlot;
 
 		natts += colcount;
 		i++;
@@ -500,11 +452,13 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, int eflags)
 	ExecAssignScanProjectionInfo(&scanstate->ss);
 
 	/*
-	 * Create a memory context that ExecMakeTableFunctionResult can use to
-	 * evaluate function arguments in.  We can't use the per-tuple context for
-	 * this because it gets reset too often; but we don't want to leak
-	 * evaluation results into the query-lifespan context either.  We just
-	 * need one context, because we evaluate each function separately.
+	 * Create a memory context that is used to evaluate function arguments in.
+	 * We can't use the per-tuple context for this because it gets reset too
+	 * often; but we don't want to leak evaluation results into the
+	 * query-lifespan context either.  We currently just use one context for
+	 * all functions, they're evaluated at the same time anyway - most of the
+	 * time creating separate contexts would use more memory, than being able
+	 * to reset separately would save.
 	 */
 	scanstate->argcontext = AllocSetContextCreate(CurrentMemoryContext,
 												  "Table function arguments",
@@ -564,58 +518,523 @@ ExecEndFunctionScan(FunctionScanState *node)
 void
 ExecReScanFunctionScan(FunctionScanState *node)
 {
-	FunctionScan *scan = (FunctionScan *) node->ss.ps.plan;
-	int			i;
-	Bitmapset  *chgparam = node->ss.ps.chgParam;
+	int i;
 
 	ExecClearTuple(node->ss.ps.ps_ResultTupleSlot);
+
 	for (i = 0; i < node->nfuncs; i++)
 	{
 		FunctionScanPerFuncState *fs = &node->funcstates[i];
 
 		if (fs->func_slot)
 			ExecClearTuple(fs->func_slot);
+
+		if (node->funcstates[i].tstore != NULL)
+		{
+			tuplestore_end(node->funcstates[i].tstore);
+			node->funcstates[i].tstore = NULL;
+		}
+
+		/*
+		 * If it is a dynamically-allocated TupleDesc, free it: it is
+		 * typically allocated in a per-query context, so we must avoid
+		 * leaking it across multiple usages.
+		 */
+		if (fs->rsinfo.setDesc && fs->rsinfo.setDesc->tdrefcount == -1)
+		{
+			FreeTupleDesc(fs->rsinfo.setDesc);
+			fs->rsinfo.setDesc = NULL;
+		}
+
+		fs->started = false;
 	}
 
 	ExecScanReScan(&node->ss);
 
+	/* Reset ordinality counter */
+	node->ordinal = 0;
+}
+
+
+static void
+ExecBeginFunctionResult(FunctionScanState *node,
+						FunctionScanPerFuncState *perfunc)
+{
+	bool		returnsSet = false;
+	MemoryContext callerContext;
+	MemoryContext oldcontext;
+	bool		direct_function_call;
+	ExprContext *econtext = node->ss.ps.ps_ExprContext;
+	ExprState *funcexpr = perfunc->funcexpr;
+	Datum		result;
+
+	callerContext = CurrentMemoryContext;
+
+	Assert(perfunc->tupdesc != NULL);
+
 	/*
-	 * Here we have a choice whether to drop the tuplestores (and recompute
-	 * the function outputs) or just rescan them.  We must recompute if an
-	 * expression contains changed parameters, else we rescan.
-	 *
-	 * XXX maybe we should recompute if the function is volatile?  But in
-	 * general the executor doesn't conditionalize its actions on that.
+	 * Prepare a resultinfo node for communication.  We always do this even if
+	 * not expecting a set result, so that we can pass expectedDesc.  In the
+	 * generic-expression case, the expression doesn't actually get to see the
+	 * resultinfo, but set it up anyway because we use some of the fields as
+	 * our own state variables.
 	 */
-	if (chgparam)
+	perfunc->rsinfo.type = T_ReturnSetInfo;
+	perfunc->rsinfo.econtext = econtext;
+	perfunc->rsinfo.expectedDesc = perfunc->tupdesc;
+	perfunc->rsinfo.allowedModes = (int) (SFRM_ValuePerCall | SFRM_Materialize);
+	perfunc->rsinfo.returnMode = SFRM_ValuePerCall;
+	/* isDone is filled below */
+	perfunc->rsinfo.setResult = NULL;
+	perfunc->rsinfo.setDesc = NULL;
+	perfunc->tstore = NULL;
+
+	perfunc->started = true;
+
+	/*
+	 * Normally the passed expression tree will be a FuncExprState, since the
+	 * grammar only allows a function call at the top level of a table
+	 * function reference.  However, if the function doesn't return set then
+	 * the planner might have replaced the function call via constant-folding
+	 * or inlining.  So if we see any other kind of expression node, execute
+	 * it via the general ExecEvalExpr() code; the only difference is that we
+	 * don't get a chance to pass a special ReturnSetInfo to any functions
+	 * buried in the expression.
+	 */
+	if (funcexpr && IsA(funcexpr, FuncExprState) &&
+		IsA(funcexpr->expr, FuncExpr))
 	{
-		ListCell   *lc;
+		FuncExprState *fcache = (FuncExprState *) funcexpr;
+		ExprDoneCond argDone;
 
-		i = 0;
-		foreach(lc, scan->functions)
+		/*
+		 * This path is similar to ExecMakeFunctionResult.
+		 */
+		direct_function_call = true;
+
+		/*
+		 * Initialize function cache if first time through
+		 */
+		if (fcache->func.fn_oid == InvalidOid)
 		{
-			RangeTblFunction *rtfunc = (RangeTblFunction *) lfirst(lc);
+			FuncExpr   *func = (FuncExpr *) fcache->xprstate.expr;
 
-			if (bms_overlap(chgparam, rtfunc->funcparams))
+			ExecInitFcache(func->funcid, func->inputcollid, fcache,
+						   econtext->ecxt_per_query_memory, false);
+		}
+		returnsSet = fcache->func.fn_retset;
+		InitFunctionCallInfoData(perfunc->fcinfo, &(fcache->func),
+								 list_length(fcache->args),
+								 fcache->fcinfo_data.fncollation,
+								 NULL, (Node *) &perfunc->rsinfo);
+
+		/*
+		 * Evaluate the function's argument list.
+		 *
+		 * We can't do this in the per-tuple context: the argument values
+		 * would disappear when we reset that context in the inner loop.  And
+		 * the caller's CurrentMemoryContext is typically a query-lifespan
+		 * context, so we don't want to leak memory there.  We require the
+		 * caller to pass a separate memory context that can be used for this,
+		 * and can be reset each time the node is re-scanned.
+		 */
+		oldcontext = MemoryContextSwitchTo(node->argcontext);
+		argDone = ExecEvalFuncArgs(&perfunc->fcinfo, fcache->args, econtext);
+		MemoryContextSwitchTo(oldcontext);
+
+		/* We don't allow sets in the arguments of the table function */
+		if (argDone != ExprSingleResult)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("set-valued function called in context that cannot accept a set")));
+
+		/*
+		 * If function is strict, and there are any NULL arguments, skip
+		 * calling the function and act like it returned NULL (or an empty
+		 * set, in the returns-set case).
+		 */
+		if (fcache->func.fn_strict)
+		{
+			int			i;
+
+			for (i = 0; i < perfunc->fcinfo.nargs; i++)
 			{
-				if (node->funcstates[i].tstore != NULL)
-				{
-					tuplestore_end(node->funcstates[i].tstore);
-					node->funcstates[i].tstore = NULL;
-				}
-				node->funcstates[i].rowcount = -1;
+				if (perfunc->fcinfo.argnull[i])
+					goto no_function_result;
 			}
-			i++;
+		}
+	}
+	else
+	{
+		/* Treat funcexpr as a generic expression */
+		direct_function_call = false;
+		InitFunctionCallInfoData(perfunc->fcinfo, NULL, 0, InvalidOid, NULL, NULL);
+	}
+
+	/*
+	 * Switch to short-lived context for calling the function or expression.
+	 */
+	MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
+
+	/*
+	 * reset per-tuple memory context before each call of the function or
+	 * expression. This cleans up any local memory the function may leak
+	 * when called.
+	 */
+	ResetExprContext(econtext);
+
+	/* Call the function or expression one time */
+	if (direct_function_call)
+	{
+		PgStat_FunctionCallUsage fcusage;
+
+		pgstat_init_function_usage(&perfunc->fcinfo, &fcusage);
+
+		perfunc->fcinfo.isnull = false;
+		perfunc->rsinfo.isDone = ExprSingleResult;
+		result = FunctionCallInvoke(&perfunc->fcinfo);
+
+		pgstat_end_function_usage(&fcusage,
+								  perfunc->rsinfo.isDone != ExprMultipleResult);
+	}
+	else
+	{
+		perfunc->rsinfo.isDone = ExprSingleResult;
+		result = ExecEvalExpr(funcexpr, econtext,
+							  &perfunc->fcinfo.isnull, NULL);
+
+		/* done after this, will use SFRM_ValuePerCall branch below */
+	}
+
+	/* Which protocol does function want to use? */
+	if (perfunc->rsinfo.returnMode == SFRM_ValuePerCall)
+	{
+		/*
+		 * Check for end of result set.
+		 */
+		if (perfunc->rsinfo.isDone == ExprEndResult)
+			goto no_function_result;
+
+		/*
+		 * Store current resultset item.
+		 */
+		if (perfunc->returnsTuple)
+		{
+			if (!perfunc->fcinfo.isnull)
+			{
+				HeapTupleHeader td = DatumGetHeapTupleHeader(result);
+				HeapTupleData tmptup;
+
+				if (perfunc->rsinfo.setDesc == NULL)
+				{
+					/*
+					 * This is the first non-NULL result from the
+					 * function.  Use the type info embedded in the
+					 * rowtype Datum to look up the needed tupdesc.  Make
+					 * a copy for the query.
+					 */
+					oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
+					perfunc->rsinfo.setDesc =
+						lookup_rowtype_tupdesc_copy(HeapTupleHeaderGetTypeId(td),
+													HeapTupleHeaderGetTypMod(td));
+					MemoryContextSwitchTo(oldcontext);
+
+					/*
+					 * Cross-check tupdesc.  We only really need to do this
+					 * for functions returning RECORD, but might as well do it
+					 * always.
+					 */
+					tupledesc_match(perfunc->tupdesc, perfunc->rsinfo.setDesc);
+				}
+
+				tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
+				tmptup.t_data = td;
+
+				ExecStoreTuple(&tmptup, perfunc->func_slot, InvalidBuffer, false);
+				/* materializing handles expanded and toasted datums */
+				/* XXX: would be nice if this could be optimized away */
+				ExecMaterializeSlot(perfunc->func_slot);
+			}
+			else
+			{
+				/*
+				 * NULL result from a tuple-returning function; expand it
+				 * to a row of all nulls.
+				 */
+				ExecStoreAllNullTuple(perfunc->func_slot);
+			}
+		}
+		else
+		{
+			/*
+			 * Scalar-type case: just store the function result
+			 */
+			ExecClearTuple(perfunc->func_slot);
+			perfunc->func_slot->tts_values[0] = result;
+			perfunc->func_slot->tts_isnull[0] = perfunc->fcinfo.isnull;
+			ExecStoreVirtualTuple(perfunc->func_slot);
+
+			/* materializing handles expanded and toasted datums */
+			ExecMaterializeSlot(perfunc->func_slot);
+		}
+	}
+	else if (perfunc->rsinfo.returnMode == SFRM_Materialize)
+	{
+		EState	   *estate;
+		ScanDirection direction;
+
+		estate = node->ss.ps.state;
+		direction = estate->es_direction;
+
+		/* check we're on the same page as the function author */
+		if (perfunc->rsinfo.isDone != ExprSingleResult)
+			ereport(ERROR,
+					(errcode(ERRCODE_E_R_I_E_SRF_PROTOCOL_VIOLATED),
+					 errmsg("table-function protocol for materialize mode was not followed")));
+
+		if (perfunc->rsinfo.setResult != NULL)
+		{
+			perfunc->tstore = perfunc->rsinfo.setResult;
+
+			/*
+			 * paranoia - cope if the function, which may have constructed the
+			 * tuplestore itself, didn't leave it pointing at the start. This
+			 * call is fast, so the overhead shouldn't be an issue.
+			 */
+			tuplestore_rescan(perfunc->rsinfo.setResult);
+
+			/*
+			 * 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 (perfunc->rsinfo.setDesc)
+			{
+				tupledesc_match(perfunc->tupdesc, perfunc->rsinfo.setDesc);
+
+				/*
+				 * If it is a dynamically-allocated TupleDesc, free it: it is
+				 * typically allocated in a per-query context, so we must avoid
+				 * leaking it across multiple usages.
+				 */
+				if (perfunc->rsinfo.setDesc->tdrefcount == -1)
+				{
+					FreeTupleDesc(perfunc->rsinfo.setDesc);
+					perfunc->rsinfo.setDesc = NULL;
+				}
+			}
+
+			/* and return first row */
+			(void) tuplestore_gettupleslot(perfunc->rsinfo.setResult,
+										   ScanDirectionIsForward(direction),
+										   false,
+										   perfunc->func_slot);
+		}
+	}
+	else
+		ereport(ERROR,
+				(errcode(ERRCODE_E_R_I_E_SRF_PROTOCOL_VIOLATED),
+				 errmsg("unrecognized table-function returnMode: %d",
+						(int) perfunc->rsinfo.returnMode)));
+	goto done;
+
+no_function_result:
+	MemoryContextSwitchTo(callerContext);
+
+	/*
+	 * If we got nothing from the function (ie, an empty-set or NULL
+	 * result), we have to manufacture a result. I.e. if it's a
+	 * non-set-returning function then return a single all-nulls row.
+	 */
+	perfunc->rsinfo.isDone = ExprEndResult;
+	if (returnsSet)
+		ExecClearTuple(perfunc->func_slot);
+	else
+		ExecStoreAllNullTuple(perfunc->func_slot);
+done:
+	MemoryContextSwitchTo(callerContext);
+}
+
+static void
+ExecNextFunctionResult(FunctionScanState *node,
+					   FunctionScanPerFuncState *perfunc)
+{
+	EState	   *estate;
+	ScanDirection direction;
+	MemoryContext callerContext;
+	ExprContext *econtext = node->ss.ps.ps_ExprContext;
+
+	estate = node->ss.ps.state;
+	direction = estate->es_direction;
+
+	callerContext = CurrentMemoryContext;
+
+	if (perfunc->tstore)
+	{
+		(void) tuplestore_gettupleslot(perfunc->tstore,
+									   ScanDirectionIsForward(direction),
+									   false,
+									   perfunc->func_slot);
+	}
+	else if (perfunc->rsinfo.isDone == ExprSingleResult ||
+			 perfunc->rsinfo.isDone == ExprEndResult)
+	{
+		ExecClearTuple(perfunc->func_slot);
+	}
+	else
+	{
+		Datum result;
+		PgStat_FunctionCallUsage fcusage;
+		ExprState *funcexpr PG_USED_FOR_ASSERTS_ONLY = perfunc->funcexpr;
+
+		/* ensure called in a sane context */
+		Assert(funcexpr && IsA(funcexpr, FuncExprState) &&
+			   IsA(funcexpr->expr, FuncExpr));
+		Assert(perfunc->rsinfo.returnMode == SFRM_ValuePerCall);
+
+		/*
+		 * Switch to short-lived context for calling the function or expression.
+		 */
+		MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
+
+		/* next call in percall mode */
+		pgstat_init_function_usage(&perfunc->fcinfo, &fcusage);
+
+		perfunc->fcinfo.isnull = false;
+		perfunc->rsinfo.isDone = ExprSingleResult;
+		result = FunctionCallInvoke(&perfunc->fcinfo);
+
+		pgstat_end_function_usage(&fcusage,
+								  perfunc->rsinfo.isDone != ExprMultipleResult);
+
+		Assert(perfunc->rsinfo.returnMode == SFRM_ValuePerCall);
+
+		if (perfunc->rsinfo.isDone == ExprEndResult)
+		{
+			ExecClearTuple(perfunc->func_slot);
+			goto out;
+		}
+
+		if (perfunc->returnsTuple)
+		{
+			if (!perfunc->fcinfo.isnull)
+			{
+				HeapTupleHeader td = DatumGetHeapTupleHeader(result);
+				HeapTupleData tmptup;
+				TupleDesc tupdesc;
+
+				if (perfunc->rsinfo.setDesc == NULL)
+				{
+					MemoryContext oldcontext;
+
+					/*
+					 * This is the first non-NULL result from the
+					 * function.  Use the type info embedded in the
+					 * rowtype Datum to look up the needed tupdesc.  Make
+					 * a copy for the query.
+					 */
+					oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
+					perfunc->rsinfo.setDesc =
+						lookup_rowtype_tupdesc_copy(HeapTupleHeaderGetTypeId(td),
+													HeapTupleHeaderGetTypMod(td));
+					MemoryContextSwitchTo(oldcontext);
+
+					/*
+					 * Cross-check tupdesc.  We only really need to do this
+					 * for functions returning RECORD, but might as well do it
+					 * always.
+					 */
+					tupledesc_match(perfunc->tupdesc, perfunc->rsinfo.setDesc);
+				}
+
+				tupdesc = perfunc->rsinfo.setDesc;
+
+				/*
+				 * Verify all later returned rows have same subtype;
+				 * necessary in case the type is RECORD.
+				 */
+				if (HeapTupleHeaderGetTypeId(td) != tupdesc->tdtypeid ||
+					HeapTupleHeaderGetTypMod(td) != tupdesc->tdtypmod)
+					ereport(ERROR,
+							(errcode(ERRCODE_DATATYPE_MISMATCH),
+							 errmsg("rows returned by function are not all of the same row type")));
+
+				tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
+				tmptup.t_data = td;
+
+				ExecStoreTuple(&tmptup, perfunc->func_slot, InvalidBuffer, false);
+				/* materializing handles expanded and toasted datums */
+				/* XXX: would be nice if this could be optimized away */
+				ExecMaterializeSlot(perfunc->func_slot);
+			}
+			else
+			{
+				ExecStoreAllNullTuple(perfunc->func_slot);
+			}
+		}
+		else
+		{
+			/* Scalar-type case: just store the function result */
+			ExecClearTuple(perfunc->func_slot);
+			perfunc->func_slot->tts_values[0] = result;
+			perfunc->func_slot->tts_isnull[0] = perfunc->fcinfo.isnull;
+			ExecStoreVirtualTuple(perfunc->func_slot);
+
+			/* materializing handles expanded and toasted datums */
+			ExecMaterializeSlot(perfunc->func_slot);
 		}
 	}
 
-	/* Reset ordinality counter */
-	node->ordinal = 0;
+out:
+	MemoryContextSwitchTo(callerContext);
+}
 
-	/* Make sure we rewind any remaining tuplestores */
-	for (i = 0; i < node->nfuncs; i++)
+
+/*
+ * 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_plural("Returned row contains %d attribute, but query expects %d.",
+				"Returned row contains %d attributes, but query expects %d.",
+								  src_tupdesc->natts,
+								  src_tupdesc->natts, dst_tupdesc->natts)));
+
+	for (i = 0; i < dst_tupdesc->natts; i++)
 	{
-		if (node->funcstates[i].tstore != NULL)
-			tuplestore_rescan(node->funcstates[i].tstore);
+		Form_pg_attribute dattr = dst_tupdesc->attrs[i];
+		Form_pg_attribute sattr = src_tupdesc->attrs[i];
+
+		if (IsBinaryCoercible(sattr->atttypid, dattr->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)));
 	}
 }
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 39521ed..7f11285 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -234,14 +234,13 @@ extern Datum GetAttributeByNum(HeapTupleHeader tuple, AttrNumber attrno,
 				  bool *isNull);
 extern Datum GetAttributeByName(HeapTupleHeader tuple, const char *attname,
 				   bool *isNull);
-extern Tuplestorestate *ExecMakeTableFunctionResult(ExprState *funcexpr,
-							ExprContext *econtext,
-							MemoryContext argContext,
-							TupleDesc expectedDesc,
-							bool randomAccess);
 extern Datum ExecEvalExprSwitchContext(ExprState *expression, ExprContext *econtext,
 						  bool *isNull, ExprDoneCond *isDone);
+extern ExprDoneCond ExecEvalFuncArgs(FunctionCallInfo fcinfo,
+				 List *argList, ExprContext *econtext);
 extern ExprState *ExecInitExpr(Expr *node, PlanState *parent);
+extern void ExecInitFcache(Oid foid, Oid input_collation, FuncExprState *fcache,
+			MemoryContext fcacheCxt, bool needDescForSets);
 extern ExprState *ExecPrepareExpr(Expr *node, EState *estate);
 extern bool ExecQual(List *qual, ExprContext *econtext, bool resultForNull);
 extern int	ExecTargetListLength(List *targetlist);
diff --git a/src/test/regress/expected/pg_lsn.out b/src/test/regress/expected/pg_lsn.out
index 2854cfd..5ed0089 100644
--- a/src/test/regress/expected/pg_lsn.out
+++ b/src/test/regress/expected/pg_lsn.out
@@ -79,14 +79,15 @@ SELECT DISTINCT (i || '/' || j)::pg_lsn f
    ->  HashAggregate
          Group Key: ((((i.i)::text || '/'::text) || (j.j)::text))::pg_lsn
          ->  Nested Loop
-               ->  Function Scan on generate_series k
-               ->  Materialize
-                     ->  Nested Loop
+               ->  Nested Loop
+                     ->  Function Scan on generate_series i
+                           Filter: (i <= 10)
+                     ->  Materialize
                            ->  Function Scan on generate_series j
                                  Filter: ((j > 0) AND (j <= 10))
-                           ->  Function Scan on generate_series i
-                                 Filter: (i <= 10)
-(12 rows)
+               ->  Materialize
+                     ->  Function Scan on generate_series k
+(13 rows)
 
 SELECT DISTINCT (i || '/' || j)::pg_lsn f
   FROM generate_series(1, 10) i,
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index a2c36e4..40fd285 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -3562,7 +3562,7 @@ select * from sc_test();
 
 create or replace function sc_test() returns setof integer as $$
 declare
-  c cursor for select * from generate_series(1, 10);
+  c scroll cursor for select * from generate_series(1, 10);
   x integer;
 begin
   open c;
diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out
index f06cfa4..a6496fc 100644
--- a/src/test/regress/expected/rangefuncs.out
+++ b/src/test/regress/expected/rangefuncs.out
@@ -1109,9 +1109,9 @@ SELECT * FROM (VALUES (1),(2),(3)) v(r), ROWS FROM( foo_sql(11,11), foo_mat(10+r
  1 | 11 | 1 | 11 | 1
  1 |    |   | 12 | 2
  1 |    |   | 13 | 3
- 2 | 11 | 1 | 12 | 4
+ 2 | 11 | 2 | 12 | 4
  2 |    |   | 13 | 5
- 3 | 11 | 1 | 13 | 6
+ 3 | 11 | 3 | 13 | 6
 (6 rows)
 
 SELECT setval('foo_rescan_seq1',1,false),setval('foo_rescan_seq2',1,false);
@@ -1126,9 +1126,9 @@ SELECT * FROM (VALUES (1),(2),(3)) v(r), ROWS FROM( foo_sql(10+r,13), foo_mat(11
  1 | 11 | 1 | 11 | 1
  1 | 12 | 2 |    |  
  1 | 13 | 3 |    |  
- 2 | 12 | 4 | 11 | 1
+ 2 | 12 | 4 | 11 | 2
  2 | 13 | 5 |    |  
- 3 | 13 | 6 | 11 | 1
+ 3 | 13 | 6 | 11 | 3
 (6 rows)
 
 SELECT setval('foo_rescan_seq1',1,false),setval('foo_rescan_seq2',1,false);
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 776f229..2d358bb 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -2952,7 +2952,7 @@ select * from sc_test();
 
 create or replace function sc_test() returns setof integer as $$
 declare
-  c cursor for select * from generate_series(1, 10);
+  c scroll cursor for select * from generate_series(1, 10);
   x integer;
 begin
   open c;
-- 
2.9.3

