Re: [HACKERS] PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

I wrote:
> As to *how* to throw an error, I think it should be possible to teach
> parse analysis to detect such cases, with something like the
> ParseExprKind mechanism that could be checked to see if we're inside
> a subexpression that restricts what's allowed.  There are some other
> checks like no-nested-aggregates that perhaps could be folded in as
> well.

I spent some time experimenting with this, and immediately found out
that information_schema.user_mapping_options contains an instance of the
problematic usage :-(.  However, that view also turns out to be a poster
child for why our old SRF semantics are a bad idea, because it's on the
hairy edge of failing completely.  It's got two SRF invocations in its
tlist, which it relies on to execute in lockstep and produce the same
number of rows --- but then it puts a CASE around one of them, with an
ELSE NULL.  So in old versions, that only works because if the CASE
doesn't return a set result at runtime then we just run the tlist the
number of times suggested by the other SRF.  And in HEAD, it only works
because we run the two SRFs in lockstep behind the scenes and then the
CASE is discarding individual results not the whole execution of the
second SRF.  If you don't have a headache at this point, re-read the above
until you do.  Or if you fully understand that and still think it's fine,
consider what will happen if the CASE's test expression generates
different results from one call to the next --- which I think is actually
possible here, because pg_has_role() operates on CatalogSnapshot time and
might possibly change its mind partway through the query.  Pre-v10, that
would have generated completely messed-up output, with a hard-to-predict
number of rows and unexpected combinations of the two SRFs' outputs.

Rewriting this view to use a LATERAL SRF call is just enormously cleaner.

Anyway, to the subject at hand.  My thought when I wrote the previous
message was to implement a "top down" mechanism whereby contexts like
CASE and COALESCE would record their presence in the ParseState while
recursively analyzing their arguments, and then SRF calls would be
responsible for throwing an error by inspecting that context.  The first
attached patch does it that way, and it seems nice and clean, but I ran
into a complete dead end while trying to extend it to handle related cases
such as disallowing SRF-inside-aggregate or nested SRFs in FROM.  The
trouble is that when we are parsing the arguments of a SRF or aggregate,
we don't actually know yet whether it's a SRF or aggregate or plain
function.  We can't find that out until we look up the pg_proc entry,
which we can't do without knowing the argument types, so we have to do
parse analysis of the arguments first.

I then considered a "bottom up" approach where the idea is for each SRF
to report its presence in the ParseState, and then the outer construct
complains if any SRF reported its presence within the outer construct's
arguments.  This isn't hard to implement, just keep a "p_last_srf" pointer
in the ParseState, as in the second attached patch.  This feels more ugly
than the first approach; for one thing, if we want to extend it to
multiple cases of "X cannot contain a Y" then we need a ParseState field
for each kind of Y we care about.  Also, it will tend to complain about
the last Y within a given X construct, not the first one; but only a true
geek would ever even notice that, let alone feel that it's strange.

I didn't actually fix the "bottom up" approach to complain about nested
SRFs.  It's clear to me how to do so, but because we analyze function
arguments before calling ParseFuncOrColumn, we'd have to have the callers
of that function remember the appropriate p_last_srf value (ie, from just
before they analyze the arguments) and then pass it to ParseFuncOrColumn.
That would add a bunch of uninteresting clutter to the patch so I didn't
do it here.

I'm inclined to go with the "bottom up" approach, but I wonder if anyone
has any comments or better ideas?

            regards, tom lane

diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql
index cbcd6cf..98bcfa0 100644
*** a/src/backend/catalog/information_schema.sql
--- b/src/backend/catalog/information_schema.sql
*************** CREATE VIEW user_mapping_options AS
*** 2936,2947 ****
      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(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
                       ELSE NULL END AS character_data) AS option_value
!     FROM _pg_user_mappings um;

  GRANT SELECT ON user_mapping_options TO PUBLIC;

--- 2936,2949 ----
      SELECT authorization_identifier,
             foreign_server_catalog,
             foreign_server_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 opts.option_value
                       ELSE NULL END AS character_data) AS option_value
!     FROM _pg_user_mappings um,
!          pg_options_to_table(um.umoptions) opts;

  GRANT SELECT ON user_mapping_options TO PUBLIC;

diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 27dd49d..889338f 100644
*** a/src/backend/parser/parse_clause.c
--- b/src/backend/parser/parse_clause.c
*************** transformRangeSubselect(ParseState *psta
*** 485,490 ****
--- 485,491 ----
       */
      Assert(pstate->p_expr_kind == EXPR_KIND_NONE);
      pstate->p_expr_kind = EXPR_KIND_FROM_SUBSELECT;
+     Assert(pstate->p_subexpr_kind == NIL);

      /*
       * If the subselect is LATERAL, make lateral_only names of this level
*************** transformRangeSubselect(ParseState *psta
*** 504,509 ****
--- 505,511 ----
      /* Restore state */
      pstate->p_lateral_active = false;
      pstate->p_expr_kind = EXPR_KIND_NONE;
+     Assert(pstate->p_subexpr_kind == NIL);

      /*
       * Check that we got a SELECT.  Anything else should be impossible given
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 92101c9..bbefb99 100644
*** a/src/backend/parser/parse_expr.c
--- b/src/backend/parser/parse_expr.c
*************** transformCaseExpr(ParseState *pstate, Ca
*** 1630,1635 ****
--- 1630,1638 ----

      newc = makeNode(CaseExpr);

+     /* show that subexpressions are within CASE */
+     PushSubExprKind(pstate, SUBEXPR_KIND_CASE);
+
      /* transform the test expression, if any */
      arg = transformExprRecurse(pstate, (Node *) c->arg);

*************** transformCaseExpr(ParseState *pstate, Ca
*** 1741,1746 ****
--- 1744,1751 ----
                                    "CASE/WHEN");
      }

+     PopSubExprKind(pstate, SUBEXPR_KIND_CASE);
+
      newc->location = c->location;

      return (Node *) newc;
*************** transformCoalesceExpr(ParseState *pstate
*** 2181,2186 ****
--- 2186,2194 ----
      List       *newcoercedargs = NIL;
      ListCell   *args;

+     /* show that subexpressions are within COALESCE */
+     PushSubExprKind(pstate, SUBEXPR_KIND_COALESCE);
+
      foreach(args, c->args)
      {
          Node       *e = (Node *) lfirst(args);
*************** transformCoalesceExpr(ParseState *pstate
*** 2205,2212 ****
--- 2213,2223 ----
          newcoercedargs = lappend(newcoercedargs, newe);
      }

+     PopSubExprKind(pstate, SUBEXPR_KIND_COALESCE);
+
      newc->args = newcoercedargs;
      newc->location = c->location;
+
      return (Node *) newc;
  }

diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 55853c2..5dd65d0 100644
*** a/src/backend/parser/parse_func.c
--- b/src/backend/parser/parse_func.c
*************** check_srf_call_placement(ParseState *pst
*** 2089,2094 ****
--- 2089,2095 ----
  {
      const char *err;
      bool        errkind;
+     ParseSubExprKind subexprkind;

      /*
       * Check to see if the set-returning function is in an invalid place
*************** check_srf_call_placement(ParseState *pst
*** 2225,2228 ****
--- 2226,2252 ----
                   errmsg("set-returning functions are not allowed in %s",
                          ParseExprKindName(pstate->p_expr_kind)),
                   parser_errposition(pstate, location)));
+
+     /*
+      * Okay, the overall expression type allows a SRF, but what about
+      * subexpressions?    (We don't currently have enough variants of this to
+      * justify a lot of work to merge code; but we do avoid generating extra
+      * translatable strings.)
+      */
+     subexprkind = inSubExprOfKind(pstate,
+                                   SUBEXPR_KIND_CASE | SUBEXPR_KIND_COALESCE);
+     if (subexprkind == SUBEXPR_KIND_CASE)
+         ereport(ERROR,
+                 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+         /* translator: %s is name of a SQL construct, eg GROUP BY */
+                  errmsg("set-returning functions are not allowed in %s",
+                         "CASE"),
+                  parser_errposition(pstate, location)));
+     else if (subexprkind == SUBEXPR_KIND_COALESCE)
+         ereport(ERROR,
+                 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+         /* translator: %s is name of a SQL construct, eg GROUP BY */
+                  errmsg("set-returning functions are not allowed in %s",
+                         "COALESCE"),
+                  parser_errposition(pstate, location)));
  }
diff --git a/src/backend/parser/parse_node.c b/src/backend/parser/parse_node.c
index fb3d117..58ca0a8 100644
*** a/src/backend/parser/parse_node.c
--- b/src/backend/parser/parse_node.c
*************** pcb_error_callback(void *arg)
*** 182,187 ****
--- 182,211 ----


  /*
+  * inSubExprOfKind
+  *        Detect whether we're parsing a subexpression of specified kind(s)
+  *
+  * sexprkinds is an OR'd mask of one or more ParseSubExprKind values.
+  * If any of these appear in the p_subexpr_kind stack, returns the most
+  * closely nested such kind.  If none of them do, returns SUBEXPR_KIND_NONE.
+  */
+ ParseSubExprKind
+ inSubExprOfKind(ParseState *pstate, int sexprkinds)
+ {
+     ListCell   *lc;
+
+     foreach(lc, pstate->p_subexpr_kind)
+     {
+         ParseSubExprKind skind = (ParseSubExprKind) lfirst_int(lc);
+
+         if (skind & sexprkinds)
+             return skind;
+     }
+     return SUBEXPR_KIND_NONE;
+ }
+
+
+ /*
   * make_var
   *        Build a Var node for an attribute identified by RTE and attrno
   */
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index 0b54840..3651be8 100644
*** a/src/include/parser/parse_node.h
--- b/src/include/parser/parse_node.h
*************** typedef enum ParseExprKind
*** 70,75 ****
--- 70,93 ----
      EXPR_KIND_PARTITION_EXPRESSION        /* PARTITION BY expression */
  } ParseExprKind;

+ /*
+  * While ParseExprKind identifies an expression's position within a SQL query,
+  * and thus can't be nested within a given query level, we also have interest
+  * in recognizing certain nestable subexpression contexts.  These are handled
+  * by creating a List of ParseSubExprKind values, with the front of the list
+  * corresponding to the most closely nested context. The list (p_subexpr_kind)
+  * is empty when we start parsing an expression.
+  *
+  * The enum values are assigned so that we can OR them together to form a
+  * mask identifying several subexpr kinds.
+  */
+ typedef enum ParseSubExprKind
+ {
+     SUBEXPR_KIND_NONE = 0,        /* not any subexpr kind */
+     SUBEXPR_KIND_CASE = 0x0001, /* subexpressions of a CASE */
+     SUBEXPR_KIND_COALESCE = 0x0002        /* COALESCE() arguments */
+ } ParseSubExprKind;
+

  /*
   * Function signatures for parser hooks
*************** typedef Node *(*CoerceParamHook) (ParseS
*** 142,147 ****
--- 160,168 ----
   * p_expr_kind: kind of expression we're currently parsing, as per enum above;
   * EXPR_KIND_NONE when not in an expression.
   *
+  * p_subexpr_kind: integer List of identifiers of nested subexpression types,
+  * as per enum above; NIL when not in a subexpression of interest.
+  *
   * p_next_resno: next TargetEntry.resno to assign, starting from 1.
   *
   * p_multiassign_exprs: partially-processed MultiAssignRef source expressions.
*************** struct ParseState
*** 181,186 ****
--- 202,208 ----
      bool        p_is_insert;    /* process assignment like INSERT not UPDATE */
      List       *p_windowdefs;    /* raw representations of window clauses */
      ParseExprKind p_expr_kind;    /* what kind of expression we're parsing */
+     List       *p_subexpr_kind; /* kind(s) of subexpressions we're parsing */
      int            p_next_resno;    /* next targetlist resno to assign */
      List       *p_multiassign_exprs;    /* junk tlist entries for multiassign */
      List       *p_locking_clause;        /* raw FOR UPDATE/FOR SHARE info */
*************** typedef struct ParseCallbackState
*** 254,259 ****
--- 276,288 ----
      ErrorContextCallback errcallback;
  } ParseCallbackState;

+ /* Macros for manipulating p_subexpr_kind list */
+ #define PushSubExprKind(pstate, sexprkind) \
+     ((pstate)->p_subexpr_kind = lcons_int(sexprkind, (pstate)->p_subexpr_kind))
+ #define PopSubExprKind(pstate, sexprkind) \
+     (AssertMacro(linitial_int((pstate)->p_subexpr_kind) == (sexprkind)), \
+      (pstate)->p_subexpr_kind = list_delete_first((pstate)->p_subexpr_kind))
+

  extern ParseState *make_parsestate(ParseState *parentParseState);
  extern void free_parsestate(ParseState *pstate);
*************** extern void setup_parser_errposition_cal
*** 263,268 ****
--- 292,299 ----
                                    ParseState *pstate, int location);
  extern void cancel_parser_errposition_callback(ParseCallbackState *pcbstate);

+ extern ParseSubExprKind inSubExprOfKind(ParseState *pstate, int sexprkinds);
+
  extern Var *make_var(ParseState *pstate, RangeTblEntry *rte, int attrno,
           int location);
  extern Oid    transformArrayType(Oid *arrayType, int32 *arrayTypmod);
diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out
index 4b6170d..5c82614 100644
*** a/src/test/regress/expected/rangefuncs.out
--- b/src/test/regress/expected/rangefuncs.out
*************** select * from foobar();  -- fail
*** 1969,1986 ****
  ERROR:  function return row and query-specified return row do not match
  DETAIL:  Returned row contains 3 attributes, but query expects 2.
  drop function foobar();
- -- check behavior when a function's input sometimes returns a set (bug #8228)
- SELECT *,
-   lower(CASE WHEN id = 2 THEN (regexp_matches(str, '^0*([1-9]\d+)$'))[1]
-         ELSE str
-         END)
- FROM
-   (VALUES (1,''), (2,'0000000049404'), (3,'FROM 10000000876')) v(id, str);
-  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 $$
    select t.q2
--- 1969,1974 ----
diff --git a/src/test/regress/expected/tsrf.out b/src/test/regress/expected/tsrf.out
index c8ae361..ca242f7 100644
*** a/src/test/regress/expected/tsrf.out
--- b/src/test/regress/expected/tsrf.out
*************** SELECT generate_series(1, generate_serie
*** 41,46 ****
--- 41,51 ----
                 3
  (6 rows)

+ -- but we've traditionally rejected the same in FROM
+ SELECT * FROM generate_series(1, generate_series(1, 3));
+ ERROR:  set-valued function called in context that cannot accept a set
+ LINE 1: SELECT * FROM generate_series(1, generate_series(1, 3));
+                                          ^
  -- srf, with two SRF arguments
  SELECT generate_series(generate_series(1,3), generate_series(2, 4));
   generate_series
*************** SELECT few.dataa, count(*) FROM few WHER
*** 190,195 ****
--- 195,209 ----
   a     |     4
  (2 rows)

+ -- SRFs are not allowed if they'd need to be conditionally executed
+ SELECT q1, case when q1 > 0 then generate_series(1,3) else 0 end FROM int8_tbl;
+ ERROR:  set-returning functions are not allowed in CASE
+ LINE 1: SELECT q1, case when q1 > 0 then generate_series(1,3) else 0...
+                                          ^
+ SELECT q1, coalesce(generate_series(1,3), 0) FROM int8_tbl;
+ ERROR:  set-returning functions are not allowed in COALESCE
+ LINE 1: SELECT q1, coalesce(generate_series(1,3), 0) FROM int8_tbl;
+                             ^
  -- SRFs are not allowed in aggregate arguments
  SELECT min(generate_series(1, 3)) FROM few;
  ERROR:  set-valued function called in context that cannot accept a set
diff --git a/src/test/regress/sql/rangefuncs.sql b/src/test/regress/sql/rangefuncs.sql
index 4ed84b1..442d397 100644
*** a/src/test/regress/sql/rangefuncs.sql
--- b/src/test/regress/sql/rangefuncs.sql
*************** select * from foobar();  -- fail
*** 600,614 ****

  drop function foobar();

- -- check behavior when a function's input sometimes returns a set (bug #8228)
-
- SELECT *,
-   lower(CASE WHEN id = 2 THEN (regexp_matches(str, '^0*([1-9]\d+)$'))[1]
-         ELSE str
-         END)
- FROM
-   (VALUES (1,''), (2,'0000000049404'), (3,'FROM 10000000876')) v(id, str);
-
  -- check whole-row-Var handling in nested lateral functions (bug #11703)

  create function extractq2(t int8_tbl) returns int8 as $$
--- 600,605 ----
diff --git a/src/test/regress/sql/tsrf.sql b/src/test/regress/sql/tsrf.sql
index 417e78c..45de099 100644
*** a/src/test/regress/sql/tsrf.sql
--- b/src/test/regress/sql/tsrf.sql
*************** SELECT generate_series(1, 2), generate_s
*** 14,19 ****
--- 14,22 ----
  -- srf, with SRF argument
  SELECT generate_series(1, generate_series(1, 3));

+ -- but we've traditionally rejected the same in FROM
+ SELECT * FROM generate_series(1, generate_series(1, 3));
+
  -- srf, with two SRF arguments
  SELECT generate_series(generate_series(1,3), generate_series(2, 4));

*************** SELECT dataa, generate_series(1,1), coun
*** 51,56 ****
--- 54,63 ----
  SELECT few.dataa, count(*) FROM few WHERE dataa = 'a' GROUP BY few.dataa ORDER BY 2;
  SELECT few.dataa, count(*) FROM few WHERE dataa = 'a' GROUP BY few.dataa, unnest('{1,1,3}'::int[]) ORDER BY 2;

+ -- SRFs are not allowed if they'd need to be conditionally executed
+ SELECT q1, case when q1 > 0 then generate_series(1,3) else 0 end FROM int8_tbl;
+ SELECT q1, coalesce(generate_series(1,3), 0) FROM int8_tbl;
+
  -- SRFs are not allowed in aggregate arguments
  SELECT min(generate_series(1, 3)) FROM few;

diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql
index cbcd6cf..98bcfa0 100644
*** a/src/backend/catalog/information_schema.sql
--- b/src/backend/catalog/information_schema.sql
*************** CREATE VIEW user_mapping_options AS
*** 2936,2947 ****
      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(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
                       ELSE NULL END AS character_data) AS option_value
!     FROM _pg_user_mappings um;

  GRANT SELECT ON user_mapping_options TO PUBLIC;

--- 2936,2949 ----
      SELECT authorization_identifier,
             foreign_server_catalog,
             foreign_server_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 opts.option_value
                       ELSE NULL END AS character_data) AS option_value
!     FROM _pg_user_mappings um,
!          pg_options_to_table(um.umoptions) opts;

  GRANT SELECT ON user_mapping_options TO PUBLIC;

diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index efe1c37..5241fd2 100644
*** a/src/backend/parser/parse_agg.c
--- b/src/backend/parser/parse_agg.c
*************** check_agg_arguments_walker(Node *node,
*** 705,710 ****
--- 705,717 ----
          }
          /* 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"),
+                  parser_errposition(context->pstate, exprLocation(node))));
      /* We can throw error on sight for a window function */
      if (IsA(node, WindowFunc))
          ereport(ERROR,
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 92101c9..102da58 100644
*** a/src/backend/parser/parse_expr.c
--- b/src/backend/parser/parse_expr.c
*************** transformMultiAssignRef(ParseState *psta
*** 1619,1625 ****
  static Node *
  transformCaseExpr(ParseState *pstate, CaseExpr *c)
  {
!     CaseExpr   *newc;
      Node       *arg;
      CaseTestExpr *placeholder;
      List       *newargs;
--- 1619,1626 ----
  static Node *
  transformCaseExpr(ParseState *pstate, CaseExpr *c)
  {
!     CaseExpr   *newc = makeNode(CaseExpr);
!     Node       *last_srf = pstate->p_last_srf;
      Node       *arg;
      CaseTestExpr *placeholder;
      List       *newargs;
*************** transformCaseExpr(ParseState *pstate, Ca
*** 1628,1635 ****
      Node       *defresult;
      Oid            ptype;

-     newc = makeNode(CaseExpr);
-
      /* transform the test expression, if any */
      arg = transformExprRecurse(pstate, (Node *) c->arg);

--- 1629,1634 ----
*************** transformCaseExpr(ParseState *pstate, Ca
*** 1741,1746 ****
--- 1740,1755 ----
                                    "CASE/WHEN");
      }

+     /* if any subexpression contained a SRF, complain */
+     if (pstate->p_last_srf != last_srf)
+         ereport(ERROR,
+                 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+         /* translator: %s is name of a SQL construct, eg GROUP BY */
+                  errmsg("set-returning functions are not allowed in %s",
+                         "CASE"),
+                  parser_errposition(pstate,
+                                     exprLocation(pstate->p_last_srf))));
+
      newc->location = c->location;

      return (Node *) newc;
*************** static Node *
*** 2177,2182 ****
--- 2186,2192 ----
  transformCoalesceExpr(ParseState *pstate, CoalesceExpr *c)
  {
      CoalesceExpr *newc = makeNode(CoalesceExpr);
+     Node       *last_srf = pstate->p_last_srf;
      List       *newargs = NIL;
      List       *newcoercedargs = NIL;
      ListCell   *args;
*************** transformCoalesceExpr(ParseState *pstate
*** 2205,2210 ****
--- 2215,2230 ----
          newcoercedargs = lappend(newcoercedargs, newe);
      }

+     /* if any subexpression contained a SRF, complain */
+     if (pstate->p_last_srf != last_srf)
+         ereport(ERROR,
+                 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+         /* translator: %s is name of a SQL construct, eg GROUP BY */
+                  errmsg("set-returning functions are not allowed in %s",
+                         "COALESCE"),
+                  parser_errposition(pstate,
+                                     exprLocation(pstate->p_last_srf))));
+
      newc->args = newcoercedargs;
      newc->location = c->location;
      return (Node *) newc;
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 55853c2..9c761c4 100644
*** a/src/backend/parser/parse_func.c
--- b/src/backend/parser/parse_func.c
*************** ParseFuncOrColumn(ParseState *pstate, Li
*** 771,776 ****
--- 771,780 ----
          retval = (Node *) wfunc;
      }

+     /* if it returns a set, remember it for error checks at higher levels */
+     if (retset)
+         pstate->p_last_srf = retval;
+
      return retval;
  }

diff --git a/src/backend/parser/parse_oper.c b/src/backend/parser/parse_oper.c
index e40b10d..b2f9a9c 100644
*** a/src/backend/parser/parse_oper.c
--- b/src/backend/parser/parse_oper.c
*************** make_op(ParseState *pstate, List *opname
*** 843,849 ****
--- 843,853 ----

      /* if it returns a set, check that's OK */
      if (result->opretset)
+     {
          check_srf_call_placement(pstate, location);
+         /* ... and remember it for error checks at higher levels */
+         pstate->p_last_srf = (Node *) result;
+     }

      ReleaseSysCache(tup);

diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index 0b54840..6a3507f 100644
*** a/src/include/parser/parse_node.h
--- b/src/include/parser/parse_node.h
*************** typedef Node *(*CoerceParamHook) (ParseS
*** 157,162 ****
--- 157,165 ----
   * p_hasAggs, p_hasWindowFuncs, etc: true if we've found any of the indicated
   * constructs in the query.
   *
+  * p_last_srf: the set-returning FuncExpr or OpExpr most recently found in
+  * the query, or NULL if none.
+  *
   * p_pre_columnref_hook, etc: optional parser hook functions for modifying the
   * interpretation of ColumnRefs and ParamRefs.
   *
*************** struct ParseState
*** 199,204 ****
--- 202,209 ----
      bool        p_hasSubLinks;
      bool        p_hasModifyingCTE;

+     Node       *p_last_srf;        /* most recent set-returning func/op found */
+
      /*
       * Optional hook functions for parser callbacks.  These are null unless
       * set up by the caller of make_parsestate.
diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out
index 4b6170d..5c82614 100644
*** a/src/test/regress/expected/rangefuncs.out
--- b/src/test/regress/expected/rangefuncs.out
*************** select * from foobar();  -- fail
*** 1969,1986 ****
  ERROR:  function return row and query-specified return row do not match
  DETAIL:  Returned row contains 3 attributes, but query expects 2.
  drop function foobar();
- -- check behavior when a function's input sometimes returns a set (bug #8228)
- SELECT *,
-   lower(CASE WHEN id = 2 THEN (regexp_matches(str, '^0*([1-9]\d+)$'))[1]
-         ELSE str
-         END)
- FROM
-   (VALUES (1,''), (2,'0000000049404'), (3,'FROM 10000000876')) v(id, str);
-  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 $$
    select t.q2
--- 1969,1974 ----
diff --git a/src/test/regress/expected/tsrf.out b/src/test/regress/expected/tsrf.out
index c8ae361..6e91ede 100644
*** a/src/test/regress/expected/tsrf.out
--- b/src/test/regress/expected/tsrf.out
*************** SELECT generate_series(1, generate_serie
*** 41,46 ****
--- 41,51 ----
                 3
  (6 rows)

+ -- but we've traditionally rejected the same in FROM
+ SELECT * FROM generate_series(1, generate_series(1, 3));
+ ERROR:  set-valued function called in context that cannot accept a set
+ LINE 1: SELECT * FROM generate_series(1, generate_series(1, 3));
+                                          ^
  -- srf, with two SRF arguments
  SELECT generate_series(generate_series(1,3), generate_series(2, 4));
   generate_series
*************** SELECT few.dataa, count(*) FROM few WHER
*** 190,198 ****
   a     |     4
  (2 rows)

  -- SRFs are not allowed in aggregate arguments
  SELECT min(generate_series(1, 3)) FROM few;
! ERROR:  set-valued function called in context that cannot accept a set
  LINE 1: SELECT min(generate_series(1, 3)) FROM few;
                     ^
  -- SRFs are not allowed in window function arguments, either
--- 195,212 ----
   a     |     4
  (2 rows)

+ -- SRFs are not allowed if they'd need to be conditionally executed
+ SELECT q1, case when q1 > 0 then generate_series(1,3) else 0 end FROM int8_tbl;
+ ERROR:  set-returning functions are not allowed in CASE
+ LINE 1: SELECT q1, case when q1 > 0 then generate_series(1,3) else 0...
+                                          ^
+ SELECT q1, coalesce(generate_series(1,3), 0) FROM int8_tbl;
+ ERROR:  set-returning functions are not allowed in COALESCE
+ LINE 1: SELECT q1, coalesce(generate_series(1,3), 0) FROM int8_tbl;
+                             ^
  -- SRFs are not allowed in aggregate arguments
  SELECT min(generate_series(1, 3)) FROM few;
! ERROR:  aggregate function calls cannot contain set-returning function calls
  LINE 1: SELECT min(generate_series(1, 3)) FROM few;
                     ^
  -- SRFs are not allowed in window function arguments, either
diff --git a/src/test/regress/sql/rangefuncs.sql b/src/test/regress/sql/rangefuncs.sql
index 4ed84b1..442d397 100644
*** a/src/test/regress/sql/rangefuncs.sql
--- b/src/test/regress/sql/rangefuncs.sql
*************** select * from foobar();  -- fail
*** 600,614 ****

  drop function foobar();

- -- check behavior when a function's input sometimes returns a set (bug #8228)
-
- SELECT *,
-   lower(CASE WHEN id = 2 THEN (regexp_matches(str, '^0*([1-9]\d+)$'))[1]
-         ELSE str
-         END)
- FROM
-   (VALUES (1,''), (2,'0000000049404'), (3,'FROM 10000000876')) v(id, str);
-
  -- check whole-row-Var handling in nested lateral functions (bug #11703)

  create function extractq2(t int8_tbl) returns int8 as $$
--- 600,605 ----
diff --git a/src/test/regress/sql/tsrf.sql b/src/test/regress/sql/tsrf.sql
index 417e78c..45de099 100644
*** a/src/test/regress/sql/tsrf.sql
--- b/src/test/regress/sql/tsrf.sql
*************** SELECT generate_series(1, 2), generate_s
*** 14,19 ****
--- 14,22 ----
  -- srf, with SRF argument
  SELECT generate_series(1, generate_series(1, 3));

+ -- but we've traditionally rejected the same in FROM
+ SELECT * FROM generate_series(1, generate_series(1, 3));
+
  -- srf, with two SRF arguments
  SELECT generate_series(generate_series(1,3), generate_series(2, 4));

*************** SELECT dataa, generate_series(1,1), coun
*** 51,56 ****
--- 54,63 ----
  SELECT few.dataa, count(*) FROM few WHERE dataa = 'a' GROUP BY few.dataa ORDER BY 2;
  SELECT few.dataa, count(*) FROM few WHERE dataa = 'a' GROUP BY few.dataa, unnest('{1,1,3}'::int[]) ORDER BY 2;

+ -- SRFs are not allowed if they'd need to be conditionally executed
+ SELECT q1, case when q1 > 0 then generate_series(1,3) else 0 end FROM int8_tbl;
+ SELECT q1, coalesce(generate_series(1,3), 0) FROM int8_tbl;
+
  -- SRFs are not allowed in aggregate arguments
  SELECT min(generate_series(1, 3)) FROM few;


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