diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index e284fd71d75..c5e8634aeda 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1516,10 +1516,11 @@ ExecInitExprRec(Expr *node, ExprState *state, /* * Read from location identified by innermost_caseval. Note * that innermost_caseval could be NULL, if this node isn't - * actually within a CASE structure; some parts of the system - * abuse CaseTestExpr to cause a read of a value externally - * supplied in econtext->caseValue_datum. We'll take care of - * that scenario at runtime. + * actually within a CaseExpr, ArrayCoerceExpr, etc structure. + * That can happen because some parts of the system abuse + * CaseTestExpr to cause a read of a value externally supplied + * in econtext->caseValue_datum. We'll take care of that + * scenario at runtime. */ scratch.opcode = EEOP_CASE_TESTVAL; scratch.d.casetest.value = state->innermost_caseval; diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index ee6f4cdf4da..21bf5dea9c3 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -1452,7 +1452,8 @@ contain_nonstrict_functions_walker(Node *node, void *context) * 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. + * SQL function for fear of creating such a situation. The same applies for + * CaseTestExpr used within the elemexpr of an ArrayCoerceExpr. * * CoerceToDomainValue would have the same issue if domain CHECK expressions * could get inlined into larger expressions, but presently that's impossible. @@ -1468,7 +1469,7 @@ contain_context_dependent_node(Node *clause) return contain_context_dependent_node_walker(clause, &flags); } -#define CCDN_IN_CASEEXPR 0x0001 /* CaseTestExpr okay here? */ +#define CCDN_CASETESTEXPR_OK 0x0001 /* CaseTestExpr okay here? */ static bool contain_context_dependent_node_walker(Node *node, int *flags) @@ -1476,8 +1477,8 @@ 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)) + return !(*flags & CCDN_CASETESTEXPR_OK); + else if (IsA(node, CaseExpr)) { CaseExpr *caseexpr = (CaseExpr *) node; @@ -1499,7 +1500,7 @@ contain_context_dependent_node_walker(Node *node, int *flags) * seem worth any extra code. If there are any bare CaseTestExprs * elsewhere in the CASE, something's wrong already. */ - *flags |= CCDN_IN_CASEEXPR; + *flags |= CCDN_CASETESTEXPR_OK; res = expression_tree_walker(node, contain_context_dependent_node_walker, (void *) flags); @@ -1507,6 +1508,24 @@ contain_context_dependent_node_walker(Node *node, int *flags) return res; } } + else if (IsA(node, ArrayCoerceExpr)) + { + ArrayCoerceExpr *ac = (ArrayCoerceExpr *) node; + int save_flags; + bool res; + + /* Check the array expression */ + if (contain_context_dependent_node_walker((Node *) ac->arg, flags)) + return true; + + /* Check the elemexpr, which is allowed to contain CaseTestExpr */ + save_flags = *flags; + *flags |= CCDN_CASETESTEXPR_OK; + res = contain_context_dependent_node_walker((Node *) ac->elemexpr, + flags); + *flags = save_flags; + return res; + } return expression_tree_walker(node, contain_context_dependent_node_walker, (void *) flags); } @@ -3125,10 +3144,31 @@ eval_const_expressions_mutator(Node *node, } case T_ArrayCoerceExpr: { - ArrayCoerceExpr *ac; + ArrayCoerceExpr *ac = makeNode(ArrayCoerceExpr); + Node *save_case_val; - /* Copy the node and const-simplify its arguments */ - ac = (ArrayCoerceExpr *) ece_generic_processing(node); + /* + * Copy the node and const-simplify its arguments. We can't + * use ece_generic_processing() here because we need to mess + * with case_val only while processing the elemexpr. + */ + memcpy(ac, node, sizeof(ArrayCoerceExpr)); + ac->arg = (Expr *) + eval_const_expressions_mutator((Node *) ac->arg, + context); + + /* + * Set up for the CaseTestExpr node contained in the elemexpr. + * We must prevent it from absorbing any outer CASE value. + */ + save_case_val = context->case_val; + context->case_val = NULL; + + ac->elemexpr = (Expr *) + eval_const_expressions_mutator((Node *) ac->elemexpr, + context); + + context->case_val = save_case_val; /* * If constant argument and the per-element expression is @@ -3142,6 +3182,7 @@ eval_const_expressions_mutator(Node *node, ac->elemexpr && !IsA(ac->elemexpr, CoerceToDomain) && !contain_mutable_functions((Node *) ac->elemexpr)) return ece_evaluate_expr(ac); + return (Node *) ac; } case T_CollateExpr: diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c index c31a5630b2f..065535a26bb 100644 --- a/src/backend/parser/parse_coerce.c +++ b/src/backend/parser/parse_coerce.c @@ -907,7 +907,12 @@ build_coercion_expression(Node *node, sourceBaseTypeId = getBaseTypeAndTypmod(exprType(node), &sourceBaseTypeMod); - /* Set up CaseTestExpr representing one element of source array */ + /* + * Set up a CaseTestExpr representing one element of the source array. + * This is an abuse of CaseTestExpr, but it's OK as long as there + * can't be any CaseExpr or ArrayCoerceExpr within the completed + * elemexpr. + */ ctest->typeId = get_element_type(sourceBaseTypeId); Assert(OidIsValid(ctest->typeId)); ctest->typeMod = sourceBaseTypeMod; diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index 4932e58022b..3d31be38d56 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -691,7 +691,13 @@ transformAssignmentIndirection(ParseState *pstate, if (indirection && !basenode) { - /* Set up a substitution. We reuse CaseTestExpr for this. */ + /* + * Set up a substitution. We abuse CaseTestExpr for this. It's safe + * to do so because the only nodes that will be above the CaseTestExpr + * in the finished expression will be FieldStore and ArrayRef nodes. + * (There could be other stuff in the tree, but it will be within + * other child fields of those node types.) + */ CaseTestExpr *ctest = makeNode(CaseTestExpr); ctest->typeId = targetTypeId; diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 40f6eb03d24..b886ed35349 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -934,8 +934,20 @@ typedef struct CaseWhen * This is effectively like a Param, but can be implemented more simply * since we need only one replacement value at a time. * - * We also use this in nested UPDATE expressions. - * See transformAssignmentIndirection(). + * We also abuse this node type for some other purposes, including: + * * Placeholder for the current array element value in ArrayCoerceExpr; + * see build_coercion_expression(). + * * Nested FieldStore/ArrayRef assignment expressions in INSERT/UPDATE; + * see transformAssignmentIndirection(). + * + * The uses in CaseExpr and ArrayCoerceExpr are safe only to the extent that + * there is not any other CaseExpr or ArrayCoerceExpr between the value source + * node and its child CaseTestExpr(s). This is true in the parse analysis + * output, but the planner's function-inlining logic has to be careful not to + * break it. + * + * The nested-assignment-expression case is safe because the only node types + * that can be above such CaseTestExprs are FieldStore and ArrayRef. */ typedef struct CaseTestExpr { diff --git a/src/test/regress/expected/case.out b/src/test/regress/expected/case.out index 36bf15c4acb..c0c8acf035a 100644 --- a/src/test/regress/expected/case.out +++ b/src/test/regress/expected/case.out @@ -372,6 +372,20 @@ SELECT CASE make_ad(1,2) right (1 row) +ROLLBACK; +-- Test interaction of CASE with ArrayCoerceExpr (bug #15471) +BEGIN; +CREATE TYPE casetestenum AS ENUM ('e', 'f', 'g'); +SELECT + CASE 'foo'::text + WHEN 'foo' THEN ARRAY['a', 'b', 'c', 'd'] || enum_range(NULL::casetestenum)::text[] + ELSE ARRAY['x', 'y'] + END; + array +----------------- + {a,b,c,d,e,f,g} +(1 row) + ROLLBACK; -- -- Clean up diff --git a/src/test/regress/sql/case.sql b/src/test/regress/sql/case.sql index 66b6e98fb18..17436c524a7 100644 --- a/src/test/regress/sql/case.sql +++ b/src/test/regress/sql/case.sql @@ -233,6 +233,19 @@ SELECT CASE make_ad(1,2) ROLLBACK; +-- Test interaction of CASE with ArrayCoerceExpr (bug #15471) +BEGIN; + +CREATE TYPE casetestenum AS ENUM ('e', 'f', 'g'); + +SELECT + CASE 'foo'::text + WHEN 'foo' THEN ARRAY['a', 'b', 'c', 'd'] || enum_range(NULL::casetestenum)::text[] + ELSE ARRAY['x', 'y'] + END; + +ROLLBACK; + -- -- Clean up --