Don't include CaseTestExpr in JsonValueExpr.formatted_expr
authorAmit Langote
Fri, 21 Jul 2023 10:28:31 +0000 (19:28 +0900)
committerAmit Langote
Fri, 21 Jul 2023 10:28:31 +0000 (19:28 +0900)
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.  Comments about and the code manipulating
formatted_expr is updated to mention that it is now always set and
is the expression that gives a JsonValueExpr its runtime value.

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.

Backpatched to 16 from the development branch to keep the code in
sync across branches.

Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com

src/backend/executor/execExpr.c
src/backend/nodes/makefuncs.c
src/backend/nodes/nodeFuncs.c
src/backend/optimizer/util/clauses.c
src/backend/parser/gram.y
src/backend/parser/parse_expr.c
src/include/nodes/makefuncs.h
src/include/nodes/primnodes.h

index e6e616865c23153defa9bc0be70644e1985c2900..bf3a08c5f08d935c90d8e9fe7f5819f396259051 100644 (file)
@@ -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;
            }
 
index 39e1884cf436c546ae50e1dda66a4980d148b71b..0e7e6e46d943b79c36825ee0f47fbad6eccd46d1 100644 (file)
@@ -848,12 +848,13 @@ makeJsonFormat(JsonFormatType type, JsonEncoding encoding, int location)
  *   creates a JsonValueExpr node
  */
 JsonValueExpr *
-makeJsonValueExpr(Expr *expr, JsonFormat *format)
+makeJsonValueExpr(Expr *raw_expr, Expr *formatted_expr,
+                 JsonFormat *format)
 {
    JsonValueExpr *jve = makeNode(JsonValueExpr);
 
-   jve->raw_expr = expr;
-   jve->formatted_expr = NULL;
+   jve->raw_expr = raw_expr;
+   jve->formatted_expr = formatted_expr;
    jve->format = format;
 
    return jve;
index c41e6bb984cddc52cb629ea105621314e9b088df..503d76aae07ebf19dd1b03a7ff0bb1f03031c080 100644 (file)
@@ -225,9 +225,7 @@ exprType(const Node *expr)
            {
                const JsonValueExpr *jve = (const JsonValueExpr *) expr;
 
-               type = exprType((Node *)
-                               (jve->formatted_expr ? jve->formatted_expr :
-                                jve->raw_expr));
+               type = exprType((Node *) jve->formatted_expr);
            }
            break;
        case T_JsonConstructorExpr:
index 7f453b04f8bed73cb3415499e42c5d8ae43dd222..da258968b8c30806afb8027b8d47e5a31bd738ef 100644 (file)
@@ -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;
            }
 
index edb6c00ece6294edb6c7db384a73505cffb372c1..c31b3733587d64b736af3f8e9158ab3b3f5931bd 100644 (file)
@@ -16353,7 +16353,9 @@ opt_asymmetric: ASYMMETRIC
 json_value_expr:
            a_expr json_format_clause_opt
            {
-               $$ = (Node *) makeJsonValueExpr((Expr *) $1, castNode(JsonFormat, $2));
+               /* formatted_expr will be set during parse-analysis. */
+               $$ = (Node *) makeJsonValueExpr((Expr *) $1, NULL,
+                                               castNode(JsonFormat, $2));
            }
        ;
 
index 286e85d72618477022f480e3608822143b5fd2d9..c08c06373a9ad82900c23fba45a189a15ea3b3b2 100644 (file)
@@ -3202,24 +3202,13 @@ 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.
+ *
+ * Returned expression is either ve->raw_expr coerced to text (if needed) or
+ * a JsonValueExpr with formatted_expr set to the coerced copy of raw_expr
+ * if the specified format requires it.
  */
 static Node *
 transformJsonValueExpr(ParseState *pstate, const char *constructName,
@@ -3268,11 +3257,8 @@ transformJsonValueExpr(ParseState *pstate, const 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 +3296,7 @@ transformJsonValueExpr(ParseState *pstate, const char *constructName,
            coerced = (Node *) fexpr;
        }
 
-       if (coerced == orig)
+       if (coerced == expr)
            expr = rawexpr;
        else
        {
@@ -3322,6 +3308,10 @@ transformJsonValueExpr(ParseState *pstate, const char *constructName,
        }
    }
 
+   /* If returning a JsonValueExpr, formatted_expr must have been set. */
+   Assert(!IsA(expr, JsonValueExpr) ||
+          ((JsonValueExpr *) expr)->formatted_expr != NULL);
+
    return expr;
 }
 
@@ -3537,8 +3527,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);
@@ -3635,7 +3639,12 @@ transformJsonArrayQueryConstructor(ParseState *pstate,
                                makeString(pstrdup("a")));
    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 = makeJsonValueExpr((Expr *) colref, (Expr *) colref,
+                                ctor->format);
    agg->absent_on_null = ctor->absent_on_null;
    agg->constructor = makeNode(JsonAggConstructor);
    agg->constructor->agg_order = NIL;
@@ -3900,13 +3909,11 @@ transformJsonParseArg(ParseState *pstate, Node *jsexpr, JsonFormat *format,
    {
        JsonValueExpr *jve;
 
-       expr = makeCaseTestExpr(raw_expr);
+       expr = raw_expr;
        expr = makeJsonByteaToTextConversion(expr, format, exprLocation(expr));
        *exprtype = TEXTOID;
 
-       jve = makeJsonValueExpr((Expr *) raw_expr, format);
-
-       jve->formatted_expr = (Expr *) expr;
+       jve = makeJsonValueExpr((Expr *) raw_expr, (Expr *) expr, format);
        expr = (Node *) jve;
    }
    else
index 06d991b7257f7c61b089a74c7aba4afc7eb11e33..3180703005507a4893cf17d32616ee1437898bc8 100644 (file)
@@ -110,7 +110,8 @@ extern VacuumRelation *makeVacuumRelation(RangeVar *relation, Oid oid, List *va_
 
 extern JsonFormat *makeJsonFormat(JsonFormatType type, JsonEncoding encoding,
                                  int location);
-extern JsonValueExpr *makeJsonValueExpr(Expr *expr, JsonFormat *format);
+extern JsonValueExpr *makeJsonValueExpr(Expr *raw_expr, Expr *formatted_expr,
+                                       JsonFormat *format);
 extern Node *makeJsonKeyValue(Node *key, Node *value);
 extern Node *makeJsonIsPredicate(Node *expr, JsonFormat *format,
                                 JsonValueType item_type, bool unique_keys,
index 792a743f72ea1a14cd2aabbe9c63a86effdf5070..e1aadc39cfbe4daf1d6b3c38ef2189efa1196d86 100644 (file)
@@ -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,16 @@ typedef struct JsonReturning
 /*
  * JsonValueExpr -
  *     representation of JSON value expression (expr [FORMAT JsonFormat])
+ *
+ * The actual value is obtained by evaluating formatted_expr.  raw_expr is
+ * only there for displaying the original user-written expression 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;