From 7000367d1f218c2833d784f3227e0f7a1a2526fd Mon Sep 17 00:00:00 2001 From: Srinath Reddy Sadipiralla Date: Thu, 9 Jul 2026 10:36:18 +0530 Subject: [PATCH v3 4/4] JSON_TRANSFORM: fix crash, add deparse, support lax/strict target paths Address review feedback on the JSON_TRANSFORM patch: Fix a backend crash when a JSON_TRANSFORM node reaches contain_mutable_functions_walker(), e.g. CREATE INDEX ON t ((JSON_TRANSFORM(j, REMOVE '$.a'))); The walker dereferenced jexpr->path_spec, but JSON_TRANSFORM leaves that NULL and carries its jsonpath in jexpr->action->pathspec instead. Use the latter when an action is present. Add ruleutils deparse support for the JSON_TRANSFORM op, which previously errored with "unrecognized JsonExpr op". Emit the operation keyword, target path, replacement value and the per-operation ON EXISTING / ON MISSING / ON NULL behavior clauses, so EXPLAIN VERBOSE now works for JSON_TRANSFORM. Support the jsonpath lax/strict mode in the target-path walker. In lax mode (the SQL/JSON default) a .key or .* accessor applied to an array unwraps the array and re-applies the accessor to each element; in strict mode that is a structural error. The walker previously ignored the mode and silently no-opped on arrays. The behavior now matches jsonb_path_query() for the same paths. --- src/backend/executor/execExprInterp.c | 71 ++++++++++++++++++---- src/backend/optimizer/util/clauses.c | 10 ++- src/backend/utils/adt/ruleutils.c | 87 +++++++++++++++++++++++++-- 3 files changed, 150 insertions(+), 18 deletions(-) diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 87aec80080c..0417eff48ac 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -194,12 +194,13 @@ typedef struct JsonTransformStep static void jtSetPath(JsonbIterator **it, JsonbInState *st, JsonTransformStep *steps, int nsteps, int level, JsonTransformOp op, JsonbValue *newval, - JsonTransformBehavior on_existing, bool *matched); + JsonTransformBehavior on_existing, bool strict, + bool unwrap, bool *matched); static void jtSetPathObject(JsonbIterator **it, JsonbInState *st, JsonTransformStep *steps, int nsteps, int level, JsonTransformOp op, JsonbValue *newval, - JsonTransformBehavior on_existing, bool *matched, - uint32 npairs); + JsonTransformBehavior on_existing, bool strict, + bool *matched, uint32 npairs); static void jtCopyValue(JsonbIterator **it, JsonbInState *st); /* execution helper functions */ @@ -5261,7 +5262,8 @@ static void jtSetPathObject(JsonbIterator **it, JsonbInState *st, JsonTransformStep *steps, int nsteps, int level, JsonTransformOp op, JsonbValue *newval, - JsonTransformBehavior on_existing, bool *matched, uint32 npairs) + JsonTransformBehavior on_existing, bool strict, + bool *matched, uint32 npairs) { JsonTransformStep *step = &steps[level]; bool is_last = (level == nsteps - 1); @@ -5348,7 +5350,7 @@ jtSetPathObject(JsonbIterator **it, JsonbInState *st, /* descend into this member's value to continue matching */ pushJsonbValue(st, WJB_KEY, &k); jtSetPath(it, st, steps, nsteps, level + 1, op, newval, - on_existing, matched); + on_existing, strict, !strict, matched); } else { @@ -5380,15 +5382,20 @@ jtSetPathObject(JsonbIterator **it, JsonbInState *st, * Dispatcher: read the container/value currently at the front of *it and * rebuild it into *st, recursing through objects along the path. * - * If the path still has steps to match but the current value is an array or - * scalar (our paths can only address object members), we can't descend, so we - * copy it verbatim -- effectively IGNORE ON MISSING. + * 'strict' is the path's global mode; 'unwrap' says whether an array at this + * position should be lax-unwrapped -- true at a primary position in lax mode, + * false when recursing into an already-unwrapped array (keeping unwrapping one + * level deep). A '.key'/'.*' accessor needs an object: applied to an array we + * unwrap it (lax) or raise a structural error (strict); applied to a scalar we + * skip it (lax) or raise a structural error (strict). This matches the + * SQL/JSON path engine used by jsonb_path_query(). */ static void jtSetPath(JsonbIterator **it, JsonbInState *st, JsonTransformStep *steps, int nsteps, int level, JsonTransformOp op, JsonbValue *newval, - JsonTransformBehavior on_existing, bool *matched) + JsonTransformBehavior on_existing, bool strict, bool unwrap, + bool *matched) { JsonbValue v; JsonbIteratorToken r; @@ -5401,13 +5408,51 @@ jtSetPath(JsonbIterator **it, JsonbInState *st, { pushJsonbValue(st, WJB_BEGIN_OBJECT, NULL); jtSetPathObject(it, st, steps, nsteps, level, op, newval, - on_existing, matched, v.val.object.nPairs); + on_existing, strict, matched, v.val.object.nPairs); r = JsonbIteratorNext(it, &v, true); Assert(r == WJB_END_OBJECT); pushJsonbValue(st, WJB_END_OBJECT, NULL); } + else if (r == WJB_BEGIN_ARRAY && unwrap) + { + /* + * Lax mode: a '.key'/'.*' accessor applied to an array auto-unwraps it. + * Rebuild the array, re-applying the *current* step to each element. + * Per the path language this unwrapping is one level deep, so we recurse + * with unwrap=false: a nested array element is then handled as a plain + * value (copied through), not unwrapped again. This mirrors + * jsonb_path_query()'s lax behavior. + */ + uint32 nelems = v.val.array.nElems; + uint32 i; + + pushJsonbValue(st, WJB_BEGIN_ARRAY, NULL); + for (i = 0; i < nelems; i++) + jtSetPath(it, st, steps, nsteps, level, op, newval, + on_existing, strict, false, matched); + r = JsonbIteratorNext(it, &v, true); + Assert(r == WJB_END_ARRAY); + pushJsonbValue(st, WJB_END_ARRAY, NULL); + } + else if (strict) + { + /* + * A '.key'/'.*' accessor requires an object, but here the value is an + * array we are not unwrapping, or a scalar. In strict mode this is a + * structural error, matching the SQL/JSON path engine. + */ + if (steps[level].wildcard) + ereport(ERROR, + errcode(ERRCODE_SQL_JSON_OBJECT_NOT_FOUND), + errmsg("jsonpath wildcard member accessor can only be applied to an object")); + else + ereport(ERROR, + errcode(ERRCODE_SQL_JSON_MEMBER_NOT_FOUND), + errmsg("jsonpath member accessor can only be applied to an object")); + } else if (r == WJB_BEGIN_ARRAY) { + /* lax, but not unwrapping (nested array): nothing matches, copy it */ int walking_level = 1; pushJsonbValue(st, WJB_BEGIN_ARRAY, NULL); @@ -5423,7 +5468,7 @@ jtSetPath(JsonbIterator **it, JsonbInState *st, } else { - /* scalar at a point the path wants to descend into: leave as-is */ + /* lax scalar: nothing matches, leave as-is */ pushJsonbValue(st, r, &v); } } @@ -5466,6 +5511,7 @@ ExecEvalJsonTransform(ExprState *state, ExprEvalStep *op, JsonbInState st = {0}; JsonTransformOp effop = action->op; bool matched = false; + bool strict; /* * The JUMP_IF_NULL guards in the step array already skip us if @@ -5476,6 +5522,7 @@ ExecEvalJsonTransform(ExprState *state, ExprEvalStep *op, in = DatumGetJsonbP(jtstate->formatted_expr.value); jp = DatumGetJsonPathP(jtstate->pathspec.value); + strict = (jp->header & JSONPATH_LAX) == 0; /* Validate path and break it into accessor steps. */ steps = JsonPathToTransformSteps(jp, action->op, &nsteps); @@ -5543,7 +5590,7 @@ ExecEvalJsonTransform(ExprState *state, ExprEvalStep *op, it = JsonbIteratorInit(&in->root); jtSetPath(&it, &st, steps, nsteps, 0, effop, newval, - action->on_existing, &matched); + action->on_existing, strict, !strict, &matched); /* ON MISSING ERROR: the path matched no existing target. */ if (!matched && action->on_missing == JSON_TRANSFORM_BEHAVIOR_ERROR) diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index aa8886ec210..2dcc7f69e1b 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -448,11 +448,17 @@ contain_mutable_functions_walker(Node *node, void *context) { JsonExpr *jexpr = castNode(JsonExpr, node); Const *cnst; + Node *path_spec; - if (!IsA(jexpr->path_spec, Const)) + if(jexpr->action) + path_spec = jexpr->action->pathspec; + else + path_spec = jexpr->path_spec; + + if (!IsA(path_spec, Const)) return true; - cnst = castNode(Const, jexpr->path_spec); + cnst = castNode(Const, path_spec); Assert(cnst->consttype == JSONPATHOID); if (cnst->constisnull) diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 1b44b7a78d2..0c91bc75a04 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -550,6 +550,7 @@ static char *generate_qualified_type_name(Oid typid); static text *string_to_text(char *str); static char *flatten_reloptions(Oid relid); void get_reloptions(StringInfo buf, Datum reloptions); +static void get_json_transform_behavior(StringInfo buf, JsonTransformBehavior behavior, const char *label); static void get_json_path_spec(Node *path_spec, deparse_context *context, bool showimplicit); static void get_json_table_columns(TableFunc *tf, JsonTablePathScan *scan, @@ -11129,6 +11130,9 @@ get_rule_expr(Node *node, deparse_context *context, case JSON_VALUE_OP: appendStringInfoString(buf, "JSON_VALUE("); break; + case JSON_TRANSFORM_OP: + appendStringInfoString(buf, "JSON_TRANSFORM("); + break; default: elog(ERROR, "unrecognized JsonExpr op: %d", (int) jexpr->op); @@ -11138,7 +11142,40 @@ get_rule_expr(Node *node, deparse_context *context, appendStringInfoString(buf, ", "); - get_json_path_spec(jexpr->path_spec, context, showimplicit); + if(jexpr->action) + { + switch (jexpr->action->op) + { + case TRANSFORM_INSERT: + appendStringInfoString(buf, "INSERT "); + break; + case TRANSFORM_REMOVE: + appendStringInfoString(buf, "REMOVE "); + break; + case TRANSFORM_RENAME: + appendStringInfoString(buf, "RENAME "); + break; + case TRANSFORM_REPLACE: + appendStringInfoString(buf, "REPLACE "); + break; + default: + elog(ERROR, "unrecognized JsonTransform op: %d", + (int) jexpr->action->op); + } + + get_json_path_spec(jexpr->action->pathspec, context, showimplicit); + if(jexpr->action->op != TRANSFORM_REMOVE) + { + appendStringInfoString(buf, " = "); + get_rule_expr(jexpr->action->value_expr, context, showimplicit); + } + + get_json_transform_behavior(buf, jexpr->action->on_existing, "EXISTING"); + get_json_transform_behavior(buf, jexpr->action->on_missing, "MISSING"); + get_json_transform_behavior(buf, jexpr->action->on_null, "NULL"); + } + else + get_json_path_spec(jexpr->path_spec, context, showimplicit); if (jexpr->passing_values) { @@ -11161,12 +11198,15 @@ get_rule_expr(Node *node, deparse_context *context, } } - if (jexpr->op != JSON_EXISTS_OP || - jexpr->returning->typid != BOOLOID) + /* JSON_TRANSFORM has no RETURNING or ON EMPTY/ERROR clauses */ + if (jexpr->op != JSON_TRANSFORM_OP && + (jexpr->op != JSON_EXISTS_OP || + jexpr->returning->typid != BOOLOID)) get_json_returning(jexpr->returning, context->buf, jexpr->op == JSON_QUERY_OP); - get_json_expr_options(jexpr, context, + if (jexpr->op != JSON_TRANSFORM_OP) + get_json_expr_options(jexpr, context, jexpr->op != JSON_EXISTS_OP ? JSON_BEHAVIOR_NULL : JSON_BEHAVIOR_FALSE); @@ -12213,6 +12253,45 @@ get_json_path_spec(Node *path_spec, deparse_context *context, bool showimplicit) get_rule_expr(path_spec, context, showimplicit); } +/* + * get_json_transform_behavior - deparse one JSON_TRANSFORM ON clause + * + * Emits " ON