From f8ab9db37ee139cfe5b875370d90398a459fc769 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 29 Jan 2025 20:12:05 -0500 Subject: [PATCH v1 4/8] Convert JsonConstructorExpr & JsonExpr to use a Param. More crank-turning. --- src/backend/executor/execExpr.c | 34 ++++-- src/backend/optimizer/util/clauses.c | 143 ++++++++++++++++++++++++++ src/backend/parser/parse_expr.c | 7 +- src/backend/parser/parse_jsontable.c | 11 ++ src/backend/utils/adt/jsonpath_exec.c | 12 +-- src/include/nodes/primnodes.h | 10 ++ 6 files changed, 199 insertions(+), 18 deletions(-) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 13185652ec..80709f14de 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -2465,16 +2465,20 @@ ExecInitExprRec(Expr *node, ExprState *state, if (ctor->coercion) { - Datum *innermost_caseval = state->innermost_caseval; - bool *innermost_isnull = state->innermost_casenull; - - state->innermost_caseval = resv; - state->innermost_casenull = resnull; + /* + * Apply the coercion expression to the result-so-far. We + * assume it has a PARAM_EXPR Param representing the input + * value. Push a step that copies the current result into + * the appropriate Param slot. + */ + scratch.opcode = EEOP_PARAM_SET_EXPR; + Assert(ctor->coparam > 0); + scratch.d.paramset.paramid = ctor->coparam; + scratch.d.paramset.setvalue = resv; + scratch.d.paramset.setnull = resnull; + ExprEvalPushStep(state, &scratch); ExecInitExprRec(ctor->coercion, state, resv, resnull); - - state->innermost_caseval = innermost_caseval; - state->innermost_casenull = innermost_isnull; } } break; @@ -2496,6 +2500,20 @@ ExecInitExprRec(Expr *node, ExprState *state, { JsonExpr *jsexpr = castNode(JsonExpr, node); + /* + * If the expression involves a Param, the caller will have + * loaded the source value into resv/resnull. Push a step + * that copies that into the appropriate Param slot. + */ + if (jsexpr->feparam > 0) + { + scratch.opcode = EEOP_PARAM_SET_EXPR; + scratch.d.paramset.paramid = jsexpr->feparam; + scratch.d.paramset.setvalue = resv; + scratch.d.paramset.setnull = resnull; + ExprEvalPushStep(state, &scratch); + } + /* * No need to initialize a full JsonExprState For * JSON_TABLE(), because the upstream caller tfuncFetchRows() diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 8aa7ae8cac..d638a290ec 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -3734,6 +3734,149 @@ eval_const_expressions_mutator(Node *node, return ece_evaluate_expr((Node *) newcre); return (Node *) newcre; } + case T_JsonConstructorExpr: + { + JsonConstructorExpr *jce = makeNode(JsonConstructorExpr); + CaseTestExpr *ctexpr = NULL; + + /* + * 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 coercion expr. + */ + memcpy(jce, node, sizeof(JsonConstructorExpr)); + jce->args = (List *) + eval_const_expressions_mutator((Node *) jce->args, + context); + jce->func = (Expr *) + eval_const_expressions_mutator((Node *) jce->func, + context); + + /* + * Find the CaseTestExpr in coercion; this is simpler than + * recomputing its type/typmod/collation from the values we + * have. Also, if we don't find it, we know that we are + * re-simplifying an already-simplified JsonConstructorExpr + * (or that coercion is empty) and must not change coparam. + */ + if (find_casetestexpr((Node *) jce->coercion, &ctexpr)) + { + /* + * Set up the node to substitute for the CaseTestExpr node + * contained in coercion. Normally we replace it with a + * PARAM_EXPR Param, but if we're just estimating, leave + * it alone. + * + * We must save and restore context->case_val so as not to + * affect surrounding constructs. + */ + Node *save_case_val = context->case_val; + + /* If we found a CaseTestExpr, we can't be re-simplifying */ + Assert(jce->coparam == 0); + if (!context->estimate) + { + Param *p; + + p = generate_new_expr_param(context, + ctexpr->typeId, + ctexpr->typeMod, + ctexpr->collation); + context->case_val = (Node *) p; + jce->coparam = p->paramid; + } + else + context->case_val = NULL; + + jce->coercion = (Expr *) + eval_const_expressions_mutator((Node *) jce->coercion, + context); + + context->case_val = save_case_val; + } + else + jce->coercion = (Expr *) + eval_const_expressions_mutator((Node *) jce->coercion, + context); + + jce->returning = (JsonReturning *) + eval_const_expressions_mutator((Node *) jce->returning, + context); + return (Node *) jce; + } + case T_JsonExpr: + { + JsonExpr *je = makeNode(JsonExpr); + CaseTestExpr *ctexpr = NULL; + + /* + * 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 formatted_expr. + */ + memcpy(je, node, sizeof(JsonExpr)); + + /* + * The formatted_expr might or might not contain a + * CaseTestExpr; if it doesn't, we don't want to uselessly + * consume a PARAM_EXPR slot. Besides that, copying its + * type/typmod/collation is much easier than trying to + * reverse-engineer what it would have been. + */ + if (find_casetestexpr(je->formatted_expr, &ctexpr)) + { + /* + * Set up the node to substitute for the CaseTestExpr node + * contained in formatted_expr. Normally we replace it + * with a PARAM_EXPR Param, but if we're just estimating, + * leave it alone. + * + * We must save and restore context->case_val so as not to + * affect surrounding constructs. + */ + Node *save_case_val = context->case_val; + + /* If we found a CaseTestExpr, we can't be re-simplifying */ + Assert(je->feparam == 0); + if (!context->estimate) + { + Param *p; + + p = generate_new_expr_param(context, + ctexpr->typeId, + ctexpr->typeMod, + ctexpr->collation); + context->case_val = (Node *) p; + je->feparam = p->paramid; + } + else + context->case_val = NULL; + + je->formatted_expr = + eval_const_expressions_mutator(je->formatted_expr, + context); + + context->case_val = save_case_val; + } + else + je->formatted_expr = + eval_const_expressions_mutator(je->formatted_expr, + context); + + je->path_spec = + eval_const_expressions_mutator(je->path_spec, + context); + je->passing_values = (List *) + eval_const_expressions_mutator((Node *) je->passing_values, + context); + je->on_empty = (JsonBehavior *) + eval_const_expressions_mutator((Node *) je->on_empty, + context); + je->on_error = (JsonBehavior *) + eval_const_expressions_mutator((Node *) je->on_error, + context); + return (Node *) je; + } default: break; } diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index a17b8a5797..ddf0d49495 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -3676,7 +3676,12 @@ makeJsonConstructorExpr(ParseState *pstate, JsonConstructorType type, * 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. + * that is of type JSON or JSONB to the coercion function. This is safe + * because the finished coercion expression cannot directly contain a + * CASE, nor any other construct that similarly abuses CaseTestExpr. + * (Subsequent inlining of a SQL-language cast function could create a + * hazard, but we leave it to the planner to deal with that scenario by + * replacing the CaseTestExpr with a Param beforehand.) */ if (fexpr) { diff --git a/src/backend/parser/parse_jsontable.c b/src/backend/parser/parse_jsontable.c index 13d533b83f..96a3b26d96 100644 --- a/src/backend/parser/parse_jsontable.c +++ b/src/backend/parser/parse_jsontable.c @@ -319,6 +319,17 @@ transformJsonTableColumns(JsonTableParseContext *cxt, List *columns, JsonFuncExpr *jfe; CaseTestExpr *param = makeNode(CaseTestExpr); + /* + * We abuse CaseTestExpr here as a placeholder to pass the + * row pattern value to any coercion function that may be + * needed. This is safe because the finished column + * expression cannot directly contain a CASE, nor any + * other construct that similarly abuses CaseTestExpr. + * (Subsequent inlining of a SQL-language cast function + * could create a hazard, but we leave it to the planner + * to deal with that scenario by replacing the + * CaseTestExpr with a Param beforehand.) + */ param->collation = InvalidOid; param->typeId = contextItemTypid; param->typeMod = -1; diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c index dbab24737e..2fef0adf89 100644 --- a/src/backend/utils/adt/jsonpath_exec.c +++ b/src/backend/utils/adt/jsonpath_exec.c @@ -4470,17 +4470,11 @@ JsonTableGetValue(TableFuncScanState *state, int colnum, /* Evaluate JsonExpr. */ else if (estate) { - Datum saved_caseValue = econtext->caseValue_datum; - bool saved_caseIsNull = econtext->caseValue_isNull; - - /* Pass the row pattern value via CaseTestExpr. */ - econtext->caseValue_datum = current->value; - econtext->caseValue_isNull = false; + /* Pass the row pattern value where the expression expects it. */ + estate->resvalue = current->value; + estate->resnull = false; result = ExecEvalExpr(estate, econtext, isnull); - - econtext->caseValue_datum = saved_caseValue; - econtext->caseValue_isNull = saved_caseIsNull; } /* ORDINAL column */ else diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 2c87e6365d..aad1fc2897 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -1753,6 +1753,11 @@ typedef enum JsonConstructorType /* * JsonConstructorExpr - * wrapper over FuncExpr/Aggref/WindowFunc for SQL/JSON constructors + * + * If a coercion to the RETURNING type is required, the coercion field holds a + * suitable coercion expression tree (else it's NULL). The base node of that + * tree is initially a CaseTestExpr, but the planner replaces that with a + * PARAM_EXPR Param, and sets coparam to that Param's paramid. */ typedef struct JsonConstructorExpr { @@ -1761,6 +1766,8 @@ typedef struct JsonConstructorExpr List *args; Expr *func; /* underlying json[b]_xxx() function call */ Expr *coercion; /* coercion to RETURNING type */ + /* ID of PARAM_EXPR Param representing input, if assigned; else 0 */ + int coparam pg_node_attr(equal_ignore, query_jumble_ignore); JsonReturning *returning; /* RETURNING clause */ bool absent_on_null; /* ABSENT ON NULL? */ bool unique; /* WITH UNIQUE KEYS? (JSON_OBJECT[AGG] only) */ @@ -1876,6 +1883,9 @@ typedef struct JsonExpr /* jsonb-valued expression to query */ Node *formatted_expr; + /* ID of PARAM_EXPR Param within formatted_expr, if any; else 0 */ + int feparam pg_node_attr(equal_ignore, query_jumble_ignore); + /* Format of the above expression needed by ruleutils.c */ JsonFormat *format; -- 2.43.5