From 7000367d1f218c2833d784f3227e0f7a1a2526fd Mon Sep 17 00:00:00 2001
From: Srinath Reddy Sadipiralla <srinath2133@gmail.com>
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 " <behavior> ON <label>" (e.g. " IGNORE ON MISSING").  The resolved
+ * behavior is JSON_TRANSFORM_BEHAVIOR_UNSPECIFIED for clauses that do not
+ * apply to the operation, in which case nothing is emitted -- so the caller
+ * can invoke this unconditionally for EXISTING/MISSING/NULL for any operation.
+ */
+static void
+get_json_transform_behavior(StringInfo buf, JsonTransformBehavior behavior,
+							const char *label)
+{
+	const char *kw;
+
+	switch (behavior)
+	{
+		case JSON_TRANSFORM_BEHAVIOR_UNSPECIFIED:
+			return;
+		case JSON_TRANSFORM_BEHAVIOR_ERROR:
+			kw = "ERROR";
+			break;
+		case JSON_TRANSFORM_BEHAVIOR_IGNORE:
+			kw = "IGNORE";
+			break;
+		case JSON_TRANSFORM_BEHAVIOR_NULL:
+			kw = "NULL";
+			break;
+		case JSON_TRANSFORM_BEHAVIOR_REMOVE:
+			kw = "REMOVE";
+			break;
+		default:
+			elog(ERROR, "unrecognized JSON_TRANSFORM behavior: %d",
+				 (int) behavior);
+			kw = NULL;			/* keep compiler quiet */
+	}
+
+	appendStringInfo(buf, " %s ON %s", kw, label);
+}
+
 /*
  * get_json_format			- Parse back a JsonFormat node
  */
-- 
2.43.0

