diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index 69bf65d00bf..cbb76d1f1cd 100644
--- a/src/backend/executor/execQual.c
+++ b/src/backend/executor/execQual.c
@@ -2970,19 +2970,30 @@ ExecEvalCase(CaseExprState *caseExpr, ExprContext *econtext,
/*
* If there's a test expression, we have to evaluate it and save the value
- * where the CaseTestExpr placeholders can find it. We must save and
+ * where the CaseTestExpr placeholders can find it. We must save and
* restore prior setting of econtext's caseValue fields, in case this node
- * is itself within a larger CASE.
+ * is itself within a larger CASE. Furthermore, don't assign to the
+ * econtext fields until after returning from evaluation of the test
+ * expression. We used to pass &econtext->caseValue_isNull to the
+ * recursive call, but that leads to aliasing that variable within said
+ * call, which can (and did) produce bugs when the test expression itself
+ * contains a CASE.
+ *
+ * If there's no test expression, we don't actually need to save and
+ * restore these fields; but it's less code to just do so unconditionally.
*/
save_datum = econtext->caseValue_datum;
save_isNull = econtext->caseValue_isNull;
if (caseExpr->arg)
{
+ bool arg_isNull;
+
econtext->caseValue_datum = ExecEvalExpr(caseExpr->arg,
econtext,
- &econtext->caseValue_isNull,
+ &arg_isNull,
NULL);
+ econtext->caseValue_isNull = arg_isNull;
}
/*
@@ -2994,10 +3005,11 @@ ExecEvalCase(CaseExprState *caseExpr, ExprContext *econtext,
{
CaseWhenState *wclause = lfirst(clause);
Datum clause_value;
+ bool clause_isNull;
clause_value = ExecEvalExpr(wclause->expr,
econtext,
- isNull,
+ &clause_isNull,
NULL);
/*
@@ -3005,7 +3017,7 @@ ExecEvalCase(CaseExprState *caseExpr, ExprContext *econtext,
* statement is satisfied. A NULL result from the test is not
* considered true.
*/
- if (DatumGetBool(clause_value) && !*isNull)
+ if (DatumGetBool(clause_value) && !clause_isNull)
{
econtext->caseValue_datum = save_datum;
econtext->caseValue_isNull = save_isNull;
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index a69af7cd7d2..4e23898ff91 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -106,6 +106,8 @@ static bool contain_volatile_functions_not_nextval_walker(Node *node, void *cont
static bool has_parallel_hazard_walker(Node *node,
has_parallel_hazard_arg *context);
static bool contain_nonstrict_functions_walker(Node *node, void *context);
+static bool contain_context_dependent_node(Node *clause);
+static bool contain_context_dependent_node_walker(Node *node, int *flags);
static bool contain_leaked_vars_walker(Node *node, void *context);
static Relids find_nonnullable_rels_walker(Node *node, bool top_level);
static List *find_nonnullable_vars_walker(Node *node, bool top_level);
@@ -1334,6 +1336,76 @@ contain_nonstrict_functions_walker(Node *node, void *context)
context);
}
+/*****************************************************************************
+ * Check clauses for context-dependent nodes
+ *****************************************************************************/
+
+/*
+ * contain_context_dependent_node
+ * Recursively search for context-dependent nodes within a clause.
+ *
+ * CaseTestExpr nodes must appear directly within the corresponding CaseExpr,
+ * not nested within another one, or they'll see the wrong test value. If one
+ * appears "bare" in the arguments of a SQL function, then we can't inline the
+ * SQL function for fear of creating such a situation.
+ *
+ * CoerceToDomainValue would have the same issue if domain CHECK expressions
+ * could get inlined into larger expressions, but presently that's impossible.
+ * Still, it might be allowed in future, or other node types with similar
+ * issues might get invented. So give this function a generic name, and set
+ * up the recursion state to allow multiple flag bits.
+ */
+static bool
+contain_context_dependent_node(Node *clause)
+{
+ int flags = 0;
+
+ return contain_context_dependent_node_walker(clause, &flags);
+}
+
+#define CCDN_IN_CASEEXPR 0x0001 /* CaseTestExpr okay here? */
+
+static bool
+contain_context_dependent_node_walker(Node *node, int *flags)
+{
+ if (node == NULL)
+ return false;
+ if (IsA(node, CaseTestExpr))
+ return !(*flags & CCDN_IN_CASEEXPR);
+ if (IsA(node, CaseExpr))
+ {
+ CaseExpr *caseexpr = (CaseExpr *) node;
+
+ /*
+ * If this CASE doesn't have a test expression, then it doesn't create
+ * a context in which CaseTestExprs should appear, so just fall
+ * through and treat it as a generic expression node.
+ */
+ if (caseexpr->arg)
+ {
+ int save_flags = *flags;
+ bool res;
+
+ /*
+ * Note: in principle, we could distinguish the various sub-parts
+ * of a CASE construct and set the flag bit only for some of them,
+ * since we are only expecting CaseTestExprs to appear in the
+ * "expr" subtree of the CaseWhen nodes. But it doesn't really
+ * seem worth any extra code. If there are any bare CaseTestExprs
+ * elsewhere in the CASE, something's wrong already.
+ */
+ *flags |= CCDN_IN_CASEEXPR;
+ res = expression_tree_walker(node,
+ contain_context_dependent_node_walker,
+ (void *) flags);
+ *flags = save_flags;
+ return res;
+ }
+ }
+ return expression_tree_walker(node, contain_context_dependent_node_walker,
+ (void *) flags);
+}
+
/*****************************************************************************
* Check clauses for Vars passed to non-leakproof functions
*****************************************************************************/
@@ -4178,6 +4250,8 @@ evaluate_function(Oid funcid, Oid result_type, int32 result_typmod,
* doesn't work in the general case because it discards information such
* as OUT-parameter declarations.
*
+ * Also, context-dependent expression nodes in the argument list are trouble.
+ *
* Returns a simplified expression if successful, or NULL if cannot
* simplify the function.
*/
@@ -4372,6 +4446,13 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid,
contain_nonstrict_functions(newexpr))
goto fail;
+ /*
+ * If any parameter expression contains a context-dependent node, we can't
+ * inline, for fear of putting such a node into the wrong context.
+ */
+ if (contain_context_dependent_node((Node *) args))
+ goto fail;
+
/*
* We may be able to do it; there are still checks on parameter usage to
* make, but those are most easily done in combination with the actual
diff --git a/src/test/regress/expected/case.out b/src/test/regress/expected/case.out
index c564eedb948..35b6476e501 100644
--- a/src/test/regress/expected/case.out
+++ b/src/test/regress/expected/case.out
@@ -296,8 +296,52 @@ SELECT * FROM CASE_TBL;
-8 | 10.1
(4 rows)
+--
+-- Nested CASE expressions
+--
+-- This test exercises a bug caused by aliasing econtext->caseValue_isNull
+-- with the isNull argument of the inner CASE's ExecEvalCase() call. After
+-- evaluating the vol(null) expression in the inner CASE's second WHEN-clause,
+-- the isNull flag for the case test value incorrectly became true, causing
+-- the third WHEN-clause not to match. The volatile function calls are needed
+-- to prevent constant-folding in the planner, which would hide the bug.
+CREATE FUNCTION vol(text) returns text as
+ 'begin return $1; end' language plpgsql volatile;
+SELECT CASE
+ (CASE vol('bar')
+ WHEN 'foo' THEN 'it was foo!'
+ WHEN vol(null) THEN 'null input'
+ WHEN 'bar' THEN 'it was bar!' END
+ )
+ WHEN 'it was foo!' THEN 'foo recognized'
+ WHEN 'it was bar!' THEN 'bar recognized'
+ ELSE 'unrecognized' END;
+ case
+----------------
+ bar recognized
+(1 row)
+
+-- In this case, we can't inline the SQL function without confusing things.
+CREATE DOMAIN foodomain AS text;
+CREATE FUNCTION volfoo(text) returns foodomain as
+ 'begin return $1::foodomain; end' language plpgsql volatile;
+CREATE FUNCTION inline_eq(foodomain, foodomain) returns boolean as
+ 'SELECT CASE $2::text WHEN $1::text THEN true ELSE false END' language sql;
+CREATE OPERATOR = (procedure = inline_eq,
+ leftarg = foodomain, rightarg = foodomain);
+SELECT CASE volfoo('bar') WHEN 'foo'::foodomain THEN 'is foo' ELSE 'is not foo' END;
+ case
+------------
+ is not foo
+(1 row)
+
--
-- Clean up
--
DROP TABLE CASE_TBL;
DROP TABLE CASE2_TBL;
+DROP OPERATOR = (foodomain, foodomain);
+DROP FUNCTION inline_eq(foodomain, foodomain);
+DROP FUNCTION volfoo(text);
+DROP DOMAIN foodomain;
+DROP FUNCTION vol(text);
diff --git a/src/test/regress/sql/case.sql b/src/test/regress/sql/case.sql
index 5f41753337d..b2377e46109 100644
--- a/src/test/regress/sql/case.sql
+++ b/src/test/regress/sql/case.sql
@@ -156,9 +156,52 @@ UPDATE CASE_TBL
SELECT * FROM CASE_TBL;
+--
+-- Nested CASE expressions
+--
+
+-- This test exercises a bug caused by aliasing econtext->caseValue_isNull
+-- with the isNull argument of the inner CASE's ExecEvalCase() call. After
+-- evaluating the vol(null) expression in the inner CASE's second WHEN-clause,
+-- the isNull flag for the case test value incorrectly became true, causing
+-- the third WHEN-clause not to match. The volatile function calls are needed
+-- to prevent constant-folding in the planner, which would hide the bug.
+
+CREATE FUNCTION vol(text) returns text as
+ 'begin return $1; end' language plpgsql volatile;
+
+SELECT CASE
+ (CASE vol('bar')
+ WHEN 'foo' THEN 'it was foo!'
+ WHEN vol(null) THEN 'null input'
+ WHEN 'bar' THEN 'it was bar!' END
+ )
+ WHEN 'it was foo!' THEN 'foo recognized'
+ WHEN 'it was bar!' THEN 'bar recognized'
+ ELSE 'unrecognized' END;
+
+-- In this case, we can't inline the SQL function without confusing things.
+CREATE DOMAIN foodomain AS text;
+
+CREATE FUNCTION volfoo(text) returns foodomain as
+ 'begin return $1::foodomain; end' language plpgsql volatile;
+
+CREATE FUNCTION inline_eq(foodomain, foodomain) returns boolean as
+ 'SELECT CASE $2::text WHEN $1::text THEN true ELSE false END' language sql;
+
+CREATE OPERATOR = (procedure = inline_eq,
+ leftarg = foodomain, rightarg = foodomain);
+
+SELECT CASE volfoo('bar') WHEN 'foo'::foodomain THEN 'is foo' ELSE 'is not foo' END;
+
--
-- Clean up
--
DROP TABLE CASE_TBL;
DROP TABLE CASE2_TBL;
+DROP OPERATOR = (foodomain, foodomain);
+DROP FUNCTION inline_eq(foodomain, foodomain);
+DROP FUNCTION volfoo(text);
+DROP DOMAIN foodomain;
+DROP FUNCTION vol(text);