Hi,

On 2017-01-16 14:13:18 -0500, Tom Lane wrote:
> Andres Freund <and...@anarazel.de> writes:
> > That worked quite well.  So we have a few questions, before I clean this
> > up:
>
> > - For now the node is named 'Srf' both internally and in explain - not
> >   sure if we want to make that something longer/easier to understand for
> >   others? Proposals? TargetFunctionScan? SetResult?
>
> "Srf" is ugly as can be, and unintelligible.  SetResult might be OK.

Named it SetResult - imo looks ok.  I think I do prefer the separate
node type over re-using Result.  The planner integration looks cleaner
to me due to not needing the srfpp special cases and such.


> > Comments?
>
> Hard to comment on your other points without a patch to look at.

Attached the current version. There's a *lot* of pending cleanup needed
(especially in execQual.c) removing outdated code/comments etc, but this
seems good enough for a first review.  I'd want that cleanup done in a
separate patch anyway.


Attached are two patches. The first is just a rebased version (just some
hunk offset changed) of your planner patch, on top of that is my
executor patch.  My patch moves some minor detail in yours around, and I
do think they should eventually be merged; but leaving it split for a
round displays the changes more cleanly.

Additional questions:
- do we care about SRFs that don't actually return a set? If so we need
  to change the error checking code in ExecEvalFunc/Oper and move it to
  the actual invocation.
- the FuncExpr/OpExpr check in ExecMakeFunctionResult is fairly ugly imo
  - but I don't quite see a much better solution.

Greetings,

Andres
>From 2c16e67f46f418239ab90a51611f168508bac66e Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Sun, 15 Jan 2017 19:23:22 -0800
Subject: [PATCH 1/2] Put SRF into a separate node v1.

Author: Tom Lane
Discussion: https://postgr.es/m/557.1473895...@sss.pgh.pa.us
---
 src/backend/nodes/outfuncs.c             |   1 +
 src/backend/optimizer/plan/createplan.c  |  33 ++++-
 src/backend/optimizer/plan/planner.c     | 219 +++++++++++++++++++++++++------
 src/backend/optimizer/util/clauses.c     | 104 ++-------------
 src/backend/optimizer/util/pathnode.c    |  75 +++++++++++
 src/backend/optimizer/util/tlist.c       | 199 ++++++++++++++++++++++++++++
 src/include/nodes/relation.h             |   1 +
 src/include/optimizer/clauses.h          |   1 -
 src/include/optimizer/pathnode.h         |   4 +
 src/include/optimizer/tlist.h            |   3 +
 src/test/regress/expected/aggregates.out |   3 +-
 src/test/regress/expected/limit.out      |  10 +-
 src/test/regress/expected/rangefuncs.out |  10 +-
 src/test/regress/expected/subselect.out  |  26 ++--
 src/test/regress/expected/tsrf.out       |  11 +-
 15 files changed, 544 insertions(+), 156 deletions(-)

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index cf0a6059e9..73fdc9706d 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -1805,6 +1805,7 @@ _outProjectionPath(StringInfo str, const ProjectionPath *node)
 
 	WRITE_NODE_FIELD(subpath);
 	WRITE_BOOL_FIELD(dummypp);
+	WRITE_BOOL_FIELD(srfpp);
 }
 
 static void
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index c7bcd9b84c..875de739a8 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -1421,8 +1421,21 @@ create_projection_plan(PlannerInfo *root, ProjectionPath *best_path)
 	Plan	   *subplan;
 	List	   *tlist;
 
-	/* Since we intend to project, we don't need to constrain child tlist */
-	subplan = create_plan_recurse(root, best_path->subpath, 0);
+	/*
+	 * XXX Possibly-temporary hack: if the subpath is a dummy ResultPath,
+	 * don't bother with it, just make a Result with no input.  This avoids an
+	 * extra Result plan node when doing "SELECT srf()".  Depending on what we
+	 * decide about the desired plan structure for SRF-expanding nodes, this
+	 * optimization might have to go away, and in any case it'll probably look
+	 * a good bit different.
+	 */
+	if (IsA(best_path->subpath, ResultPath) &&
+		((ResultPath *) best_path->subpath)->path.pathtarget->exprs == NIL &&
+		((ResultPath *) best_path->subpath)->quals == NIL)
+		subplan = NULL;
+	else
+		/* Since we intend to project, we don't need to constrain child tlist */
+		subplan = create_plan_recurse(root, best_path->subpath, 0);
 
 	tlist = build_path_tlist(root, &best_path->path);
 
@@ -1441,8 +1454,9 @@ create_projection_plan(PlannerInfo *root, ProjectionPath *best_path)
 	 * creation, but that would add expense to creating Paths we might end up
 	 * not using.)
 	 */
-	if (is_projection_capable_path(best_path->subpath) ||
-		tlist_same_exprs(tlist, subplan->targetlist))
+	if (!best_path->srfpp &&
+		(is_projection_capable_path(best_path->subpath) ||
+		 tlist_same_exprs(tlist, subplan->targetlist)))
 	{
 		/* Don't need a separate Result, just assign tlist to subplan */
 		plan = subplan;
@@ -6192,6 +6206,17 @@ is_projection_capable_path(Path *path)
 			 * projection to its dummy path.
 			 */
 			return IS_DUMMY_PATH(path);
+		case T_Result:
+
+			/*
+			 * If the path is doing SRF evaluation, claim it can't project, so
+			 * we don't jam a new tlist into it and thereby break the property
+			 * that the SRFs appear at top level.
+			 */
+			if (IsA(path, ProjectionPath) &&
+				((ProjectionPath *) path)->srfpp)
+				return false;
+			break;
 		default:
 			break;
 	}
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index f936710171..70870bbbe0 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -153,6 +153,8 @@ static List *make_pathkeys_for_window(PlannerInfo *root, WindowClause *wc,
 static PathTarget *make_sort_input_target(PlannerInfo *root,
 					   PathTarget *final_target,
 					   bool *have_postponed_srfs);
+static void adjust_paths_for_srfs(PlannerInfo *root, RelOptInfo *rel,
+					  List *targets, List *targets_contain_srfs);
 
 
 /*****************************************************************************
@@ -1434,8 +1436,9 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
 	int64		count_est = 0;
 	double		limit_tuples = -1.0;
 	bool		have_postponed_srfs = false;
-	double		tlist_rows;
 	PathTarget *final_target;
+	List	   *final_targets;
+	List	   *final_targets_contain_srfs;
 	RelOptInfo *current_rel;
 	RelOptInfo *final_rel;
 	ListCell   *lc;
@@ -1498,6 +1501,10 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
 		/* Also extract the PathTarget form of the setop result tlist */
 		final_target = current_rel->cheapest_total_path->pathtarget;
 
+		/* The setop result tlist couldn't contain any SRFs */
+		Assert(!parse->hasTargetSRFs);
+		final_targets = final_targets_contain_srfs = NIL;
+
 		/*
 		 * Can't handle FOR [KEY] UPDATE/SHARE here (parser should have
 		 * checked already, but let's make sure).
@@ -1523,8 +1530,14 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
 	{
 		/* No set operations, do regular planning */
 		PathTarget *sort_input_target;
+		List	   *sort_input_targets;
+		List	   *sort_input_targets_contain_srfs;
 		PathTarget *grouping_target;
+		List	   *grouping_targets;
+		List	   *grouping_targets_contain_srfs;
 		PathTarget *scanjoin_target;
+		List	   *scanjoin_targets;
+		List	   *scanjoin_targets_contain_srfs;
 		bool		have_grouping;
 		AggClauseCosts agg_costs;
 		WindowFuncLists *wflists = NULL;
@@ -1775,8 +1788,50 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
 			scanjoin_target = grouping_target;
 
 		/*
-		 * Forcibly apply scan/join target to all the Paths for the scan/join
-		 * rel.
+		 * If there are any SRFs in the targetlist, we must separate each of
+		 * these PathTargets into SRF-computing and SRF-free targets.  Replace
+		 * each of the named targets with a SRF-free version, and remember the
+		 * list of additional projection steps we need to add afterwards.
+		 */
+		if (parse->hasTargetSRFs)
+		{
+			/* final_target doesn't recompute any SRFs in sort_input_target */
+			split_pathtarget_at_srfs(root, final_target, sort_input_target,
+									 &final_targets,
+									 &final_targets_contain_srfs);
+			final_target = (PathTarget *) linitial(final_targets);
+			Assert(!linitial_int(final_targets_contain_srfs));
+			/* likewise for sort_input_target vs. grouping_target */
+			split_pathtarget_at_srfs(root, sort_input_target, grouping_target,
+									 &sort_input_targets,
+									 &sort_input_targets_contain_srfs);
+			sort_input_target = (PathTarget *) linitial(sort_input_targets);
+			Assert(!linitial_int(sort_input_targets_contain_srfs));
+			/* likewise for grouping_target vs. scanjoin_target */
+			split_pathtarget_at_srfs(root, grouping_target, scanjoin_target,
+									 &grouping_targets,
+									 &grouping_targets_contain_srfs);
+			grouping_target = (PathTarget *) linitial(grouping_targets);
+			Assert(!linitial_int(grouping_targets_contain_srfs));
+			/* scanjoin_target will not have any SRFs precomputed for it */
+			split_pathtarget_at_srfs(root, scanjoin_target, NULL,
+									 &scanjoin_targets,
+									 &scanjoin_targets_contain_srfs);
+			scanjoin_target = (PathTarget *) linitial(scanjoin_targets);
+			Assert(!linitial_int(scanjoin_targets_contain_srfs));
+		}
+		else
+		{
+			/* initialize lists, just to keep compiler quiet */
+			final_targets = final_targets_contain_srfs = NIL;
+			sort_input_targets = sort_input_targets_contain_srfs = NIL;
+			grouping_targets = grouping_targets_contain_srfs = NIL;
+			scanjoin_targets = scanjoin_targets_contain_srfs = NIL;
+		}
+
+		/*
+		 * Forcibly apply SRF-free scan/join target to all the Paths for the
+		 * scan/join rel.
 		 *
 		 * In principle we should re-run set_cheapest() here to identify the
 		 * cheapest path, but it seems unlikely that adding the same tlist
@@ -1847,6 +1902,12 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
 			current_rel->partial_pathlist = NIL;
 		}
 
+		/* Now fix things up if scan/join target contains SRFs */
+		if (parse->hasTargetSRFs)
+			adjust_paths_for_srfs(root, current_rel,
+								  scanjoin_targets,
+								  scanjoin_targets_contain_srfs);
+
 		/*
 		 * Save the various upper-rel PathTargets we just computed into
 		 * root->upper_targets[].  The core code doesn't use this, but it
@@ -1871,6 +1932,11 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
 												&agg_costs,
 												rollup_lists,
 												rollup_groupclauses);
+			/* Fix things up if grouping_target contains SRFs */
+			if (parse->hasTargetSRFs)
+				adjust_paths_for_srfs(root, current_rel,
+									  grouping_targets,
+									  grouping_targets_contain_srfs);
 		}
 
 		/*
@@ -1886,6 +1952,11 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
 											  tlist,
 											  wflists,
 											  activeWindows);
+			/* Fix things up if sort_input_target contains SRFs */
+			if (parse->hasTargetSRFs)
+				adjust_paths_for_srfs(root, current_rel,
+									  sort_input_targets,
+									  sort_input_targets_contain_srfs);
 		}
 
 		/*
@@ -1914,40 +1985,11 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
 										   final_target,
 										   have_postponed_srfs ? -1.0 :
 										   limit_tuples);
-	}
-
-	/*
-	 * If there are set-returning functions in the tlist, scale up the output
-	 * rowcounts of all surviving Paths to account for that.  Note that if any
-	 * SRFs appear in sorting or grouping columns, we'll have underestimated
-	 * the numbers of rows passing through earlier steps; but that's such a
-	 * weird usage that it doesn't seem worth greatly complicating matters to
-	 * account for it.
-	 */
-	if (parse->hasTargetSRFs)
-		tlist_rows = tlist_returns_set_rows(tlist);
-	else
-		tlist_rows = 1;
-
-	if (tlist_rows > 1)
-	{
-		foreach(lc, current_rel->pathlist)
-		{
-			Path	   *path = (Path *) lfirst(lc);
-
-			/*
-			 * We assume that execution costs of the tlist as such were
-			 * already accounted for.  However, it still seems appropriate to
-			 * charge something more for the executor's general costs of
-			 * processing the added tuples.  The cost is probably less than
-			 * cpu_tuple_cost, though, so we arbitrarily use half of that.
-			 */
-			path->total_cost += path->rows * (tlist_rows - 1) *
-				cpu_tuple_cost / 2;
-
-			path->rows *= tlist_rows;
-		}
-		/* No need to run set_cheapest; we're keeping all paths anyway. */
+		/* Fix things up if final_target contains SRFs */
+		if (parse->hasTargetSRFs)
+			adjust_paths_for_srfs(root, current_rel,
+								  final_targets,
+								  final_targets_contain_srfs);
 	}
 
 	/*
@@ -5151,6 +5193,109 @@ get_cheapest_fractional_path(RelOptInfo *rel, double tuple_fraction)
 }
 
 /*
+ * adjust_paths_for_srfs
+ *		Fix up the Paths of the given upperrel to handle tSRFs properly.
+ *
+ * The executor can only handle set-returning functions that appear at the
+ * top level of the targetlist of a Result plan node.  If we have any SRFs
+ * that are not at top level, we need to split up the evaluation into multiple
+ * plan levels in which each level satisfies this constraint.  This function
+ * modifies each Path of an upperrel that (might) compute any SRFs in its
+ * output tlist to insert appropriate projection steps.
+ *
+ * The given targets and targets_contain_srfs lists are from
+ * split_pathtarget_at_srfs().  We assume the existing Paths emit the first
+ * target in targets.
+ */
+static void
+adjust_paths_for_srfs(PlannerInfo *root, RelOptInfo *rel,
+					  List *targets, List *targets_contain_srfs)
+{
+	ListCell   *lc;
+
+	Assert(list_length(targets) == list_length(targets_contain_srfs));
+	Assert(!linitial_int(targets_contain_srfs));
+
+	/* If no SRFs appear at this plan level, nothing to do */
+	if (list_length(targets) == 1)
+		return;
+
+	/*
+	 * Stack SRF-evaluation nodes atop each path for the rel.
+	 *
+	 * In principle we should re-run set_cheapest() here to identify the
+	 * cheapest path, but it seems unlikely that adding the same tlist eval
+	 * costs to all the paths would change that, so we don't bother. Instead,
+	 * just assume that the cheapest-startup and cheapest-total paths remain
+	 * so.  (There should be no parameterized paths anymore, so we needn't
+	 * worry about updating cheapest_parameterized_paths.)
+	 */
+	foreach(lc, rel->pathlist)
+	{
+		Path	   *subpath = (Path *) lfirst(lc);
+		Path	   *newpath = subpath;
+		ListCell   *lc1,
+				   *lc2;
+
+		Assert(subpath->param_info == NULL);
+		forboth(lc1, targets, lc2, targets_contain_srfs)
+		{
+			PathTarget *thistarget = (PathTarget *) lfirst(lc1);
+			bool		contains_srfs = (bool) lfirst_int(lc2);
+
+			/* If this level doesn't contain SRFs, do regular projection */
+			if (contains_srfs)
+				newpath = (Path *) create_srf_projection_path(root,
+															  rel,
+															  newpath,
+															  thistarget);
+			else
+				newpath = (Path *) apply_projection_to_path(root,
+															rel,
+															newpath,
+															thistarget);
+		}
+		lfirst(lc) = newpath;
+		if (subpath == rel->cheapest_startup_path)
+			rel->cheapest_startup_path = newpath;
+		if (subpath == rel->cheapest_total_path)
+			rel->cheapest_total_path = newpath;
+	}
+
+	/* Likewise for partial paths, if any */
+	foreach(lc, rel->partial_pathlist)
+	{
+		Path	   *subpath = (Path *) lfirst(lc);
+		Path	   *newpath = subpath;
+		ListCell   *lc1,
+				   *lc2;
+
+		Assert(subpath->param_info == NULL);
+		forboth(lc1, targets, lc2, targets_contain_srfs)
+		{
+			PathTarget *thistarget = (PathTarget *) lfirst(lc1);
+			bool		contains_srfs = (bool) lfirst_int(lc2);
+
+			/* If this level doesn't contain SRFs, do regular projection */
+			if (contains_srfs)
+				newpath = (Path *) create_srf_projection_path(root,
+															  rel,
+															  newpath,
+															  thistarget);
+			else
+			{
+				/* avoid apply_projection_to_path, in case of multiple refs */
+				newpath = (Path *) create_projection_path(root,
+														  rel,
+														  newpath,
+														  thistarget);
+			}
+		}
+		lfirst(lc) = newpath;
+	}
+}
+
+/*
  * expression_planner
  *		Perform planner's transformations on a standalone expression.
  *
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 59ccdf43d4..a763c7fe24 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -99,7 +99,6 @@ static bool contain_agg_clause_walker(Node *node, void *context);
 static bool get_agg_clause_costs_walker(Node *node,
 							get_agg_clause_costs_context *context);
 static bool find_window_functions_walker(Node *node, WindowFuncLists *lists);
-static bool expression_returns_set_rows_walker(Node *node, double *count);
 static bool contain_subplans_walker(Node *node, void *context);
 static bool contain_mutable_functions_walker(Node *node, void *context);
 static bool contain_volatile_functions_walker(Node *node, void *context);
@@ -790,114 +789,37 @@ find_window_functions_walker(Node *node, WindowFuncLists *lists)
 /*
  * expression_returns_set_rows
  *	  Estimate the number of rows returned by a set-returning expression.
- *	  The result is 1 if there are no set-returning functions.
+ *	  The result is 1 if it's not a set-returning expression.
  *
- * We use the product of the rowcount estimates of all the functions in
- * the given tree (this corresponds to the behavior of ExecMakeFunctionResult
- * for nested set-returning functions).
+ * We should only examine the top-level function or operator; it used to be
+ * appropriate to recurse, but not anymore.  (Even if there are more SRFs in
+ * the function's inputs, their multipliers are accounted for separately.)
  *
  * Note: keep this in sync with expression_returns_set() in nodes/nodeFuncs.c.
  */
 double
 expression_returns_set_rows(Node *clause)
 {
-	double		result = 1;
-
-	(void) expression_returns_set_rows_walker(clause, &result);
-	return clamp_row_est(result);
-}
-
-static bool
-expression_returns_set_rows_walker(Node *node, double *count)
-{
-	if (node == NULL)
-		return false;
-	if (IsA(node, FuncExpr))
+	if (clause == NULL)
+		return 1.0;
+	if (IsA(clause, FuncExpr))
 	{
-		FuncExpr   *expr = (FuncExpr *) node;
+		FuncExpr   *expr = (FuncExpr *) clause;
 
 		if (expr->funcretset)
-			*count *= get_func_rows(expr->funcid);
+			return clamp_row_est(get_func_rows(expr->funcid));
 	}
-	if (IsA(node, OpExpr))
+	if (IsA(clause, OpExpr))
 	{
-		OpExpr	   *expr = (OpExpr *) node;
+		OpExpr	   *expr = (OpExpr *) clause;
 
 		if (expr->opretset)
 		{
 			set_opfuncid(expr);
-			*count *= get_func_rows(expr->opfuncid);
+			return clamp_row_est(get_func_rows(expr->opfuncid));
 		}
 	}
-
-	/* Avoid recursion for some cases that can't return a set */
-	if (IsA(node, Aggref))
-		return false;
-	if (IsA(node, WindowFunc))
-		return false;
-	if (IsA(node, DistinctExpr))
-		return false;
-	if (IsA(node, NullIfExpr))
-		return false;
-	if (IsA(node, ScalarArrayOpExpr))
-		return false;
-	if (IsA(node, BoolExpr))
-		return false;
-	if (IsA(node, SubLink))
-		return false;
-	if (IsA(node, SubPlan))
-		return false;
-	if (IsA(node, AlternativeSubPlan))
-		return false;
-	if (IsA(node, ArrayExpr))
-		return false;
-	if (IsA(node, RowExpr))
-		return false;
-	if (IsA(node, RowCompareExpr))
-		return false;
-	if (IsA(node, CoalesceExpr))
-		return false;
-	if (IsA(node, MinMaxExpr))
-		return false;
-	if (IsA(node, XmlExpr))
-		return false;
-
-	return expression_tree_walker(node, expression_returns_set_rows_walker,
-								  (void *) count);
-}
-
-/*
- * tlist_returns_set_rows
- *	  Estimate the number of rows returned by a set-returning targetlist.
- *	  The result is 1 if there are no set-returning functions.
- *
- * Here, the result is the largest rowcount estimate of any of the tlist's
- * expressions, not the product as you would get from naively applying
- * expression_returns_set_rows() to the whole tlist.  The behavior actually
- * implemented by ExecTargetList produces a number of rows equal to the least
- * common multiple of the expression rowcounts, so that the product would be
- * a worst-case estimate that is typically not realistic.  Taking the max as
- * we do here is a best-case estimate that might not be realistic either,
- * but it's probably closer for typical usages.  We don't try to compute the
- * actual LCM because we're working with very approximate estimates, so their
- * LCM would be unduly noisy.
- */
-double
-tlist_returns_set_rows(List *tlist)
-{
-	double		result = 1;
-	ListCell   *lc;
-
-	foreach(lc, tlist)
-	{
-		TargetEntry *tle = (TargetEntry *) lfirst(lc);
-		double		colresult;
-
-		colresult = expression_returns_set_rows((Node *) tle->expr);
-		if (result < colresult)
-			result = colresult;
-	}
-	return result;
+	return 1.0;
 }
 
 
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 3b7c56d3c7..aa635fd057 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -2227,6 +2227,9 @@ create_projection_path(PlannerInfo *root,
 			(cpu_tuple_cost + target->cost.per_tuple) * subpath->rows;
 	}
 
+	/* Assume no SRFs around */
+	pathnode->srfpp = false;
+
 	return pathnode;
 }
 
@@ -2320,6 +2323,78 @@ apply_projection_to_path(PlannerInfo *root,
 }
 
 /*
+ * create_srf_projection_path
+ *	  Creates a pathnode that represents performing a SRF projection.
+ *
+ * For the moment, we just use ProjectionPath for this, and generate a
+ * Result plan node.  That's likely to change.
+ *
+ * 'rel' is the parent relation associated with the result
+ * 'subpath' is the path representing the source of data
+ * 'target' is the PathTarget to be computed
+ */
+ProjectionPath *
+create_srf_projection_path(PlannerInfo *root,
+						   RelOptInfo *rel,
+						   Path *subpath,
+						   PathTarget *target)
+{
+	ProjectionPath *pathnode = makeNode(ProjectionPath);
+	double		tlist_rows;
+	ListCell   *lc;
+
+	pathnode->path.pathtype = T_Result;
+	pathnode->path.parent = rel;
+	pathnode->path.pathtarget = target;
+	/* For now, assume we are above any joins, so no parameterization */
+	pathnode->path.param_info = NULL;
+	pathnode->path.parallel_aware = false;
+	pathnode->path.parallel_safe = rel->consider_parallel &&
+		subpath->parallel_safe &&
+		is_parallel_safe(root, (Node *) target->exprs);
+	pathnode->path.parallel_workers = subpath->parallel_workers;
+	/* Projection does not change the sort order */
+	pathnode->path.pathkeys = subpath->pathkeys;
+
+	pathnode->subpath = subpath;
+
+	/* Always need the Result node */
+	pathnode->dummypp = false;
+	pathnode->srfpp = true;
+
+	/*
+	 * Estimate number of rows produced by SRFs for each row of input; if
+	 * there's more than one in this node, use the maximum.
+	 */
+	tlist_rows = 1;
+	foreach(lc, target->exprs)
+	{
+		Node	   *node = (Node *) lfirst(lc);
+		double		itemrows;
+
+		itemrows = expression_returns_set_rows(node);
+		if (tlist_rows < itemrows)
+			tlist_rows = itemrows;
+	}
+
+	/*
+	 * In addition to the cost of evaluating the tlist, charge cpu_tuple_cost
+	 * per input row, and half of cpu_tuple_cost for each added output row.
+	 * This is slightly bizarre maybe, but it's what 9.6 did; we may revisit
+	 * this estimate later.
+	 */
+	pathnode->path.rows = subpath->rows * tlist_rows;
+	pathnode->path.startup_cost = subpath->startup_cost +
+		target->cost.startup;
+	pathnode->path.total_cost = subpath->total_cost +
+		target->cost.startup +
+		(cpu_tuple_cost + target->cost.per_tuple) * subpath->rows +
+		(pathnode->path.rows - subpath->rows) * cpu_tuple_cost / 2;
+
+	return pathnode;
+}
+
+/*
  * create_sort_path
  *	  Creates a pathnode that represents performing an explicit sort.
  *
diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c
index 45205a830f..4e92ebdf41 100644
--- a/src/backend/optimizer/util/tlist.c
+++ b/src/backend/optimizer/util/tlist.c
@@ -16,9 +16,20 @@
 
 #include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
+#include "optimizer/cost.h"
 #include "optimizer/tlist.h"
 
 
+typedef struct
+{
+	List	   *nextlevel_tlist;
+	bool		nextlevel_contains_srfs;
+} split_pathtarget_context;
+
+static bool split_pathtarget_walker(Node *node,
+						split_pathtarget_context *context);
+
+
 /*****************************************************************************
  *		Target list creation and searching utilities
  *****************************************************************************/
@@ -759,3 +770,191 @@ apply_pathtarget_labeling_to_tlist(List *tlist, PathTarget *target)
 		i++;
 	}
 }
+
+/*
+ * split_pathtarget_at_srfs
+ *		Split given PathTarget into multiple levels to position SRFs safely
+ *
+ * The executor can only handle set-returning functions that appear at the
+ * top level of the targetlist of a Result plan node.  If we have any SRFs
+ * that are not at top level, we need to split up the evaluation into multiple
+ * plan levels in which each level satisfies this constraint.  This function
+ * creates appropriate PathTarget(s) for each level.
+ *
+ * As an example, consider the tlist expression
+ *		x + srf1(srf2(y + z))
+ * This expression should appear as-is in the top PathTarget, but below that
+ * we must have a PathTarget containing
+ *		x, srf1(srf2(y + z))
+ * and below that, another PathTarget containing
+ *		x, srf2(y + z)
+ * and below that, another PathTarget containing
+ *		x, y, z
+ * When these tlists are processed by setrefs.c, subexpressions that match
+ * output expressions of the next lower tlist will be replaced by Vars,
+ * so that what the executor gets are tlists looking like
+ *		Var1 + Var2
+ *		Var1, srf1(Var2)
+ *		Var1, srf2(Var2 + Var3)
+ *		x, y, z
+ * which satisfy the desired property.
+ *
+ * In some cases, a SRF has already been evaluated in some previous plan level
+ * and we shouldn't expand it again (that is, what we see in the target is
+ * already meant as a reference to a lower subexpression).  So, don't expand
+ * any tlist expressions that appear in input_target, if that's not NULL.
+ * In principle we might need to consider matching subexpressions to
+ * input_target, but for now it's not necessary because only ORDER BY and
+ * GROUP BY expressions are at issue and those will look the same at both
+ * plan levels.
+ *
+ * The outputs of this function are two parallel lists, one a list of
+ * PathTargets and the other an integer list of bool flags indicating
+ * whether the corresponding PathTarget contains any top-level SRFs.
+ * The lists are given in the order they'd need to be evaluated in, with
+ * the "lowest" PathTarget first.  So the last list entry is always the
+ * originally given PathTarget, and any entries before it indicate evaluation
+ * levels that must be inserted below it.  The first list entry must not
+ * contain any SRFs, since it will typically be attached to a plan node
+ * that cannot evaluate SRFs.
+ *
+ * Note: using a list for the flags may seem like overkill, since there
+ * are only a few possible patterns for which levels contain SRFs.
+ * But this representation decouples callers from that knowledge.
+ */
+void
+split_pathtarget_at_srfs(PlannerInfo *root,
+						 PathTarget *target, PathTarget *input_target,
+						 List **targets, List **targets_contain_srfs)
+{
+	/* Initialize output lists to empty; we prepend to them within loop */
+	*targets = *targets_contain_srfs = NIL;
+
+	/* Loop to consider each level of PathTarget we need */
+	for (;;)
+	{
+		bool		target_contains_srfs = false;
+		split_pathtarget_context context;
+		ListCell   *lc;
+
+		context.nextlevel_tlist = NIL;
+		context.nextlevel_contains_srfs = false;
+
+		/*
+		 * Scan the PathTarget looking for SRFs.  Top-level SRFs are handled
+		 * in this loop, ones lower down are found by split_pathtarget_walker.
+		 */
+		foreach(lc, target->exprs)
+		{
+			Node	   *node = (Node *) lfirst(lc);
+
+			/*
+			 * A tlist item that is just a reference to an expression already
+			 * computed in input_target need not be evaluated here, so just
+			 * make sure it's included in the next PathTarget.
+			 */
+			if (input_target && list_member(input_target->exprs, node))
+			{
+				context.nextlevel_tlist = lappend(context.nextlevel_tlist, node);
+				continue;
+			}
+
+			/* Else, we need to compute this expression. */
+			if (IsA(node, FuncExpr) &&
+				((FuncExpr *) node)->funcretset)
+			{
+				/* Top-level SRF: it can be evaluated here */
+				target_contains_srfs = true;
+				/* Recursively examine SRF's inputs */
+				split_pathtarget_walker((Node *) ((FuncExpr *) node)->args,
+										&context);
+			}
+			else if (IsA(node, OpExpr) &&
+					 ((OpExpr *) node)->opretset)
+			{
+				/* Same as above, but for set-returning operator */
+				target_contains_srfs = true;
+				split_pathtarget_walker((Node *) ((OpExpr *) node)->args,
+										&context);
+			}
+			else
+			{
+				/* Not a top-level SRF, so recursively examine expression */
+				split_pathtarget_walker(node, &context);
+			}
+		}
+
+		/*
+		 * Prepend current target and associated flag to output lists.
+		 */
+		*targets = lcons(target, *targets);
+		*targets_contain_srfs = lcons_int(target_contains_srfs,
+										  *targets_contain_srfs);
+
+		/*
+		 * Done if we found no SRFs anywhere in this target; the tentative
+		 * tlist we built for the next level can be discarded.
+		 */
+		if (!target_contains_srfs && !context.nextlevel_contains_srfs)
+			break;
+
+		/*
+		 * Else build the next PathTarget down, and loop back to process it.
+		 * Copy the subexpressions to make sure PathTargets don't share
+		 * substructure (might be unnecessary, but be safe); and drop any
+		 * duplicate entries in the sub-targetlist.
+		 */
+		target = create_empty_pathtarget();
+		add_new_columns_to_pathtarget(target,
+							   (List *) copyObject(context.nextlevel_tlist));
+		set_pathtarget_cost_width(root, target);
+	}
+}
+
+/* Recursively examine expressions for split_pathtarget_at_srfs */
+static bool
+split_pathtarget_walker(Node *node, split_pathtarget_context *context)
+{
+	if (node == NULL)
+		return false;
+	if (IsA(node, Var) ||
+		IsA(node, PlaceHolderVar) ||
+		IsA(node, Aggref) ||
+		IsA(node, GroupingFunc) ||
+		IsA(node, WindowFunc))
+	{
+		/*
+		 * Pass these items down to the child plan level for evaluation.
+		 *
+		 * We assume that these constructs cannot contain any SRFs (if one
+		 * does, there will be an executor failure from a misplaced SRF).
+		 */
+		context->nextlevel_tlist = lappend(context->nextlevel_tlist, node);
+
+		/* Having done that, we need not examine their sub-structure */
+		return false;
+	}
+	else if ((IsA(node, FuncExpr) &&
+			  ((FuncExpr *) node)->funcretset) ||
+			 (IsA(node, OpExpr) &&
+			  ((OpExpr *) node)->opretset))
+	{
+		/*
+		 * Pass SRFs down to the child plan level for evaluation, and mark
+		 * that it contains SRFs.  (We are not at top level of our own tlist,
+		 * else this would have been picked up by split_pathtarget_at_srfs.)
+		 */
+		context->nextlevel_tlist = lappend(context->nextlevel_tlist, node);
+		context->nextlevel_contains_srfs = true;
+
+		/* Inputs to the SRF need not be considered here, so we're done */
+		return false;
+	}
+
+	/*
+	 * Otherwise, the node is evaluatable within the current PathTarget, so
+	 * recurse to examine its inputs.
+	 */
+	return expression_tree_walker(node, split_pathtarget_walker,
+								  (void *) context);
+}
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index e1d31c795a..de4092d679 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -1293,6 +1293,7 @@ typedef struct ProjectionPath
 	Path		path;
 	Path	   *subpath;		/* path representing input source */
 	bool		dummypp;		/* true if no separate Result is needed */
+	bool		srfpp;			/* true if SRFs are being evaluated here */
 } ProjectionPath;
 
 /*
diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h
index 6173ef8d75..cc0d7b0a26 100644
--- a/src/include/optimizer/clauses.h
+++ b/src/include/optimizer/clauses.h
@@ -54,7 +54,6 @@ extern bool contain_window_function(Node *clause);
 extern WindowFuncLists *find_window_functions(Node *clause, Index maxWinRef);
 
 extern double expression_returns_set_rows(Node *clause);
-extern double tlist_returns_set_rows(List *tlist);
 
 extern bool contain_subplans(Node *clause);
 
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index d16f879fc1..c11c59df23 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -144,6 +144,10 @@ extern Path *apply_projection_to_path(PlannerInfo *root,
 						 RelOptInfo *rel,
 						 Path *path,
 						 PathTarget *target);
+extern ProjectionPath *create_srf_projection_path(PlannerInfo *root,
+						   RelOptInfo *rel,
+						   Path *subpath,
+						   PathTarget *target);
 extern SortPath *create_sort_path(PlannerInfo *root,
 				 RelOptInfo *rel,
 				 Path *subpath,
diff --git a/src/include/optimizer/tlist.h b/src/include/optimizer/tlist.h
index f80b31a673..976024a164 100644
--- a/src/include/optimizer/tlist.h
+++ b/src/include/optimizer/tlist.h
@@ -61,6 +61,9 @@ extern void add_column_to_pathtarget(PathTarget *target,
 extern void add_new_column_to_pathtarget(PathTarget *target, Expr *expr);
 extern void add_new_columns_to_pathtarget(PathTarget *target, List *exprs);
 extern void apply_pathtarget_labeling_to_tlist(List *tlist, PathTarget *target);
+extern void split_pathtarget_at_srfs(PlannerInfo *root,
+						 PathTarget *target, PathTarget *input_target,
+						 List **targets, List **targets_contain_srfs);
 
 /* Convenience macro to get a PathTarget with valid cost/width fields */
 #define create_pathtarget(root, tlist) \
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index fa1f5e7879..b71d81ee21 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -823,7 +823,8 @@ explain (costs off)
            ->  Index Only Scan Backward using tenk1_unique2 on tenk1
                  Index Cond: (unique2 IS NOT NULL)
    ->  Result
-(7 rows)
+         ->  Result
+(8 rows)
 
 select max(unique2), generate_series(1,3) as g from tenk1 order by g desc;
  max  | g 
diff --git a/src/test/regress/expected/limit.out b/src/test/regress/expected/limit.out
index 9c3eecfc3b..a7ded3ad05 100644
--- a/src/test/regress/expected/limit.out
+++ b/src/test/regress/expected/limit.out
@@ -208,13 +208,15 @@ select currval('testseq');
 explain (verbose, costs off)
 select unique1, unique2, generate_series(1,10)
   from tenk1 order by unique2 limit 7;
-                        QUERY PLAN                        
-----------------------------------------------------------
+                                                                         QUERY PLAN                                                                          
+-------------------------------------------------------------------------------------------------------------------------------------------------------------
  Limit
    Output: unique1, unique2, (generate_series(1, 10))
-   ->  Index Scan using tenk1_unique2 on public.tenk1
+   ->  Result
          Output: unique1, unique2, generate_series(1, 10)
-(4 rows)
+         ->  Index Scan using tenk1_unique2 on public.tenk1
+               Output: unique1, unique2, two, four, ten, twenty, hundred, thousand, twothousand, fivethous, tenthous, odd, even, stringu1, stringu2, string4
+(6 rows)
 
 select unique1, unique2, generate_series(1,10)
   from tenk1 order by unique2 limit 7;
diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out
index f06cfa4b21..9634fa16d2 100644
--- a/src/test/regress/expected/rangefuncs.out
+++ b/src/test/regress/expected/rangefuncs.out
@@ -1995,12 +1995,10 @@ SELECT *,
         END)
 FROM
   (VALUES (1,''), (2,'0000000049404'), (3,'FROM 10000000876')) v(id, str);
- id |       str        |      lower       
-----+------------------+------------------
-  1 |                  | 
-  2 | 0000000049404    | 49404
-  3 | FROM 10000000876 | from 10000000876
-(3 rows)
+ id |      str      | lower 
+----+---------------+-------
+  2 | 0000000049404 | 49404
+(1 row)
 
 -- check whole-row-Var handling in nested lateral functions (bug #11703)
 create function extractq2(t int8_tbl) returns int8 as $$
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index eda319d24b..3ed089aa46 100644
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -807,24 +807,28 @@ select * from int4_tbl where
 explain (verbose, costs off)
 select * from int4_tbl o where (f1, f1) in
   (select f1, generate_series(1,2) / 10 g from int4_tbl i group by f1);
-                           QUERY PLAN                           
-----------------------------------------------------------------
- Hash Semi Join
+                            QUERY PLAN                             
+-------------------------------------------------------------------
+ Nested Loop Semi Join
    Output: o.f1
-   Hash Cond: (o.f1 = "ANY_subquery".f1)
+   Join Filter: (o.f1 = "ANY_subquery".f1)
    ->  Seq Scan on public.int4_tbl o
          Output: o.f1
-   ->  Hash
+   ->  Materialize
          Output: "ANY_subquery".f1, "ANY_subquery".g
          ->  Subquery Scan on "ANY_subquery"
                Output: "ANY_subquery".f1, "ANY_subquery".g
                Filter: ("ANY_subquery".f1 = "ANY_subquery".g)
-               ->  HashAggregate
-                     Output: i.f1, (generate_series(1, 2) / 10)
-                     Group Key: i.f1
-                     ->  Seq Scan on public.int4_tbl i
-                           Output: i.f1
-(15 rows)
+               ->  Result
+                     Output: i.f1, ((generate_series(1, 2)) / 10)
+                     ->  Result
+                           Output: i.f1, generate_series(1, 2)
+                           ->  HashAggregate
+                                 Output: i.f1
+                                 Group Key: i.f1
+                                 ->  Seq Scan on public.int4_tbl i
+                                       Output: i.f1
+(19 rows)
 
 select * from int4_tbl o where (f1, f1) in
   (select f1, generate_series(1,2) / 10 g from int4_tbl i group by f1);
diff --git a/src/test/regress/expected/tsrf.out b/src/test/regress/expected/tsrf.out
index 7bb6d17fcb..f257537925 100644
--- a/src/test/regress/expected/tsrf.out
+++ b/src/test/regress/expected/tsrf.out
@@ -43,7 +43,16 @@ SELECT generate_series(1, generate_series(1, 3));
 
 -- srf, with two SRF arguments
 SELECT generate_series(generate_series(1,3), generate_series(2, 4));
-ERROR:  functions and operators can take at most one set argument
+ generate_series 
+-----------------
+               1
+               2
+               2
+               3
+               3
+               4
+(6 rows)
+
 CREATE TABLE few(id int, dataa text, datab text);
 INSERT INTO few VALUES(1, 'a', 'foo'),(2, 'a', 'bar'),(3, 'b', 'bar');
 -- SRF output order of sorting is maintained, if SRF is not referenced
-- 
2.11.0.22.g8d7a455.dirty

>From 6e07070ac1f2544ce8f0e455cc34b25144dd4a3e Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Mon, 16 Jan 2017 12:40:13 -0800
Subject: [PATCH 2/2] Implement targetlist set returning functions in a new
 pipeline node.

---
 src/backend/commands/explain.c           |   5 +
 src/backend/executor/Makefile            |   4 +-
 src/backend/executor/execAmi.c           |   5 +
 src/backend/executor/execProcnode.c      |  14 ++
 src/backend/executor/execQual.c          |  85 ++++----
 src/backend/executor/nodeSetResult.c     | 322 +++++++++++++++++++++++++++++++
 src/backend/nodes/copyfuncs.c            |  19 ++
 src/backend/nodes/outfuncs.c             |  12 +-
 src/backend/nodes/readfuncs.c            |  16 ++
 src/backend/optimizer/path/allpaths.c    |   3 +
 src/backend/optimizer/plan/createplan.c  |  93 ++++++---
 src/backend/optimizer/plan/planner.c     |   4 +-
 src/backend/optimizer/plan/setrefs.c     |  21 ++
 src/backend/optimizer/plan/subselect.c   |   1 +
 src/backend/optimizer/util/pathnode.c    |  17 +-
 src/include/executor/executor.h          |   4 +
 src/include/executor/nodeSetResult.h     |  24 +++
 src/include/nodes/execnodes.h            |  15 ++
 src/include/nodes/nodes.h                |   3 +
 src/include/nodes/plannodes.h            |   7 +
 src/include/nodes/relation.h             |  11 +-
 src/include/optimizer/pathnode.h         |   2 +-
 src/test/regress/expected/aggregates.out |   2 +-
 src/test/regress/expected/limit.out      |   8 +-
 src/test/regress/expected/portals.out    |   8 +-
 src/test/regress/expected/subselect.out  |  13 +-
 src/test/regress/expected/tsrf.out       |   8 +-
 src/test/regress/expected/union.out      |   2 +-
 28 files changed, 631 insertions(+), 97 deletions(-)
 create mode 100644 src/backend/executor/nodeSetResult.c
 create mode 100644 src/include/executor/nodeSetResult.h

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index ee7046c47b..a1a42f747d 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -852,6 +852,11 @@ ExplainNode(PlanState *planstate, List *ancestors,
 		case T_Result:
 			pname = sname = "Result";
 			break;
+
+		case T_SetResult:
+			pname = sname = "SetResult";
+			break;
+
 		case T_ModifyTable:
 			sname = "ModifyTable";
 			switch (((ModifyTable *) plan)->operation)
diff --git a/src/backend/executor/Makefile b/src/backend/executor/Makefile
index 51edd4c5e7..15587435d7 100644
--- a/src/backend/executor/Makefile
+++ b/src/backend/executor/Makefile
@@ -22,8 +22,8 @@ OBJS = execAmi.o execCurrent.o execGrouping.o execIndexing.o execJunk.o \
        nodeLimit.o nodeLockRows.o \
        nodeMaterial.o nodeMergeAppend.o nodeMergejoin.o nodeModifyTable.o \
        nodeNestloop.o nodeFunctionscan.o nodeRecursiveunion.o nodeResult.o \
-       nodeSamplescan.o nodeSeqscan.o nodeSetOp.o nodeSort.o nodeUnique.o \
-       nodeValuesscan.o nodeCtescan.o nodeWorktablescan.o \
+       nodeSamplescan.o nodeSeqscan.o nodeSetOp.o nodeSetResult.o nodeSort.o \
+       nodeUnique.o nodeValuesscan.o nodeCtescan.o nodeWorktablescan.o \
        nodeGroup.o nodeSubplan.o nodeSubqueryscan.o nodeTidscan.o \
        nodeForeignscan.o nodeWindowAgg.o tstoreReceiver.o tqueue.o spi.o
 
diff --git a/src/backend/executor/execAmi.c b/src/backend/executor/execAmi.c
index 3ea36979b3..c9c222f446 100644
--- a/src/backend/executor/execAmi.c
+++ b/src/backend/executor/execAmi.c
@@ -44,6 +44,7 @@
 #include "executor/nodeSamplescan.h"
 #include "executor/nodeSeqscan.h"
 #include "executor/nodeSetOp.h"
+#include "executor/nodeSetResult.h"
 #include "executor/nodeSort.h"
 #include "executor/nodeSubplan.h"
 #include "executor/nodeSubqueryscan.h"
@@ -130,6 +131,10 @@ ExecReScan(PlanState *node)
 			ExecReScanResult((ResultState *) node);
 			break;
 
+		case T_SetResultState:
+			ExecReScanSetResult((SetResultState *) node);
+			break;
+
 		case T_ModifyTableState:
 			ExecReScanModifyTable((ModifyTableState *) node);
 			break;
diff --git a/src/backend/executor/execProcnode.c b/src/backend/executor/execProcnode.c
index b8edd36470..f3cc706f13 100644
--- a/src/backend/executor/execProcnode.c
+++ b/src/backend/executor/execProcnode.c
@@ -106,6 +106,7 @@
 #include "executor/nodeSamplescan.h"
 #include "executor/nodeSeqscan.h"
 #include "executor/nodeSetOp.h"
+#include "executor/nodeSetResult.h"
 #include "executor/nodeSort.h"
 #include "executor/nodeSubplan.h"
 #include "executor/nodeSubqueryscan.h"
@@ -155,6 +156,11 @@ ExecInitNode(Plan *node, EState *estate, int eflags)
 												  estate, eflags);
 			break;
 
+		case T_SetResult:
+			result = (PlanState *) ExecInitSetResult((SetResult *) node,
+													 estate, eflags);
+			break;
+
 		case T_ModifyTable:
 			result = (PlanState *) ExecInitModifyTable((ModifyTable *) node,
 													   estate, eflags);
@@ -392,6 +398,10 @@ ExecProcNode(PlanState *node)
 			result = ExecResult((ResultState *) node);
 			break;
 
+		case T_SetResultState:
+			result = ExecSetResult((SetResultState *) node);
+			break;
+
 		case T_ModifyTableState:
 			result = ExecModifyTable((ModifyTableState *) node);
 			break;
@@ -634,6 +644,10 @@ ExecEndNode(PlanState *node)
 			ExecEndResult((ResultState *) node);
 			break;
 
+		case T_SetResultState:
+			ExecEndSetResult((SetResultState *) node);
+			break;
+
 		case T_ModifyTableState:
 			ExecEndModifyTable((ModifyTableState *) node);
 			break;
diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index bf007b7efd..475efedad2 100644
--- a/src/backend/executor/execQual.c
+++ b/src/backend/executor/execQual.c
@@ -104,10 +104,6 @@ static void ExecPrepareTuplestoreResult(FuncExprState *fcache,
 							Tuplestorestate *resultStore,
 							TupleDesc resultDesc);
 static void tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc);
-static Datum ExecMakeFunctionResult(FuncExprState *fcache,
-					   ExprContext *econtext,
-					   bool *isNull,
-					   ExprDoneCond *isDone);
 static Datum ExecMakeFunctionResultNoSets(FuncExprState *fcache,
 							 ExprContext *econtext,
 							 bool *isNull, ExprDoneCond *isDone);
@@ -1681,7 +1677,7 @@ tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc)
  * This function handles the most general case, wherein the function or
  * one of its arguments can return a set.
  */
-static Datum
+Datum
 ExecMakeFunctionResult(FuncExprState *fcache,
 					   ExprContext *econtext,
 					   bool *isNull,
@@ -1702,6 +1698,32 @@ restart:
 	check_stack_depth();
 
 	/*
+	 * Initialize function cache if first time through. Unfortunately the
+	 * parent can be either an FuncExpr or OpExpr.  This is a bit ugly.
+	 */
+	if (fcache->func.fn_oid == InvalidOid)
+	{
+		if (IsA(fcache->xprstate.expr, FuncExpr))
+		{
+			FuncExpr   *func = (FuncExpr *) fcache->xprstate.expr;
+
+			init_fcache(func->funcid, func->inputcollid, fcache,
+						econtext->ecxt_per_query_memory, true);
+		}
+		else if (IsA(fcache->xprstate.expr, OpExpr))
+		{
+			OpExpr	   *op = (OpExpr *) fcache->xprstate.expr;
+
+			init_fcache(op->opfuncid, op->inputcollid, fcache,
+						econtext->ecxt_per_query_memory, true);
+		}
+		else
+		{
+			elog(ERROR, "unexpected type");
+		}
+	}
+
+	/*
 	 * If a previous call of the function returned a set result in the form of
 	 * a tuplestore, continue reading rows from the tuplestore until it's
 	 * empty.
@@ -2423,24 +2445,18 @@ ExecEvalFunc(FuncExprState *fcache,
 
 	/* Initialize function lookup info */
 	init_fcache(func->funcid, func->inputcollid, fcache,
-				econtext->ecxt_per_query_memory, true);
+				econtext->ecxt_per_query_memory, false);
 
-	/*
-	 * We need to invoke ExecMakeFunctionResult if either the function itself
-	 * or any of its input expressions can return a set.  Otherwise, invoke
-	 * ExecMakeFunctionResultNoSets.  In either case, change the evalfunc
-	 * pointer to go directly there on subsequent uses.
-	 */
-	if (fcache->func.fn_retset || expression_returns_set((Node *) func->args))
+	if (expression_returns_set((Node *) func->args) ||
+		fcache->func.fn_retset)
 	{
-		fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResult;
-		return ExecMakeFunctionResult(fcache, econtext, isNull, isDone);
-	}
-	else
-	{
-		fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResultNoSets;
-		return ExecMakeFunctionResultNoSets(fcache, econtext, isNull, isDone);
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("set-valued function called in context that cannot accept a set")));
 	}
+
+	fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResultNoSets;
+	return ExecMakeFunctionResultNoSets(fcache, econtext, isNull, isDone);
 }
 
 /* ----------------------------------------------------------------
@@ -2458,24 +2474,23 @@ ExecEvalOper(FuncExprState *fcache,
 
 	/* Initialize function lookup info */
 	init_fcache(op->opfuncid, op->inputcollid, fcache,
-				econtext->ecxt_per_query_memory, true);
+				econtext->ecxt_per_query_memory, false);
 
-	/*
-	 * We need to invoke ExecMakeFunctionResult if either the function itself
-	 * or any of its input expressions can return a set.  Otherwise, invoke
-	 * ExecMakeFunctionResultNoSets.  In either case, change the evalfunc
-	 * pointer to go directly there on subsequent uses.
-	 */
-	if (fcache->func.fn_retset || expression_returns_set((Node *) op->args))
+	/* should never get here */
+	if (expression_returns_set((Node *) op->args) ||
+		fcache->func.fn_retset)
 	{
-		fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResult;
-		return ExecMakeFunctionResult(fcache, econtext, isNull, isDone);
-	}
-	else
-	{
-		fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResultNoSets;
-		return ExecMakeFunctionResultNoSets(fcache, econtext, isNull, isDone);
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("set-valued function called in context that cannot accept a set")));
 	}
+
+	/* should never get here */
+	Assert(!expression_returns_set((Node *) op->args));
+	Assert(!fcache->func.fn_retset);
+
+	fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResultNoSets;
+	return ExecMakeFunctionResultNoSets(fcache, econtext, isNull, isDone);
 }
 
 /* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeSetResult.c b/src/backend/executor/nodeSetResult.c
new file mode 100644
index 0000000000..55a9789632
--- /dev/null
+++ b/src/backend/executor/nodeSetResult.c
@@ -0,0 +1,322 @@
+/*-------------------------------------------------------------------------
+ *
+ * nodeSetResult.c
+ *	  support for evaluating targetlist set returning functions
+ *
+ * DESCRIPTION
+ *
+ *		SetResult nodes are inserted by the planner to evaluate set returning
+ *		functions in the targetlist. It's guaranteed that all set returning
+ *		functions are directly at the top level of the targetlist, i.e. there
+ *		can't be inside a more complex expressions.  If that'd otherwise be
+ *		the case, the planner adds additional SetResult nodes.
+ *
+ * Portions Copyright (c) 1996-2017, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ *	  src/backend/executor/nodeSetResult.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "executor/executor.h"
+#include "executor/nodeSetResult.h"
+#include "utils/memutils.h"
+
+
+static TupleTableSlot *
+ExecProjectSRF(SetResultState *node, bool continuing);
+
+
+/* ----------------------------------------------------------------
+ *		ExecSetResult(node)
+ *
+ *		returns the tuples from the outer plan which satisfy the
+ *		qualification clause.  Since result nodes with right
+ *		subtrees are never planned, we ignore the right subtree
+ *		entirely (for now).. -cim 10/7/89
+ *
+ *		The qualification containing only constant clauses are
+ *		checked first before any processing is done. It always returns
+ *		'nil' if the constant qualification is not satisfied.
+ * ----------------------------------------------------------------
+ */
+TupleTableSlot *
+ExecSetResult(SetResultState *node)
+{
+	TupleTableSlot *outerTupleSlot;
+	TupleTableSlot *resultSlot;
+	PlanState  *outerPlan;
+	ExprContext *econtext;
+
+	econtext = node->ps.ps_ExprContext;
+
+	/*
+	 * Check to see if we're still projecting out tuples from a previous scan
+	 * tuple (because there is a function-returning-set in the projection
+	 * expressions).  If so, try to project another one.
+	 */
+	if (node->pending_srf_tuples)
+	{
+		resultSlot = ExecProjectSRF(node, true);
+
+		if (resultSlot != NULL)
+			return resultSlot;
+	}
+
+	/*
+	 * Reset per-tuple memory context to free any expression evaluation
+	 * storage allocated in the previous tuple cycle.  Note this can't happen
+	 * until we're done projecting out tuples from a scan tuple.
+	 */
+	ResetExprContext(econtext);
+
+	/*
+	 * if input_done is true then it means that we were asked to return a
+	 * constant tuple and we already did the last time ExecSetResult() was
+	 * called. Either way, now we are through.
+	 */
+	while (!node->input_done)
+	{
+		outerPlan = outerPlanState(node);
+
+		if (outerPlan != NULL)
+		{
+			/*
+			 * Retrieve tuples from the outer plan until there are no more.
+			 */
+			outerTupleSlot = ExecProcNode(outerPlan);
+
+			if (TupIsNull(outerTupleSlot))
+				return NULL;
+
+			/*
+			 * Prepare to compute projection expressions, which will expect to
+			 * access the input tuples as varno OUTER.
+			 */
+			econtext->ecxt_outertuple = outerTupleSlot;
+		}
+		else
+		{
+			/*
+			 * If we don't have an outer plan, then we are just generating the
+			 * results from a constant target list.  Do it only once.
+			 */
+			node->input_done = true;
+		}
+
+		resultSlot = ExecProjectSRF(node, false);
+
+		/*
+		 * Return the tuple unless the projection produced now rows (due to an
+		 * empty set), in which case we must loop back to see if there are
+		 * more outerPlan tuples.
+		 */
+		if (resultSlot)
+			return resultSlot;
+	}
+
+	return NULL;
+}
+
+/* ----------------------------------------------------------------
+ *		ExecProjectSRF
+ *
+ *		Project a targetlist containing one or more set returning functions.
+ *
+ *		continuing is to be set to true if we're continuing to project rows
+ *		for the same input tuple.
+ *
+ *		Returns NULL if no output tuple has been produced.
+ *
+ * ----------------------------------------------------------------
+ */
+static TupleTableSlot *
+ExecProjectSRF(SetResultState *node, bool continuing)
+{
+	TupleTableSlot *resultSlot = node->ps.ps_ResultTupleSlot;
+	ExprContext *econtext = node->ps.ps_ExprContext;
+	ListCell *lc;
+	int argno;
+	bool hasresult;
+	bool hassrf = false PG_USED_FOR_ASSERTS_ONLY;
+
+	ExecClearTuple(resultSlot);
+
+	/*
+	 * Assume no further tuples are produces unless an ExprMultipleResult is
+	 * encountered from a set returning function.
+	 */
+	node->pending_srf_tuples = false;
+
+	hasresult = false;
+	argno = 0;
+	foreach(lc, node->ps.targetlist)
+	{
+		GenericExprState *gstate = (GenericExprState *) lfirst(lc);
+		ExprDoneCond  *isdone = &node->elemdone[argno];
+		Datum *result = &resultSlot->tts_values[argno];
+		bool *isnull = &resultSlot->tts_isnull[argno];
+
+		if (continuing && *isdone == ExprEndResult)
+		{
+			/*
+			 * If we're continuing to project output rows from a source tuple,
+			 * return NULLs once the SRF has been exhausted.
+			 */
+			*result = 0;
+			*isnull = true;
+			hassrf = true;
+		}
+		else if (IsA(gstate->arg, FuncExprState) &&
+				 ((FuncExpr *) gstate->arg->expr)->funcretset)
+		{
+			/*
+			 * Evaluate SRF - possibly continuing previously started output.
+			 */
+			*result = ExecMakeFunctionResult((FuncExprState *) gstate->arg,
+											 econtext, isnull, isdone);
+
+			if (node->elemdone[argno] != ExprEndResult)
+				hasresult = true;
+			if (node->elemdone[argno] == ExprMultipleResult)
+				node->pending_srf_tuples = true;
+			hassrf = true;
+		}
+		else
+		{
+			*result = ExecEvalExpr(gstate->arg, econtext, isnull, NULL);
+			*isdone = ExprSingleResult;
+		}
+
+		argno++;
+	}
+
+	/* SetResult should not be used if there's no SRFs */
+	Assert(hassrf);
+
+	/*
+	 * If all the SRFs returned EndResult, we consider that as no result being
+	 * produced.
+	 */
+	if (hasresult)
+	{
+		ExecStoreVirtualTuple(resultSlot);
+		return resultSlot;
+	}
+
+	return NULL;
+}
+
+/* ----------------------------------------------------------------
+ *		ExecInitResult
+ *
+ *		Creates the run-time state information for the SetResult node
+ *		produced by the planner and initializes outer relations
+ *		(child nodes).
+ * ----------------------------------------------------------------
+ */
+SetResultState *
+ExecInitSetResult(SetResult *node, EState *estate, int eflags)
+{
+	SetResultState *state;
+
+	/* check for unsupported flags */
+	Assert(!(eflags & (EXEC_FLAG_MARK | EXEC_FLAG_BACKWARD)) ||
+		   outerPlan(node) != NULL);
+
+	/*
+	 * create state structure
+	 */
+	state = makeNode(SetResultState);
+	state->ps.plan = (Plan *) node;
+	state->ps.state = estate;
+
+	state->input_done = false;
+	state->pending_srf_tuples = false;
+
+	/*
+	 * Miscellaneous initialization
+	 *
+	 * create expression context for node
+	 */
+	ExecAssignExprContext(estate, &state->ps);
+
+	/*
+	 * tuple table initialization
+	 */
+	ExecInitResultTupleSlot(estate, &state->ps);
+
+	/*
+	 * initialize child expressions
+	 */
+	state->ps.targetlist = (List *)
+		ExecInitExpr((Expr *) node->plan.targetlist,
+					 (PlanState *) state);
+	state->ps.qual = (List *)
+		ExecInitExpr((Expr *) node->plan.qual,
+					 (PlanState *) state);
+
+	/*
+	 * initialize child nodes
+	 */
+	outerPlanState(state) = ExecInitNode(outerPlan(node), estate, eflags);
+
+	/*
+	 * we don't use inner plan
+	 */
+	Assert(innerPlan(node) == NULL);
+
+	/*
+	 * initialize tuple type and projection info
+	 */
+	ExecAssignResultTypeFromTL(&state->ps);
+
+	state->nelems = list_length(node->plan.targetlist);
+	state->elemdone = palloc(sizeof(ExprDoneCond) * state->nelems);
+
+	return state;
+}
+
+/* ----------------------------------------------------------------
+ *		ExecEndSetResult
+ *
+ *		frees up storage allocated through C routines
+ * ----------------------------------------------------------------
+ */
+void
+ExecEndSetResult(SetResultState *node)
+{
+	/*
+	 * Free the exprcontext
+	 */
+	ExecFreeExprContext(&node->ps);
+
+	/*
+	 * clean out the tuple table
+	 */
+	ExecClearTuple(node->ps.ps_ResultTupleSlot);
+
+	/*
+	 * shut down subplans
+	 */
+	ExecEndNode(outerPlanState(node));
+}
+
+void
+ExecReScanSetResult(SetResultState *node)
+{
+	node->input_done = false;
+	node->pending_srf_tuples = false;
+
+	/*
+	 * If chgParam of subnode is not null then plan will be re-scanned by
+	 * first ExecProcNode.
+	 */
+	if (node->ps.lefttree &&
+		node->ps.lefttree->chgParam == NULL)
+		ExecReScan(node->ps.lefttree);
+}
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 7107bbf164..37fbb35455 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -166,6 +166,22 @@ _copyResult(const Result *from)
 }
 
 /*
+ * _copySetResult
+ */
+static SetResult *
+_copySetResult(const SetResult *from)
+{
+	SetResult	   *newnode = makeNode(SetResult);
+
+	/*
+	 * copy node superclass fields
+	 */
+	CopyPlanFields((const Plan *) from, (Plan *) newnode);
+
+	return newnode;
+}
+
+/*
  * _copyModifyTable
  */
 static ModifyTable *
@@ -4413,6 +4429,9 @@ copyObject(const void *from)
 		case T_Result:
 			retval = _copyResult(from);
 			break;
+		case T_SetResult:
+			retval = _copySetResult(from);
+			break;
 		case T_ModifyTable:
 			retval = _copyModifyTable(from);
 			break;
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 73fdc9706d..6a1b9a4536 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -327,6 +327,14 @@ _outResult(StringInfo str, const Result *node)
 }
 
 static void
+_outSetResult(StringInfo str, const SetResult *node)
+{
+	WRITE_NODE_TYPE("SETRESULT");
+
+	_outPlanInfo(str, (const Plan *) node);
+}
+
+static void
 _outModifyTable(StringInfo str, const ModifyTable *node)
 {
 	WRITE_NODE_TYPE("MODIFYTABLE");
@@ -1805,7 +1813,6 @@ _outProjectionPath(StringInfo str, const ProjectionPath *node)
 
 	WRITE_NODE_FIELD(subpath);
 	WRITE_BOOL_FIELD(dummypp);
-	WRITE_BOOL_FIELD(srfpp);
 }
 
 static void
@@ -3362,6 +3369,9 @@ outNode(StringInfo str, const void *obj)
 			case T_Result:
 				_outResult(str, obj);
 				break;
+			case T_SetResult:
+				_outSetResult(str, obj);
+				break;
 			case T_ModifyTable:
 				_outModifyTable(str, obj);
 				break;
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index e02dd94f05..f47b841947 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -1483,6 +1483,20 @@ _readResult(void)
 	READ_DONE();
 }
 
+
+/*
+ * _readSetResult
+ */
+static SetResult *
+_readSetResult(void)
+{
+	READ_LOCALS_NO_FIELDS(SetResult);
+
+	ReadCommonPlan(&local_node->plan);
+
+	READ_DONE();
+}
+
 /*
  * _readModifyTable
  */
@@ -2450,6 +2464,8 @@ parseNodeString(void)
 		return_value = _readPlan();
 	else if (MATCH("RESULT", 6))
 		return_value = _readResult();
+	else if (MATCH("SETRESULT", 9))
+		return_value = _readSetResult();
 	else if (MATCH("MODIFYTABLE", 11))
 		return_value = _readModifyTable();
 	else if (MATCH("APPEND", 6))
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 46d7d064d4..1708e8062c 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2976,6 +2976,9 @@ print_path(PlannerInfo *root, Path *path, int indent)
 		case T_ResultPath:
 			ptype = "Result";
 			break;
+		case T_SetResultPath:
+			ptype = "SetResult";
+			break;
 		case T_MaterialPath:
 			ptype = "Material";
 			subpath = ((MaterialPath *) path)->subpath;
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 875de739a8..78f9d1b4c3 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -81,6 +81,7 @@ static Plan *create_join_plan(PlannerInfo *root, JoinPath *best_path);
 static Plan *create_append_plan(PlannerInfo *root, AppendPath *best_path);
 static Plan *create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path);
 static Result *create_result_plan(PlannerInfo *root, ResultPath *best_path);
+static SetResult *create_set_result_plan(PlannerInfo *root, SetProjectionPath *best_path);
 static Material *create_material_plan(PlannerInfo *root, MaterialPath *best_path,
 					 int flags);
 static Plan *create_unique_plan(PlannerInfo *root, UniquePath *best_path,
@@ -264,6 +265,7 @@ static SetOp *make_setop(SetOpCmd cmd, SetOpStrategy strategy, Plan *lefttree,
 		   long numGroups);
 static LockRows *make_lockrows(Plan *lefttree, List *rowMarks, int epqParam);
 static Result *make_result(List *tlist, Node *resconstantqual, Plan *subplan);
+static SetResult *make_set_result(List *tlist, Plan *subplan);
 static ModifyTable *make_modifytable(PlannerInfo *root,
 				 CmdType operation, bool canSetTag,
 				 Index nominalRelation,
@@ -392,6 +394,10 @@ create_plan_recurse(PlannerInfo *root, Path *best_path, int flags)
 												   (ResultPath *) best_path);
 			}
 			break;
+		case T_SetResult:
+			plan = (Plan *) create_set_result_plan(root,
+											(SetProjectionPath *) best_path);
+			break;
 		case T_Material:
 			plan = (Plan *) create_material_plan(root,
 												 (MaterialPath *) best_path,
@@ -1142,6 +1148,44 @@ create_result_plan(PlannerInfo *root, ResultPath *best_path)
 }
 
 /*
+ * create_set_result_plan
+ *	  Create a SetResult plan for 'best_path'.
+ *
+ *	  Returns a Plan node.
+ */
+static SetResult *
+create_set_result_plan(PlannerInfo *root, SetProjectionPath *best_path)
+{
+	SetResult  *plan;
+	Plan	   *subplan;
+	List	   *tlist;
+
+	/*
+	 * XXX Possibly-temporary hack: if the subpath is a dummy ResultPath,
+	 * don't bother with it, just make a SetResult with no input.  This avoids
+	 * an extra Result plan node when doing "SELECT srf()".  Depending on what
+	 * we decide about the desired plan structure for SRF-expanding nodes,
+	 * this optimization might have to go away, and in any case it'll probably
+	 * look a good bit different.
+	 */
+	if (IsA(best_path->subpath, ResultPath) &&
+		((ResultPath *) best_path->subpath)->path.pathtarget->exprs == NIL &&
+		((ResultPath *) best_path->subpath)->quals == NIL)
+		subplan = NULL;
+	else
+		/* Since we intend to project, we don't need to constrain child tlist */
+		subplan = create_plan_recurse(root, best_path->subpath, 0);
+
+	tlist = build_path_tlist(root, &best_path->path);
+
+	plan = make_set_result(tlist, subplan);
+
+	copy_generic_path_info(&plan->plan, (Path *) best_path);
+
+	return plan;
+}
+
+/*
  * create_material_plan
  *	  Create a Material plan for 'best_path' and (recursively) plans
  *	  for its subpaths.
@@ -1421,21 +1465,8 @@ create_projection_plan(PlannerInfo *root, ProjectionPath *best_path)
 	Plan	   *subplan;
 	List	   *tlist;
 
-	/*
-	 * XXX Possibly-temporary hack: if the subpath is a dummy ResultPath,
-	 * don't bother with it, just make a Result with no input.  This avoids an
-	 * extra Result plan node when doing "SELECT srf()".  Depending on what we
-	 * decide about the desired plan structure for SRF-expanding nodes, this
-	 * optimization might have to go away, and in any case it'll probably look
-	 * a good bit different.
-	 */
-	if (IsA(best_path->subpath, ResultPath) &&
-		((ResultPath *) best_path->subpath)->path.pathtarget->exprs == NIL &&
-		((ResultPath *) best_path->subpath)->quals == NIL)
-		subplan = NULL;
-	else
-		/* Since we intend to project, we don't need to constrain child tlist */
-		subplan = create_plan_recurse(root, best_path->subpath, 0);
+	/* Since we intend to project, we don't need to constrain child tlist */
+	subplan = create_plan_recurse(root, best_path->subpath, 0);
 
 	tlist = build_path_tlist(root, &best_path->path);
 
@@ -1454,9 +1485,8 @@ create_projection_plan(PlannerInfo *root, ProjectionPath *best_path)
 	 * creation, but that would add expense to creating Paths we might end up
 	 * not using.)
 	 */
-	if (!best_path->srfpp &&
-		(is_projection_capable_path(best_path->subpath) ||
-		 tlist_same_exprs(tlist, subplan->targetlist)))
+	if (is_projection_capable_path(best_path->subpath) ||
+		tlist_same_exprs(tlist, subplan->targetlist))
 	{
 		/* Don't need a separate Result, just assign tlist to subplan */
 		plan = subplan;
@@ -6041,6 +6071,25 @@ make_result(List *tlist,
 }
 
 /*
+ * make_set_result
+ *	  Build a SetResult plan node
+ */
+static SetResult *
+make_set_result(List *tlist,
+				Plan *subplan)
+{
+	SetResult  *node = makeNode(SetResult);
+	Plan	   *plan = &node->plan;
+
+	plan->targetlist = tlist;
+	plan->qual = NIL;
+	plan->lefttree = subplan;
+	plan->righttree = NULL;
+
+	return node;
+}
+
+/*
  * make_modifytable
  *	  Build a ModifyTable plan node
  */
@@ -6206,17 +6255,15 @@ is_projection_capable_path(Path *path)
 			 * projection to its dummy path.
 			 */
 			return IS_DUMMY_PATH(path);
-		case T_Result:
 
+		case T_SetResult:
 			/*
 			 * If the path is doing SRF evaluation, claim it can't project, so
 			 * we don't jam a new tlist into it and thereby break the property
 			 * that the SRFs appear at top level.
 			 */
-			if (IsA(path, ProjectionPath) &&
-				((ProjectionPath *) path)->srfpp)
-				return false;
-			break;
+			return false;
+
 		default:
 			break;
 	}
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 70870bbbe0..a208f511d9 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -5245,7 +5245,7 @@ adjust_paths_for_srfs(PlannerInfo *root, RelOptInfo *rel,
 
 			/* If this level doesn't contain SRFs, do regular projection */
 			if (contains_srfs)
-				newpath = (Path *) create_srf_projection_path(root,
+				newpath = (Path *) create_set_projection_path(root,
 															  rel,
 															  newpath,
 															  thistarget);
@@ -5278,7 +5278,7 @@ adjust_paths_for_srfs(PlannerInfo *root, RelOptInfo *rel,
 
 			/* If this level doesn't contain SRFs, do regular projection */
 			if (contains_srfs)
-				newpath = (Path *) create_srf_projection_path(root,
+				newpath = (Path *) create_set_projection_path(root,
 															  rel,
 															  newpath,
 															  thistarget);
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 413a0d9da2..e77312d6af 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -733,6 +733,27 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
 					fix_scan_expr(root, splan->resconstantqual, rtoffset);
 			}
 			break;
+
+		case T_SetResult:
+			{
+				SetResult *splan = (SetResult *) plan;
+
+				/*
+				 * SetResult may or may not have a subplan; if not, it's more
+				 * like a scan node than an upper node.
+				 */
+				if (splan->plan.lefttree != NULL)
+					set_upper_references(root, plan, rtoffset);
+				else
+				{
+					splan->plan.targetlist =
+						fix_scan_list(root, splan->plan.targetlist, rtoffset);
+					splan->plan.qual =
+						fix_scan_list(root, splan->plan.qual, rtoffset);
+				}
+			}
+			break;
+
 		case T_ModifyTable:
 			{
 				ModifyTable *splan = (ModifyTable *) plan;
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index aad0b684ed..ad8b75b4d9 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -2680,6 +2680,7 @@ finalize_plan(PlannerInfo *root, Plan *plan, Bitmapset *valid_params,
 							  &context);
 			break;
 
+		case T_SetResult:
 		case T_Hash:
 		case T_Material:
 		case T_Sort:
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index aa635fd057..2e30af20af 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -2227,9 +2227,6 @@ create_projection_path(PlannerInfo *root,
 			(cpu_tuple_cost + target->cost.per_tuple) * subpath->rows;
 	}
 
-	/* Assume no SRFs around */
-	pathnode->srfpp = false;
-
 	return pathnode;
 }
 
@@ -2333,17 +2330,17 @@ apply_projection_to_path(PlannerInfo *root,
  * 'subpath' is the path representing the source of data
  * 'target' is the PathTarget to be computed
  */
-ProjectionPath *
-create_srf_projection_path(PlannerInfo *root,
+SetProjectionPath *
+create_set_projection_path(PlannerInfo *root,
 						   RelOptInfo *rel,
 						   Path *subpath,
 						   PathTarget *target)
 {
-	ProjectionPath *pathnode = makeNode(ProjectionPath);
+	SetProjectionPath *pathnode = makeNode(SetProjectionPath);
 	double		tlist_rows;
 	ListCell   *lc;
 
-	pathnode->path.pathtype = T_Result;
+	pathnode->path.pathtype = T_SetResult;
 	pathnode->path.parent = rel;
 	pathnode->path.pathtarget = target;
 	/* For now, assume we are above any joins, so no parameterization */
@@ -2353,15 +2350,11 @@ create_srf_projection_path(PlannerInfo *root,
 		subpath->parallel_safe &&
 		is_parallel_safe(root, (Node *) target->exprs);
 	pathnode->path.parallel_workers = subpath->parallel_workers;
-	/* Projection does not change the sort order */
+	/* Projection does not change the sort order XXX? */
 	pathnode->path.pathkeys = subpath->pathkeys;
 
 	pathnode->subpath = subpath;
 
-	/* Always need the Result node */
-	pathnode->dummypp = false;
-	pathnode->srfpp = true;
-
 	/*
 	 * Estimate number of rows produced by SRFs for each row of input; if
 	 * there's more than one in this node, use the maximum.
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index b9c7f72903..59fae35ab5 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -262,6 +262,10 @@ extern int	ExecTargetListLength(List *targetlist);
 extern int	ExecCleanTargetListLength(List *targetlist);
 extern TupleTableSlot *ExecProject(ProjectionInfo *projInfo,
 			ExprDoneCond *isDone);
+extern Datum ExecMakeFunctionResult(FuncExprState *fcache,
+					   ExprContext *econtext,
+					   bool *isNull,
+					   ExprDoneCond *isDone);
 
 /*
  * prototypes from functions in execScan.c
diff --git a/src/include/executor/nodeSetResult.h b/src/include/executor/nodeSetResult.h
new file mode 100644
index 0000000000..f51cf32956
--- /dev/null
+++ b/src/include/executor/nodeSetResult.h
@@ -0,0 +1,24 @@
+/*-------------------------------------------------------------------------
+ *
+ * nodeSetResult.h
+ *
+ *
+ *
+ * Portions Copyright (c) 1996-2017, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/executor/nodeSetResult.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef NODESETRESULT_H
+#define NODESETRESULT_H
+
+#include "nodes/execnodes.h"
+
+extern SetResultState *ExecInitSetResult(SetResult *node, EState *estate, int eflags);
+extern TupleTableSlot *ExecSetResult(SetResultState *node);
+extern void ExecEndSetResult(SetResultState *node);
+extern void ExecReScanSetResult(SetResultState *node);
+
+#endif   /* NODESETRESULT_H */
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index ce13bf7635..69de3ebbd9 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1129,6 +1129,21 @@ typedef struct ResultState
 	bool		rs_checkqual;	/* do we need to check the qual? */
 } ResultState;
 
+
+/* ----------------
+ *	 SetResultState information
+ * ----------------
+ */
+typedef struct SetResultState
+{
+	PlanState	ps;				/* its first field is NodeTag */
+	int			nelems;
+	ExprDoneCond *elemdone;
+	bool		input_done;		/* done reading source tuple? */
+	bool		pending_srf_tuples; /* evaluating srfs in tlist? */
+} SetResultState;
+
+
 /* ----------------
  *	 ModifyTableState information
  * ----------------
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 4c4319bcab..be397fb138 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -43,6 +43,7 @@ typedef enum NodeTag
 	 */
 	T_Plan,
 	T_Result,
+	T_SetResult,
 	T_ModifyTable,
 	T_Append,
 	T_MergeAppend,
@@ -91,6 +92,7 @@ typedef enum NodeTag
 	 */
 	T_PlanState,
 	T_ResultState,
+	T_SetResultState,
 	T_ModifyTableState,
 	T_AppendState,
 	T_MergeAppendState,
@@ -245,6 +247,7 @@ typedef enum NodeTag
 	T_UniquePath,
 	T_GatherPath,
 	T_ProjectionPath,
+	T_SetProjectionPath,
 	T_SortPath,
 	T_GroupPath,
 	T_UpperUniquePath,
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 6810f8c099..3405f018fc 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -176,6 +176,13 @@ typedef struct Result
 	Node	   *resconstantqual;
 } Result;
 
+
+typedef struct SetResult
+{
+	Plan		plan;
+} SetResult;
+
+
 /* ----------------
  *	 ModifyTable node -
  *		Apply rows produced by subplan(s) to result table(s),
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index de4092d679..50fa79926a 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -1293,10 +1293,19 @@ typedef struct ProjectionPath
 	Path		path;
 	Path	   *subpath;		/* path representing input source */
 	bool		dummypp;		/* true if no separate Result is needed */
-	bool		srfpp;			/* true if SRFs are being evaluated here */
 } ProjectionPath;
 
 /*
+ * SetProjectionPath represents an evaluation of a targetlist set returning
+ * function.
+ */
+typedef struct SetProjectionPath
+{
+	Path		path;
+	Path	   *subpath;		/* path representing input source */
+} SetProjectionPath;
+
+/*
  * SortPath represents an explicit sort step
  *
  * The sort keys are, by definition, the same as path.pathkeys.
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index c11c59df23..9cbd87c0a2 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -144,7 +144,7 @@ extern Path *apply_projection_to_path(PlannerInfo *root,
 						 RelOptInfo *rel,
 						 Path *path,
 						 PathTarget *target);
-extern ProjectionPath *create_srf_projection_path(PlannerInfo *root,
+extern SetProjectionPath *create_set_projection_path(PlannerInfo *root,
 						   RelOptInfo *rel,
 						   Path *subpath,
 						   PathTarget *target);
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index b71d81ee21..c7a87a25a9 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -822,7 +822,7 @@ explain (costs off)
      ->  Limit
            ->  Index Only Scan Backward using tenk1_unique2 on tenk1
                  Index Cond: (unique2 IS NOT NULL)
-   ->  Result
+   ->  SetResult
          ->  Result
 (8 rows)
 
diff --git a/src/test/regress/expected/limit.out b/src/test/regress/expected/limit.out
index a7ded3ad05..f3124394a3 100644
--- a/src/test/regress/expected/limit.out
+++ b/src/test/regress/expected/limit.out
@@ -212,7 +212,7 @@ select unique1, unique2, generate_series(1,10)
 -------------------------------------------------------------------------------------------------------------------------------------------------------------
  Limit
    Output: unique1, unique2, (generate_series(1, 10))
-   ->  Result
+   ->  SetResult
          Output: unique1, unique2, generate_series(1, 10)
          ->  Index Scan using tenk1_unique2 on public.tenk1
                Output: unique1, unique2, two, four, ten, twenty, hundred, thousand, twothousand, fivethous, tenthous, odd, even, stringu1, stringu2, string4
@@ -238,7 +238,7 @@ select unique1, unique2, generate_series(1,10)
 --------------------------------------------------------------------
  Limit
    Output: unique1, unique2, (generate_series(1, 10)), tenthous
-   ->  Result
+   ->  SetResult
          Output: unique1, unique2, generate_series(1, 10), tenthous
          ->  Sort
                Output: unique1, unique2, tenthous
@@ -265,7 +265,7 @@ explain (verbose, costs off)
 select generate_series(0,2) as s1, generate_series((random()*.1)::int,2) as s2;
                                               QUERY PLAN                                              
 ------------------------------------------------------------------------------------------------------
- Result
+ SetResult
    Output: generate_series(0, 2), generate_series(((random() * '0.1'::double precision))::integer, 2)
 (2 rows)
 
@@ -285,7 +285,7 @@ order by s2 desc;
  Sort
    Output: (generate_series(0, 2)), (generate_series(((random() * '0.1'::double precision))::integer, 2))
    Sort Key: (generate_series(((random() * '0.1'::double precision))::integer, 2)) DESC
-   ->  Result
+   ->  SetResult
          Output: generate_series(0, 2), generate_series(((random() * '0.1'::double precision))::integer, 2)
 (5 rows)
 
diff --git a/src/test/regress/expected/portals.out b/src/test/regress/expected/portals.out
index 3ae918a63c..b49fa17eb3 100644
--- a/src/test/regress/expected/portals.out
+++ b/src/test/regress/expected/portals.out
@@ -1322,14 +1322,14 @@ begin;
 explain (costs off) declare c2 cursor for select generate_series(1,3) as g;
  QUERY PLAN 
 ------------
- Result
+ SetResult
 (1 row)
 
 explain (costs off) declare c2 scroll cursor for select generate_series(1,3) as g;
-  QUERY PLAN  
---------------
+   QUERY PLAN    
+-----------------
  Materialize
-   ->  Result
+   ->  SetResult
 (2 rows)
 
 declare c2 scroll cursor for select generate_series(1,3) as g;
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index 3ed089aa46..0215c9a663 100644
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -821,7 +821,7 @@ select * from int4_tbl o where (f1, f1) in
                Filter: ("ANY_subquery".f1 = "ANY_subquery".g)
                ->  Result
                      Output: i.f1, ((generate_series(1, 2)) / 10)
-                     ->  Result
+                     ->  SetResult
                            Output: i.f1, generate_series(1, 2)
                            ->  HashAggregate
                                  Output: i.f1
@@ -903,7 +903,7 @@ select * from
  Subquery Scan on ss
    Output: x, u
    Filter: tattle(ss.x, 8)
-   ->  Result
+   ->  SetResult
          Output: 9, unnest('{1,2,3,11,12,13}'::integer[])
 (5 rows)
 
@@ -934,10 +934,11 @@ select * from
   where tattle(x, 8);
                      QUERY PLAN                     
 ----------------------------------------------------
- Result
+ SetResult
    Output: 9, unnest('{1,2,3,11,12,13}'::integer[])
-   One-Time Filter: tattle(9, 8)
-(3 rows)
+   ->  Result
+         One-Time Filter: tattle(9, 8)
+(4 rows)
 
 select * from
   (select 9 as x, unnest(array[1,2,3,11,12,13]) as u) ss
@@ -963,7 +964,7 @@ select * from
  Subquery Scan on ss
    Output: x, u
    Filter: tattle(ss.x, ss.u)
-   ->  Result
+   ->  SetResult
          Output: 9, unnest('{1,2,3,11,12,13}'::integer[])
 (5 rows)
 
diff --git a/src/test/regress/expected/tsrf.out b/src/test/regress/expected/tsrf.out
index f257537925..8c47f0f668 100644
--- a/src/test/regress/expected/tsrf.out
+++ b/src/test/regress/expected/tsrf.out
@@ -25,8 +25,8 @@ SELECT generate_series(1, 2), generate_series(1,4);
 -----------------+-----------------
                1 |               1
                2 |               2
-               1 |               3
-               2 |               4
+                 |               3
+                 |               4
 (4 rows)
 
 -- srf, with SRF argument
@@ -127,15 +127,15 @@ SELECT few.dataa, count(*), min(id), max(id), unnest('{1,1,3}'::int[]) FROM few
 SELECT few.dataa, count(*), min(id), max(id), unnest('{1,1,3}'::int[]) FROM few WHERE few.id = 1 GROUP BY few.dataa, unnest('{1,1,3}'::int[]);
  dataa | count | min | max | unnest 
 -------+-------+-----+-----+--------
- a     |     2 |   1 |   1 |      1
  a     |     1 |   1 |   1 |      3
+ a     |     2 |   1 |   1 |      1
 (2 rows)
 
 SELECT few.dataa, count(*), min(id), max(id), unnest('{1,1,3}'::int[]) FROM few WHERE few.id = 1 GROUP BY few.dataa, 5;
  dataa | count | min | max | unnest 
 -------+-------+-----+-----+--------
- a     |     2 |   1 |   1 |      1
  a     |     1 |   1 |   1 |      3
+ a     |     2 |   1 |   1 |      1
 (2 rows)
 
 -- check HAVING works when GROUP BY does [not] reference SRF output
diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out
index 67f5fc4361..743d0bd0ed 100644
--- a/src/test/regress/expected/union.out
+++ b/src/test/regress/expected/union.out
@@ -636,7 +636,7 @@ ORDER BY x;
          ->  HashAggregate
                Group Key: (1), (generate_series(1, 10))
                ->  Append
-                     ->  Result
+                     ->  SetResult
                      ->  Result
 (9 rows)
 
-- 
2.11.0.22.g8d7a455.dirty

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to