From 47ac4473ac018f1ec5af5efb2d202646169232eb Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 8 Jan 2014 20:18:10 -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 138818313b..0beb2739b2 100644 --- a/src/backend/executor/execQual.c +++ b/src/backend/executor/execQual.c @@ -1634,9 +1634,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, @@ -1906,13 +1904,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; @@ -2371,10 +2368,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); + } } /* ---------------------------------------------------------------- @@ -2394,10 +2403,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 b71cb8420e..8c61c0c15e 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 462c6663f2..1d0b0c82d4 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); -- 2.40.0