From d7240e3bbdd1179135e097ff2df641eba27f0240 Mon Sep 17 00:00:00 2001 From: Ilya Staroverov Date: Fri, 16 Oct 2026 11:08:16 +0300 Subject: [PATCH v1] Don't evaluate JsonValueExpr.raw_expr at runtime Since 11c87216d, ExecInitExprRec() recurses into both raw_expr and formatted_expr of a JsonValueExpr. Initializing raw_expr is needed, for example so that any Aggrefs in it are linked into the parent Agg node, but this also made raw_expr be evaluated at runtime, although its value is never used: formatted_expr contains the same user-specified expression, possibly coerced, and its value overwrites that of raw_expr. So the user-specified expression was evaluated twice. That is visible if it is volatile, for example when a volatile function is used as the context item of JSON_QUERY() or as a FORMAT JSON argument of JSON_OBJECT(), and it makes expensive expressions twice as costly. Fix by still emitting the steps for raw_expr, so that it is initialized as before, but jumping over them at runtime. Also update the comment of JsonValueExpr accordingly. This doesn't change the case where raw_expr contains an Aggref with volatile arguments, as in the query from bug #18657: the planner computes the Aggrefs in raw_expr and in formatted_expr as two separate aggregates. Avoiding that would require not duplicating the expression in the parse tree, which can't be back-patched. Oversight in 11c87216d. Author: Ilya Staroverov Backpatch-through: 16 --- src/backend/executor/execExpr.c | 18 +++++++++++++ src/include/nodes/primnodes.h | 5 ++-- src/test/regress/expected/sqljson.out | 23 ++++++++++++++++ .../regress/expected/sqljson_queryfuncs.out | 26 +++++++++++++++++++ src/test/regress/sql/sqljson.sql | 9 +++++++ src/test/regress/sql/sqljson_queryfuncs.sql | 12 +++++++++ 6 files changed, 91 insertions(+), 2 deletions(-) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 3f063b79f..559ae8f42 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -2361,9 +2361,27 @@ ExecInitExprRec(Expr *node, ExprState *state, case T_JsonValueExpr: { JsonValueExpr *jve = (JsonValueExpr *) node; + int jumpstep; + + /* + * Only the value of formatted_expr is used, and it contains + * the same expression as raw_expr, possibly coerced, so also + * evaluating raw_expr would evaluate the user-specified + * expression twice, which matters if it is volatile. We + * still have to initialize raw_expr, for example so that the + * parent Agg node knows about any Aggrefs in it, so emit its + * steps but jump over them. + */ + scratch.opcode = EEOP_JUMP; + scratch.d.jump.jumpdone = -1; /* computed later */ + ExprEvalPushStep(state, &scratch); + jumpstep = state->steps_len - 1; Assert(jve->raw_expr != NULL); ExecInitExprRec(jve->raw_expr, state, resv, resnull); + + state->steps[jumpstep].d.jump.jumpdone = state->steps_len; + Assert(jve->formatted_expr != NULL); ExecInitExprRec(jve->formatted_expr, state, resv, resnull); break; diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 726c186c7..40ce09d45 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -1686,8 +1686,9 @@ typedef struct JsonReturning * clause or an enclosing node's RETURNING clause. * * When deparsing a JsonValueExpr, get_rule_expr() prints raw_expr. However, - * during the evaluation of a JsonValueExpr, the value of formatted_expr - * takes precedence over that of raw_expr. + * only formatted_expr, which contains the same expression, is evaluated; + * raw_expr is only initialized by ExecInitExprRec(), so that e.g. any + * Aggrefs in it are set up. */ typedef struct JsonValueExpr { diff --git a/src/test/regress/expected/sqljson.out b/src/test/regress/expected/sqljson.out index 30f9e10ef..b9f4b8846 100644 --- a/src/test/regress/expected/sqljson.out +++ b/src/test/regress/expected/sqljson.out @@ -1789,6 +1789,29 @@ SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': 1 RETURNING text) FORMAT JSON); (1 row) DROP FUNCTION volatile_one, stable_one; +-- The expression in a JsonValueExpr must be evaluated only once +CREATE SEQUENCE sqljson_seq; +SELECT JSON_OBJECT('a': nextval('sqljson_seq')::text FORMAT JSON), + JSON_ARRAY(nextval('sqljson_seq')::text FORMAT JSON); + json_object | json_array +-------------+------------ + {"a" : 1} | [2] +(1 row) + +SELECT JSON_ARRAYAGG(nextval('sqljson_seq')::text FORMAT JSON) + FROM generate_series(1, 2); + json_arrayagg +--------------- + [3, 4] +(1 row) + +SELECT currval('sqljson_seq'); + currval +--------- + 4 +(1 row) + +DROP SEQUENCE sqljson_seq; -- Test deparsing of JSON aggregates that are computed below a WindowAgg -- node. EXPLAIN (VERBOSE, COSTS OFF) diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out index fe8aa927b..1b043ed79 100644 --- a/src/test/regress/expected/sqljson_queryfuncs.out +++ b/src/test/regress/expected/sqljson_queryfuncs.out @@ -1473,6 +1473,32 @@ SELECT JSON_QUERY(jsonb 'null', '$"Xyz"' PASSING 1 AS "Xyz"); 1 (1 row) +-- The context item and the PASSING arguments must be evaluated only once +CREATE DOMAIN queryfuncs_int AS int; +CREATE SEQUENCE queryfuncs_seq; +SELECT JSON_QUERY(nextval('queryfuncs_seq')::text, '$'), + JSON_VALUE(jsonb 'null', '$x' + PASSING nextval('queryfuncs_seq')::queryfuncs_int AS x); + json_query | json_value +------------+------------ + 1 | 2 +(1 row) + +SELECT * FROM JSON_TABLE(nextval('queryfuncs_seq')::text, '$' + COLUMNS (v int PATH '$')); + v +--- + 3 +(1 row) + +SELECT currval('queryfuncs_seq'); + currval +--------- + 3 +(1 row) + +DROP SEQUENCE queryfuncs_seq; +DROP DOMAIN queryfuncs_int; -- Test PASSING and toasted values CREATE TABLE test_passing_toast (t text); ALTER TABLE test_passing_toast ALTER COLUMN t SET STORAGE EXTENDED; diff --git a/src/test/regress/sql/sqljson.sql b/src/test/regress/sql/sqljson.sql index 9fa203ccb..49c92619d 100644 --- a/src/test/regress/sql/sqljson.sql +++ b/src/test/regress/sql/sqljson.sql @@ -726,6 +726,15 @@ EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': 1 RETUR SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': 1 RETURNING text) FORMAT JSON); DROP FUNCTION volatile_one, stable_one; +-- The expression in a JsonValueExpr must be evaluated only once +CREATE SEQUENCE sqljson_seq; +SELECT JSON_OBJECT('a': nextval('sqljson_seq')::text FORMAT JSON), + JSON_ARRAY(nextval('sqljson_seq')::text FORMAT JSON); +SELECT JSON_ARRAYAGG(nextval('sqljson_seq')::text FORMAT JSON) + FROM generate_series(1, 2); +SELECT currval('sqljson_seq'); +DROP SEQUENCE sqljson_seq; + -- Test deparsing of JSON aggregates that are computed below a WindowAgg -- node. EXPLAIN (VERBOSE, COSTS OFF) diff --git a/src/test/regress/sql/sqljson_queryfuncs.sql b/src/test/regress/sql/sqljson_queryfuncs.sql index 0767db9cf..dd6e29369 100644 --- a/src/test/regress/sql/sqljson_queryfuncs.sql +++ b/src/test/regress/sql/sqljson_queryfuncs.sql @@ -488,6 +488,18 @@ SELECT JSON_QUERY(jsonb 'null', '$Xyz' PASSING 1 AS Xyz); SELECT JSON_QUERY(jsonb 'null', '$Xyz' PASSING 1 AS "Xyz"); SELECT JSON_QUERY(jsonb 'null', '$"Xyz"' PASSING 1 AS "Xyz"); +-- The context item and the PASSING arguments must be evaluated only once +CREATE DOMAIN queryfuncs_int AS int; +CREATE SEQUENCE queryfuncs_seq; +SELECT JSON_QUERY(nextval('queryfuncs_seq')::text, '$'), + JSON_VALUE(jsonb 'null', '$x' + PASSING nextval('queryfuncs_seq')::queryfuncs_int AS x); +SELECT * FROM JSON_TABLE(nextval('queryfuncs_seq')::text, '$' + COLUMNS (v int PATH '$')); +SELECT currval('queryfuncs_seq'); +DROP SEQUENCE queryfuncs_seq; +DROP DOMAIN queryfuncs_int; + -- Test PASSING and toasted values CREATE TABLE test_passing_toast (t text); ALTER TABLE test_passing_toast ALTER COLUMN t SET STORAGE EXTENDED; -- 2.43.0