>From 4dc1c9e5014c9fb421cd9ecd24200d5668ff651f Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 27 Aug 2016 12:36:13 -0700
Subject: [PATCH 2/6] Shore up some weird corner cases for targetlist SRFs.

---
 src/backend/executor/README        | 13 +++++--------
 src/backend/parser/analyze.c       |  8 ++++++++
 src/backend/parser/parse_clause.c  |  7 +++++++
 src/test/regress/expected/tsrf.out | 36 ++++++++++++------------------------
 src/test/regress/sql/tsrf.sql      |  6 +++---
 5 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/src/backend/executor/README b/src/backend/executor/README
index 8afa1e3..141ddc2 100644
--- a/src/backend/executor/README
+++ b/src/backend/executor/README
@@ -192,11 +192,8 @@ relations, such as a ValuesScan or FunctionScan.  For these, since there
 is no equivalent of TID, the only practical solution seems to be to include
 the entire row value in the join output row.
 
-We disallow set-returning functions in the targetlist of SELECT FOR UPDATE,
-so as to ensure that at most one tuple can be returned for any particular
-set of scan tuples.  Otherwise we'd get duplicates due to the original
-query returning the same set of scan tuples multiple times.  (Note: there
-is no explicit prohibition on SRFs in UPDATE, but the net effect will be
-that only the first result row of an SRF counts, because all subsequent
-rows will result in attempts to re-update an already updated target row.
-This is historical behavior and seems not worth changing.)
+We disallow set-returning functions in the targetlist of UPDATE and
+SELECT FOR UPDATE, so as to ensure that at most one tuple can be
+returned for any particular set of scan tuples.  Otherwise we'd get
+duplicates due to the original query returning the same set of scan
+tuples multiple times.
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index eac86cc..cf5bc86 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -2233,6 +2233,14 @@ transformUpdateTargetList(ParseState *pstate, List *origTlist)
 						 RelationGetRelationName(pstate->p_target_relation)),
 					 parser_errposition(pstate, origTarget->location)));
 
+		/* nonsensical, we wouldn't know which of the returned rows to use */
+		if (expression_returns_set((Node *) tle->expr))
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("set-valued function called in context that cannot accept a set"),
+					 parser_errposition(pstate,
+										exprLocation((Node *) tle->expr))));
+
 		updateTargetListEntry(pstate, tle, origTarget->name,
 							  attrno,
 							  origTarget->indirection,
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 751de4b..9b7fcc3 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -1794,6 +1794,13 @@ findTargetlistEntrySQL99(ParseState *pstate, Node *node, List **tlist,
 			return tle;
 	}
 
+	if (expression_returns_set(expr))
+		/* FIXME: decent error message */
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("GROUP BY / ORDER BY cannot reference set returning function not present in target list"),
+				 parser_errposition(pstate, exprLocation(expr))));
+
 	/*
 	 * If no matches, construct a new target entry which is appended to the
 	 * end of the target list.  This target is given resjunk = TRUE so that it
diff --git a/src/test/regress/expected/tsrf.out b/src/test/regress/expected/tsrf.out
index e4e6059..f520a91 100644
--- a/src/test/regress/expected/tsrf.out
+++ b/src/test/regress/expected/tsrf.out
@@ -90,21 +90,11 @@ SELECT few.id, generate_series(1,3) g FROM few ORDER BY id, generate_series(1,3)
   3 | 1
 (9 rows)
 
--- it's weird to have ORDER BYs that increase the number of results
+-- it's weird to have ORDER BYs that increase the number of results (error)
 SELECT few.id FROM few ORDER BY id, generate_series(1,3) DESC;
- id 
-----
-  1
-  1
-  1
-  2
-  2
-  2
-  3
-  3
-  3
-(9 rows)
-
+ERROR:  GROUP BY / ORDER BY cannot reference set returning function not present in target list
+LINE 1: SELECT few.id FROM few ORDER BY id, generate_series(1,3) DES...
+                                            ^
 -- SRFs are computed after aggregation
 SELECT few.dataa, count(*), min(id), max(id), unnest('{1,1,3}'::int[]) FROM few WHERE few.id = 1 GROUP BY few.dataa;
  dataa | count | min | max | unnest 
@@ -129,7 +119,7 @@ SELECT few.dataa, count(*), min(id), max(id), unnest('{1,1,3}'::int[]) FROM few
  a     |     1 |   1 |   1 |      3
 (2 rows)
 
--- it's weird to GROUP BYs that increase the number of results
+-- it's weird to GROUP BYs that increase the number of results (2nd is an error)
 SELECT few.dataa, count(*), min(id), max(id) FROM few GROUP BY few.dataa;
  dataa | count | min | max 
 -------+-------+-----+-----
@@ -138,14 +128,9 @@ SELECT few.dataa, count(*), min(id), max(id) FROM few GROUP BY few.dataa;
 (2 rows)
 
 SELECT few.dataa, count(*), min(id), max(id) FROM few GROUP BY few.dataa, unnest('{1,1,3}'::int[]);
- dataa | count | min | max 
--------+-------+-----+-----
- b     |     2 |   3 |   3
- a     |     4 |   1 |   2
- b     |     1 |   3 |   3
- a     |     2 |   1 |   2
-(4 rows)
-
+ERROR:  GROUP BY / ORDER BY cannot reference set returning function not present in target list
+LINE 1: ...*), min(id), max(id) FROM few GROUP BY few.dataa, unnest('{1...
+                                                             ^
 -- SRFs are computed after window functions
 SELECT id,lag(id) OVER(), count(*) OVER(), generate_series(1,3) FROM few;
  id | lag | count | generate_series 
@@ -200,8 +185,11 @@ SELECT * FROM fewmore;
     5
 (5 rows)
 
--- nonsensically that seems to be allowed
+-- it'd not be clear which value to use for the update (error)
 UPDATE fewmore SET data = generate_series(4,9);
+ERROR:  set-valued function called in context that cannot accept a set
+LINE 1: UPDATE fewmore SET data = generate_series(4,9);
+                                  ^
 -- SRFs are now allowed in RETURNING
 INSERT INTO fewmore VALUES(1) RETURNING generate_series(1,3);
 ERROR:  set-valued function called in context that cannot accept a set
diff --git a/src/test/regress/sql/tsrf.sql b/src/test/regress/sql/tsrf.sql
index 0ef41ab..b52ec07 100644
--- a/src/test/regress/sql/tsrf.sql
+++ b/src/test/regress/sql/tsrf.sql
@@ -27,7 +27,7 @@ SELECT few.id, generate_series(1,3) g FROM few ORDER BY id DESC;
 SELECT few.id, generate_series(1,3) g FROM few ORDER BY id, g DESC;
 SELECT few.id, generate_series(1,3) g FROM few ORDER BY id, generate_series(1,3) DESC;
 
--- it's weird to have ORDER BYs that increase the number of results
+-- it's weird to have ORDER BYs that increase the number of results (error)
 SELECT few.id FROM few ORDER BY id, generate_series(1,3) DESC;
 
 -- SRFs are computed after aggregation
@@ -36,7 +36,7 @@ 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[]);
 SELECT few.dataa, count(*), min(id), max(id), unnest('{1,1,3}'::int[]) FROM few WHERE few.id = 1 GROUP BY few.dataa, 5;
 
--- it's weird to GROUP BYs that increase the number of results
+-- it's weird to GROUP BYs that increase the number of results (2nd is an error)
 SELECT few.dataa, count(*), min(id), max(id) FROM few GROUP BY few.dataa;
 SELECT few.dataa, count(*), min(id), max(id) FROM few GROUP BY few.dataa, unnest('{1,1,3}'::int[]);
 
@@ -54,7 +54,7 @@ CREATE TABLE fewmore AS SELECT generate_series(1,3) AS data;
 INSERT INTO fewmore VALUES(generate_series(4,5));
 SELECT * FROM fewmore;
 
--- nonsensically that seems to be allowed
+-- it'd not be clear which value to use for the update (error)
 UPDATE fewmore SET data = generate_series(4,9);
 
 -- SRFs are now allowed in RETURNING
-- 
2.9.3

