From bedf92b475b6ad8b3023a0c3b8975db38c87cff1 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Fri, 7 Jul 2023 20:21:58 +0900 Subject: [PATCH v5 2/7] Don't include CaseTestExpr in JsonValueExpr.formatted_expr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CaseTestExpr is normally (ab)used as a placeholder expression to represent the source value for computing an expression when the expression that provides the source value is computed independently. A CaseTestExpr is currently put into JsonValueExpr.formatted_expr as placeholder for the result of evaluating JsonValueExpr.raw_expr, which in turn is evaluated separately. Though, there's no need for this indirection if raw_expr itself can be embedded into formatted_expr and evaluated as part of evaluating the latter, so this commit makes it so. JsonValueExpr.raw_expr is no longer evaluated by ExecInterpExpr(), eval_const_exprs_mutator(), etc. and only used for displaying the original "unformatted" expression. While at it, this also removes the function makeCaseTestExpr(), because the code in makeJsonConstructorExpr() looks more readable without it, IMO, and isn't used by anyone else either. Finally, a note is added in the comment above CaseTestExpr's definition that JsonConstructorExpr is using it. Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com --- src/backend/executor/execExpr.c | 17 ++--------- src/backend/nodes/makefuncs.c | 4 +++ src/backend/optimizer/util/clauses.c | 23 ++++----------- src/backend/parser/parse_expr.c | 43 ++++++++++++++-------------- src/include/nodes/primnodes.h | 7 ++++- 5 files changed, 39 insertions(+), 55 deletions(-) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index e6e616865c..bf3a08c5f0 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -2294,21 +2294,8 @@ ExecInitExprRec(Expr *node, ExprState *state, { JsonValueExpr *jve = (JsonValueExpr *) node; - ExecInitExprRec(jve->raw_expr, state, resv, resnull); - - if (jve->formatted_expr) - { - Datum *innermost_caseval = state->innermost_caseval; - bool *innermost_isnull = state->innermost_casenull; - - state->innermost_caseval = resv; - state->innermost_casenull = resnull; - - ExecInitExprRec(jve->formatted_expr, state, resv, resnull); - - state->innermost_caseval = innermost_caseval; - state->innermost_casenull = innermost_isnull; - } + Assert(jve->formatted_expr != NULL); + ExecInitExprRec(jve->formatted_expr, state, resv, resnull); break; } diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c index 39e1884cf4..c6c310d253 100644 --- a/src/backend/nodes/makefuncs.c +++ b/src/backend/nodes/makefuncs.c @@ -853,6 +853,10 @@ makeJsonValueExpr(Expr *expr, JsonFormat *format) JsonValueExpr *jve = makeNode(JsonValueExpr); jve->raw_expr = expr; + + /* + * Set after checking the format, if needed, in transformJsonValueExpr(). + */ jve->formatted_expr = NULL; jve->format = format; diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 7f453b04f8..da258968b8 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -2827,25 +2827,12 @@ eval_const_expressions_mutator(Node *node, case T_JsonValueExpr: { JsonValueExpr *jve = (JsonValueExpr *) node; - Node *raw; + Node *formatted; - raw = eval_const_expressions_mutator((Node *) jve->raw_expr, - context); - if (raw && IsA(raw, Const)) - { - Node *formatted; - Node *save_case_val = context->case_val; - - context->case_val = raw; - - formatted = eval_const_expressions_mutator((Node *) jve->formatted_expr, - context); - - context->case_val = save_case_val; - - if (formatted && IsA(formatted, Const)) - return formatted; - } + formatted = eval_const_expressions_mutator((Node *) jve->formatted_expr, + context); + if (formatted && IsA(formatted, Const)) + return formatted; break; } diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 5bf790cf0f..8d7a44408c 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -3202,21 +3202,6 @@ makeJsonByteaToTextConversion(Node *expr, JsonFormat *format, int location) return (Node *) fexpr; } -/* - * Make a CaseTestExpr node. - */ -static Node * -makeCaseTestExpr(Node *expr) -{ - CaseTestExpr *placeholder = makeNode(CaseTestExpr); - - placeholder->typeId = exprType(expr); - placeholder->typeMod = exprTypmod(expr); - placeholder->collation = exprCollation(expr); - - return (Node *) placeholder; -} - /* * Transform JSON value expression using specified input JSON format or * default format otherwise. @@ -3268,11 +3253,8 @@ transformJsonValueExpr(ParseState *pstate, char *constructName, if (format != JS_FORMAT_DEFAULT) { Oid targettype = format == JS_FORMAT_JSONB ? JSONBOID : JSONOID; - Node *orig = makeCaseTestExpr(expr); Node *coerced; - expr = orig; - if (exprtype != BYTEAOID && typcategory != TYPCATEGORY_STRING) ereport(ERROR, errcode(ERRCODE_DATATYPE_MISMATCH), @@ -3310,7 +3292,7 @@ transformJsonValueExpr(ParseState *pstate, char *constructName, coerced = (Node *) fexpr; } - if (coerced == orig) + if (coerced == expr) expr = rawexpr; else { @@ -3537,8 +3519,22 @@ makeJsonConstructorExpr(ParseState *pstate, JsonConstructorType type, jsctor->absent_on_null = absent_on_null; jsctor->location = location; + /* + * Coerce to the RETURNING type and format, if needed. We need to abuse + * CaseTestExpr here as placeholder to pass the result of either evaluating + * 'fexpr' or whatever is produced by ExecEvalJsonConstructor() that is of + * type JSON or JSONB to the coercion function. + */ if (fexpr) - placeholder = makeCaseTestExpr((Node *) fexpr); + { + CaseTestExpr *cte = makeNode(CaseTestExpr); + + cte->typeId = exprType((Node *) fexpr); + cte->typeMod = exprTypmod((Node *) fexpr); + cte->collation = exprCollation((Node *) fexpr); + + placeholder = (Node *) cte; + } else { CaseTestExpr *cte = makeNode(CaseTestExpr); @@ -3636,6 +3632,11 @@ transformJsonArrayQueryConstructor(ParseState *pstate, colref->location = ctor->location; agg->arg = makeJsonValueExpr((Expr *) colref, ctor->format); + /* + * No formatting necessary, so set formatted_expr to be the same as + * raw_expr. + */ + agg->arg->formatted_expr = agg->arg->raw_expr; agg->absent_on_null = ctor->absent_on_null; agg->constructor = makeNode(JsonAggConstructor); agg->constructor->agg_order = NIL; @@ -3900,7 +3901,7 @@ transformJsonParseArg(ParseState *pstate, Node *jsexpr, JsonFormat *format, { JsonValueExpr *jve; - expr = makeCaseTestExpr(raw_expr); + expr = raw_expr; expr = makeJsonByteaToTextConversion(expr, format, exprLocation(expr)); *exprtype = TEXTOID; diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 792a743f72..0d2df069b3 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -1264,6 +1264,8 @@ typedef struct CaseWhen * see build_coercion_expression(). * * Nested FieldStore/SubscriptingRef assignment expressions in INSERT/UPDATE; * see transformAssignmentIndirection(). + * * Placeholder for intermediate results in some SQL/JSON expression nodes, + * such as JsonConstructorExpr. * * 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 @@ -1593,12 +1595,15 @@ typedef struct JsonReturning /* * JsonValueExpr - * representation of JSON value expression (expr [FORMAT JsonFormat]) + * + * Note that raw_expr is only there for displaying and is not evaluated by + * ExecInterpExpr() and eval_const_exprs_mutator(). */ typedef struct JsonValueExpr { NodeTag type; Expr *raw_expr; /* raw expression */ - Expr *formatted_expr; /* formatted expression or NULL */ + Expr *formatted_expr; /* formatted expression */ JsonFormat *format; /* FORMAT clause, if specified */ } JsonValueExpr; -- 2.35.3