Disallow set-returning functions inside CASE or COALESCE.

When we reimplemented SRFs in commit 69f4b9c, our initial choice was
to allow the behavior to vary from historical practice in cases where a
SRF call appeared within a conditional-execution construct (currently,
only CASE or COALESCE).  But that was controversial to begin with, and
subsequent discussion has resulted in a consensus that it's better to
throw an error instead of executing the query differently from before,
so long as we can provide a reasonably clear error message and a way to
rewrite the query.

Hence, add a parser mechanism to allow detection of such cases during
parse analysis.  The mechanism just requires storing, in the ParseState,
a pointer to the set-returning FuncExpr or OpExpr most recently emitted
by parse analysis.  Then the parsing functions for CASE and COALESCE can
detect the presence of a SRF in their arguments by noting whether this
pointer changes while analyzing their arguments.  Furthermore, if it does,
it provides a suitable error cursor location for the complaint.  (This
means that if there's more than one SRF in the arguments, the error will
point at the last one to be analyzed not the first.  While connoisseurs of
parsing behavior might find that odd, it's unlikely the average user would
ever notice.)

While at it, we can also provide more specific error messages than before
about some pre-existing restrictions, such as no-SRFs-within-aggregates.
Also, reject at parse time cases where a NULLIF or IS DISTINCT FROM
construct would need to return a set.  We've never supported that, but the
restriction is depended on in more subtle ways now, so it seems wise to
detect it at the start.

Also, provide some documentation about how to rewrite a SRF-within-CASE
query using a custom wrapper SRF.

It turns out that the information_schema.user_mapping_options view
contained an instance of exactly the behavior we're now forbidding; but
rewriting it makes it more clear and safer too.

initdb forced because of user_mapping_options change.

Patch by me, with error message suggestions from Alvaro Herrera and
Andres Freund, pursuant to a complaint from Regina Obe.

Discussion: https://postgr.es/m/000001d2d5de$d8d66170$8a832450$@pcorp.us

Author: tglsfdc

doc/src/sgml/xfunc.sgml

@@ -997,6 +997,29 @@ SELECT name, listchildren(name) FROM nodes;
      the <literal>LATERAL</> syntax.
     </para>
 
+    <para>
+     <productname>PostgreSQL</>'s behavior for a set-returning function in a
+     query's select list is almost exactly the same as if the set-returning
+     function had been written in a <literal>LATERAL FROM</>-clause item
+     instead.  For example,
+<programlisting>
+SELECT x, generate_series(1,5) AS g FROM tab;
+</programlisting>
+     is almost equivalent to
+<programlisting>
+SELECT x, g FROM tab, LATERAL generate_series(1,5) AS g;
+</programlisting>
+     It would be exactly the same, except that in this specific example,
+     the planner could choose to put <structname>g</> on the outside of the
+     nestloop join, since <structname>g</> has no actual lateral dependency
+     on <structname>tab</>.  That would result in a different output row
+     order.  Set-returning functions in the select list are always evaluated
+     as though they are on the inside of a nestloop join with the rest of
+     the <literal>FROM</> clause, so that the function(s) are run to
+     completion before the next row from the <literal>FROM</> clause is
+     considered.
+    </para>
+
     <para>
      If there is more than one set-returning function in the query's select
      list, the behavior is similar to what you get from putting the functions
@@ -1028,32 +1051,19 @@ SELECT srf1(srf2(x), srf3(y)), srf4(srf5(z)) FROM tab;
     </para>
 
     <para>
-     This behavior also means that set-returning functions will be evaluated
-     even when it might appear that they should be skipped because of a
-     conditional-evaluation construct, such as <literal>CASE</>
-     or <literal>COALESCE</>.  For example, consider
+     Set-returning functions cannot be used within conditional-evaluation
+     constructs, such as <literal>CASE</> or <literal>COALESCE</>.  For
+     example, consider
 <programlisting>
 SELECT x, CASE WHEN x &gt; 0 THEN generate_series(1, 5) ELSE 0 END FROM tab;
 </programlisting>
-     It might seem that this should produce five repetitions of input
-     rows that have <literal>x &gt; 0</>, and a single repetition of those
-     that do not; but actually it will produce five repetitions of every
-     input row.  This is because <function>generate_series()</> is run first,
-     and then the <literal>CASE</> expression is applied to its result rows.
-     The behavior is thus comparable to
-<programlisting>
-SELECT x, CASE WHEN x &gt; 0 THEN g ELSE 0 END
-  FROM tab, LATERAL generate_series(1,5) AS g;
-</programlisting>
-     It would be exactly the same, except that in this specific example,
-     the planner could choose to put <structname>g</> on the outside of the
-     nestloop join, since <structname>g</> has no actual lateral dependency
-     on <structname>tab</>.  That would result in a different output row
-     order.  Set-returning functions in the select list are always evaluated
-     as though they are on the inside of a nestloop join with the rest of
-     the <literal>FROM</> clause, so that the function(s) are run to
-     completion before the next row from the <literal>FROM</> clause is
-     considered.
+     It might seem that this should produce five repetitions of input rows
+     that have <literal>x &gt; 0</>, and a single repetition of those that do
+     not; but actually, because <function>generate_series(1, 5)</> would be
+     run in an implicit <literal>LATERAL FROM</> item before
+     the <literal>CASE</> expression is ever evaluated, it would produce five
+     repetitions of every input row.  To reduce confusion, such cases produce
+     a parse-time error instead.
     </para>
 
     <note>
@@ -1078,11 +1088,34 @@ SELECT x, CASE WHEN x &gt; 0 THEN g ELSE 0 END
       functions.  Also, nested set-returning functions did not work as
       described above; instead, a set-returning function could have at most
       one set-returning argument, and each nest of set-returning functions
-      was run independently.  The behavior for conditional execution
-      (set-returning functions inside <literal>CASE</> etc) was different too.
+      was run independently.  Also, conditional execution (set-returning
+      functions inside <literal>CASE</> etc) was previously allowed,
+      complicating things even more.
       Use of the <literal>LATERAL</> syntax is recommended when writing
       queries that need to work in older <productname>PostgreSQL</> versions,
       because that will give consistent results across different versions.
+      If you have a query that is relying on conditional execution of a
+      set-returning function, you may be able to fix it by moving the
+      conditional test into a custom set-returning function.  For example,
+<programlisting>
+SELECT x, CASE WHEN y &gt; 0 THEN generate_series(1, z) ELSE 5 END FROM tab;
+</programlisting>
+      could become
+<programlisting>
+CREATE FUNCTION case_generate_series(cond bool, start int, fin int, els int)
+  RETURNS SETOF int AS $$
+BEGIN
+  IF cond THEN
+    RETURN QUERY SELECT generate_series(start, fin);
+  ELSE
+    RETURN QUERY SELECT els;
+  END IF;
+END$$ LANGUAGE plpgsql;
+
+SELECT x, case_generate_series(y &gt; 0, 1, z, 5) FROM tab;
+</programlisting>
+      This formulation will work the same in all versions
+      of <productname>PostgreSQL</>.
      </para>
     </note>
    </sect2>

src/backend/catalog/information_schema.sql

@@ -2936,12 +2936,14 @@ CREATE VIEW user_mapping_options AS
     SELECT authorization_identifier,
            foreign_server_catalog,
            foreign_server_name,
-           CAST((pg_options_to_table(um.umoptions)).option_name AS sql_identifier) AS option_name,
+           CAST(opts.option_name AS sql_identifier) AS option_name,
            CAST(CASE WHEN (umuser <> 0 AND authorization_identifier = current_user)
                        OR (umuser = 0 AND pg_has_role(srvowner, 'USAGE'))
-                       OR (SELECT rolsuper FROM pg_authid WHERE rolname = current_user) THEN (pg_options_to_table(um.umoptions)).option_value
+                       OR (SELECT rolsuper FROM pg_authid WHERE rolname = current_user)
+                     THEN opts.option_value
                      ELSE NULL END AS character_data) AS option_value
-    FROM _pg_user_mappings um;
+    FROM _pg_user_mappings um,
+         pg_options_to_table(um.umoptions) opts;
 
 GRANT SELECT ON user_mapping_options TO PUBLIC;
 

src/backend/executor/functions.c

@@ -388,6 +388,7 @@ sql_fn_post_column_ref(ParseState *pstate, ColumnRef *cref, Node *var)
 		param = ParseFuncOrColumn(pstate,
 								  list_make1(subfield),
 								  list_make1(param),
+								  pstate->p_last_srf,
 								  NULL,
 								  cref->location);
 	}

src/backend/parser/parse_agg.c

@@ -705,6 +705,14 @@ check_agg_arguments_walker(Node *node,
 		}
 		/* Continue and descend into subtree */
 	}
+	/* We can throw error on sight for a set-returning function */
+	if ((IsA(node, FuncExpr) &&((FuncExpr *) node)->funcretset) ||
+		(IsA(node, OpExpr) &&((OpExpr *) node)->opretset))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("aggregate function calls cannot contain set-returning function calls"),
+				 errhint("You might be able to move the set-returning function into a LATERAL FROM item."),
+				 parser_errposition(context->pstate, exprLocation(node))));
 	/* We can throw error on sight for a window function */
 	if (IsA(node, WindowFunc))
 		ereport(ERROR,

src/backend/parser/parse_clause.c

@@ -572,6 +572,8 @@ transformRangeFunction(ParseState *pstate, RangeFunction *r)
 		List	   *pair = (List *) lfirst(lc);
 		Node	   *fexpr;
 		List	   *coldeflist;
+		Node	   *newfexpr;
+		Node	   *last_srf;
 
 		/* Disassemble the function-call/column-def-list pairs */
 		Assert(list_length(pair) == 2);
@@ -618,13 +620,25 @@ transformRangeFunction(ParseState *pstate, RangeFunction *r)
 					Node	   *arg = (Node *) lfirst(lc);
 					FuncCall   *newfc;
 
+					last_srf = pstate->p_last_srf;
+
 					newfc = makeFuncCall(SystemFuncName("unnest"),
 										 list_make1(arg),
 										 fc->location);
 
-					funcexprs = lappend(funcexprs,
-										transformExpr(pstate, (Node *) newfc,
-												   EXPR_KIND_FROM_FUNCTION));
+					newfexpr = transformExpr(pstate, (Node *) newfc,
+											 EXPR_KIND_FROM_FUNCTION);
+
+					/* nodeFunctionscan.c requires SRFs to be at top level */
+					if (pstate->p_last_srf != last_srf &&
+						pstate->p_last_srf != newfexpr)
+						ereport(ERROR,
+								(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+								 errmsg("set-returning functions must appear at top level of FROM"),
+								 parser_errposition(pstate,
+										 exprLocation(pstate->p_last_srf))));
+
+					funcexprs = lappend(funcexprs, newfexpr);
 
 					funcnames = lappend(funcnames,
 										FigureColname((Node *) newfc));
@@ -638,9 +652,21 @@ transformRangeFunction(ParseState *pstate, RangeFunction *r)
 		}
 
 		/* normal case ... */
-		funcexprs = lappend(funcexprs,
-							transformExpr(pstate, fexpr,
-										  EXPR_KIND_FROM_FUNCTION));
+		last_srf = pstate->p_last_srf;
+
+		newfexpr = transformExpr(pstate, fexpr,
+								 EXPR_KIND_FROM_FUNCTION);
+
+		/* nodeFunctionscan.c requires SRFs to be at top level */
+		if (pstate->p_last_srf != last_srf &&
+			pstate->p_last_srf != newfexpr)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("set-returning functions must appear at top level of FROM"),
+					 parser_errposition(pstate,
+										exprLocation(pstate->p_last_srf))));
+
+		funcexprs = lappend(funcexprs, newfexpr);
 
 		funcnames = lappend(funcnames,
 							FigureColname(fexpr));