Don't include CaseTestExpr in JsonValueExpr.formatted_expr

A CaseTestExpr is currently being 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, especially as there is no
special reason to evaluate it separately.  So this commit makes it
so.  As a result, JsonValueExpr.raw_expr no longer needs to be
evaluated in ExecInterpExpr(), eval_const_exprs_mutator() etc. and
is now only used for displaying the original "unformatted"
expression in ruleutils.c.

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 also using it.

Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index e6e616865c23153defa9bc0be70644e1985c2900..bf3a08c5f08d935c90d8e9fe7f5819f396259051 100644 (file)
--- 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 39e1884cf436c546ae50e1dda66a4980d148b71b..c6c310d2531ac37b0cd6cbe498f91410d4f8a465 100644 (file)
--- 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 7f453b04f8bed73cb3415499e42c5d8ae43dd222..da258968b8c30806afb8027b8d47e5a31bd738ef 100644 (file)
--- 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 5bf790cf0fe946374642092fead019c4af7216e1..26344743e40f4042afe8d2ab82954ee8e049754b 100644 (file)
--- 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 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 792a743f72ea1a14cd2aabbe9c63a86effdf5070..0d2df069b3b06b064d4815f526035a2c337ed169 100644 (file)
--- 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;