Fix "cannot accept a set" error when only some arms of a CASE return a set.

In commit c1352052ef1d4eeb2eb1d822a207ddc2d106cb13, I implemented an
optimization that assumed that a function's argument expressions would
either always return a set (ie multiple rows), or always not.  This is
wrong however: we allow CASE expressions in which some arms return a set
of some type and others just return a scalar of that type.  There may be
other examples as well.  To fix, replace the run-time test of whether an
argument returned a set with a static precheck (expression_returns_set).
This adds a little bit of query startup overhead, but it seems barely
measurable.

Per bug #8228 from David Johnston.  This has been broken since 8.0,
so patch all supported branches.

diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index 79e0cc76381974f0cf41dcd849f291bc9c5529d1..d3c2f437d11cc17bf03f05e5dcff2fe7a7cc1d3f 100644 (file)
--- a/src/backend/executor/execQual.c
+++ b/src/backend/executor/execQual.c
@@ -1557,9 +1557,7 @@ tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc)
  * init_fcache is presumed already run on the FuncExprState.
  *
  * This function handles the most general case, wherein the function or
- * one of its arguments might (or might not) return a set.     If we find
- * no sets involved, we will change the FuncExprState's function pointer
- * to use a simpler method on subsequent calls.
+ * one of its arguments can return a set.
  */
 static Datum
 ExecMakeFunctionResult(FuncExprState *fcache,
@@ -1838,13 +1836,12 @@ restart:
                /*
                 * Non-set case: much easier.
                 *
-                * We change the ExprState function pointer to use the simpler
-                * ExecMakeFunctionResultNoSets on subsequent calls.  This amounts to
-                * assuming that no argument can return a set if it didn't do so the
-                * first time.
+                * In common cases, this code path is unreachable because we'd have
+                * selected ExecMakeFunctionResultNoSets instead.  However, it's
+                * possible to get here if an argument sometimes produces set results
+                * and sometimes scalar results.  For example, a CASE expression might
+                * call a set-returning function in only some of its arms.
                 */
-               fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResultNoSets;
-
                if (isDone)
                        *isDone = ExprSingleResult;
 
@@ -2300,10 +2297,22 @@ ExecEvalFunc(FuncExprState *fcache,
        /* Initialize function lookup info */
        init_fcache(func->funcid, fcache, econtext->ecxt_per_query_memory, true);
 
-       /* Go directly to ExecMakeFunctionResult on subsequent uses */
-       fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResult;
-
-       return ExecMakeFunctionResult(fcache, econtext, isNull, isDone);
+       /*
+        * 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))
+       {
+               fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResult;
+               return ExecMakeFunctionResult(fcache, econtext, isNull, isDone);
+       }
+       else
+       {
+               fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResultNoSets;
+               return ExecMakeFunctionResultNoSets(fcache, econtext, isNull, isDone);
+       }
 }
 
 /* ----------------------------------------------------------------
@@ -2322,10 +2331,22 @@ ExecEvalOper(FuncExprState *fcache,
        /* Initialize function lookup info */
        init_fcache(op->opfuncid, fcache, econtext->ecxt_per_query_memory, true);
 
-       /* Go directly to ExecMakeFunctionResult on subsequent uses */
-       fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResult;
-
-       return ExecMakeFunctionResult(fcache, econtext, isNull, isDone);
+       /*
+        * 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))
+       {
+               fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResult;
+               return ExecMakeFunctionResult(fcache, econtext, isNull, isDone);
+       }
+       else
+       {
+               fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResultNoSets;
+               return ExecMakeFunctionResultNoSets(fcache, econtext, isNull, isDone);
+       }
 }
 
 /* ----------------------------------------------------------------

diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out
index b8a6d5e40ab81dbf9b41ab972f8ed7db066596d7..d6fa3ed816cb27b242777a4a39d762be788a1974 100644 (file)
--- a/src/test/regress/expected/rangefuncs.out
+++ b/src/test/regress/expected/rangefuncs.out
@@ -922,3 +922,17 @@ select * from foobar();  -- fail
 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, E'^0*([1-9]\\d+)$'))[1]
+        ELSE str
+        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)
+

diff --git a/src/test/regress/sql/rangefuncs.sql b/src/test/regress/sql/rangefuncs.sql
index 9e1fcd67b4ff2e9f9ff0e0a7654498c86ec38046..42ef61e4c0c7932da77b0807f81485c3e3c2fdd5 100644 (file)
--- a/src/test/regress/sql/rangefuncs.sql
+++ b/src/test/regress/sql/rangefuncs.sql
@@ -449,3 +449,12 @@ $$ select (1, 2.1, 3) $$ language sql;
 select * from foobar();  -- fail
 
 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, E'^0*([1-9]\\d+)$'))[1]
+        ELSE str
+        END)
+FROM
+  (VALUES (1,''), (2,'0000000049404'), (3,'FROM 10000000876')) v(id, str);