From 97a39f295fd38a6083dfac96a0f1745f60ed5f9a Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed, 8 Jan 2014 20:18:13 -0500
Subject: [PATCH] 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.
---
src/backend/executor/execQual.c | 55 ++++++++++++++++--------
src/test/regress/expected/rangefuncs.out | 14 ++++++
src/test/regress/sql/rangefuncs.sql | 9 ++++
3 files changed, 61 insertions(+), 17 deletions(-)
diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index 85c8d15560f..d1132e0b26a 100644
--- a/src/backend/executor/execQual.c
+++ b/src/backend/executor/execQual.c
@@ -1631,9 +1631,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,
@@ -1895,13 +1893,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;
@@ -2360,10 +2357,22 @@ ExecEvalFunc(FuncExprState *fcache,
init_fcache(func->funcid, func->inputcollid, 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);
+ }
}
/* ----------------------------------------------------------------
@@ -2383,10 +2392,22 @@ ExecEvalOper(FuncExprState *fcache,
init_fcache(op->opfuncid, op->inputcollid, 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 7a2c81caf17..bc826f69419 100644
--- a/src/test/regress/expected/rangefuncs.out
+++ b/src/test/regress/expected/rangefuncs.out
@@ -930,3 +930,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, '^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 9a333c266ea..e0779ba8dd4 100644
--- a/src/test/regress/sql/rangefuncs.sql
+++ b/src/test/regress/sql/rangefuncs.sql
@@ -455,3 +455,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, '^0*([1-9]\d+)$'))[1]
+ ELSE str
+ END)
+FROM
+ (VALUES (1,''), (2,'0000000049404'), (3,'FROM 10000000876')) v(id, str);
--
GitLab