From 7d69cb42d276afc78db78375bbe5cf871b80aad3 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Wed, 16 Oct 2024 18:04:54 +0900 Subject: [PATCH v2 1/2] SQL/JSON: Fix some oversights in commit b6e1157e7 JsonValueExpr.raw_expr should not be touched by expression_tree_walker() and expression_tree_mutator(). One problem it causes, as demonstrated in BUG #18657, is that the Aggref nodes referenced in formatted_expr, which is evaluated, end up with the wrong value of aggtransno. This occurs because the Aggref's copy in raw_expr is also added to PlannerInfo.aggtransinfos by preprocess_aggref(), leading to incorrect state in the planner. This can result in a crash during ExecBuildAggTrans(), as seen in the bug report. Bug: #18657 Reported-by: Alexander Lakhin Diagnosed-by: Tender Wang Discussion: https://postgr.es/m/18657-1b90ccce2b16bdb8@postgresql.org Backpatch-through: 16 --- src/backend/nodes/nodeFuncs.c | 3 -- src/test/regress/expected/sqljson.out | 45 ++++++++++++++++++++------- src/test/regress/sql/sqljson.sql | 10 ++++++ 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c index 0d00e029f3..579c85f0d0 100644 --- a/src/backend/nodes/nodeFuncs.c +++ b/src/backend/nodes/nodeFuncs.c @@ -2334,8 +2334,6 @@ expression_tree_walker_impl(Node *node, { JsonValueExpr *jve = (JsonValueExpr *) node; - if (WALK(jve->raw_expr)) - return true; if (WALK(jve->formatted_expr)) return true; } @@ -3370,7 +3368,6 @@ expression_tree_mutator_impl(Node *node, JsonValueExpr *newnode; FLATCOPY(newnode, jve, JsonValueExpr); - MUTATE(newnode->raw_expr, jve->raw_expr, Expr *); MUTATE(newnode->formatted_expr, jve->formatted_expr, Expr *); MUTATE(newnode->format, jve->format, JsonFormat *); diff --git a/src/test/regress/expected/sqljson.out b/src/test/regress/expected/sqljson.out index 9adfbe15f6..dd30a5323f 100644 --- a/src/test/regress/expected/sqljson.out +++ b/src/test/regress/expected/sqljson.out @@ -995,10 +995,10 @@ DROP VIEW json_array_view; EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_OBJECTAGG(i: ('111' || i)::bytea FORMAT JSON WITH UNIQUE RETURNING text) FILTER (WHERE i > 3) FROM generate_series(1,5) i; - QUERY PLAN --------------------------------------------------------------------------------------------------------------------------------------- + QUERY PLAN +------------------------------------------------------------------------------------------------------------------------------ Aggregate - Output: JSON_OBJECTAGG(i : (('111'::text || (i)::text))::bytea FORMAT JSON WITH UNIQUE KEYS RETURNING text) FILTER (WHERE (i > 3)) + Output: JSON_OBJECTAGG(i : (('111'::text || i))::bytea FORMAT JSON WITH UNIQUE KEYS RETURNING text) FILTER (WHERE (i > 3)) -> Function Scan on pg_catalog.generate_series i Output: i Function Call: generate_series(1, 5) @@ -1007,10 +1007,10 @@ FROM generate_series(1,5) i; EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_OBJECTAGG(i: ('111' || i)::bytea FORMAT JSON WITH UNIQUE RETURNING text) OVER (PARTITION BY i % 2) FROM generate_series(1,5) i; - QUERY PLAN ------------------------------------------------------------------------------------------------------------------------------------ + QUERY PLAN +--------------------------------------------------------------------------------------------------------------------------- WindowAgg - Output: JSON_OBJECTAGG(i : (('111'::text || (i)::text))::bytea FORMAT JSON WITH UNIQUE KEYS RETURNING text) OVER (?), ((i % 2)) + Output: JSON_OBJECTAGG(i : (('111'::text || i))::bytea FORMAT JSON WITH UNIQUE KEYS RETURNING text) OVER (?), ((i % 2)) -> Sort Output: ((i % 2)), i Sort Key: ((i.i % 2)) @@ -1031,10 +1031,10 @@ DROP VIEW json_objectagg_view; EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_ARRAYAGG(('111' || i)::bytea FORMAT JSON NULL ON NULL RETURNING text) FILTER (WHERE i > 3) FROM generate_series(1,5) i; - QUERY PLAN ------------------------------------------------------------------------------------------------------------------------------ + QUERY PLAN +--------------------------------------------------------------------------------------------------------------------- Aggregate - Output: JSON_ARRAYAGG((('111'::text || (i)::text))::bytea FORMAT JSON NULL ON NULL RETURNING text) FILTER (WHERE (i > 3)) + Output: JSON_ARRAYAGG((('111'::text || i))::bytea FORMAT JSON NULL ON NULL RETURNING text) FILTER (WHERE (i > 3)) -> Function Scan on pg_catalog.generate_series i Output: i Function Call: generate_series(1, 5) @@ -1043,10 +1043,10 @@ FROM generate_series(1,5) i; EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_ARRAYAGG(('111' || i)::bytea FORMAT JSON NULL ON NULL RETURNING text) OVER (PARTITION BY i % 2) FROM generate_series(1,5) i; - QUERY PLAN --------------------------------------------------------------------------------------------------------------------------- + QUERY PLAN +------------------------------------------------------------------------------------------------------------------ WindowAgg - Output: JSON_ARRAYAGG((('111'::text || (i)::text))::bytea FORMAT JSON NULL ON NULL RETURNING text) OVER (?), ((i % 2)) + Output: JSON_ARRAYAGG((('111'::text || i))::bytea FORMAT JSON NULL ON NULL RETURNING text) OVER (?), ((i % 2)) -> Sort Output: ((i % 2)), i Sort Key: ((i.i % 2)) @@ -1300,3 +1300,24 @@ SELECT JSON_SERIALIZE('123' RETURNING sqljson_char2); ERROR: value too long for type character(2) SELECT JSON_SERIALIZE('12' RETURNING sqljson_char2); ERROR: value for domain sqljson_char2 violates check constraint "sqljson_char2_check" +-- Bug #18657: JsonValueExpr.raw_expr was accidentally processed by the +-- planner (preprocess_aggref()) causing the plan to carry Aggrefs containing +-- incorrect / out-of-sync value of aggtransno. One symptom of that is the +-- crash in ExecBuildAggTrans() as mentioned in the bug report: +-- https://postgr.es/m/18657-1b90ccce2b16bdb8@postgresql.org +CREATE FUNCTION volatile_one() RETURNS int AS $$ BEGIN RETURN 1; END; $$ LANGUAGE plpgsql VOLATILE; +EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': volatile_one() RETURNING text) FORMAT JSON); + QUERY PLAN +------------------------------------------------------------------------------------------------------------- + Aggregate + Output: JSON_OBJECT('a' : JSON_OBJECTAGG('b' : volatile_one() RETURNING text) FORMAT JSON RETURNING json) + -> Result +(3 rows) + +SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': volatile_one() RETURNING text) FORMAT JSON); + json_object +--------------------- + {"a" : { "b" : 1 }} +(1 row) + +DROP FUNCTION volatile_one; diff --git a/src/test/regress/sql/sqljson.sql b/src/test/regress/sql/sqljson.sql index 42dbec26d6..2100da02d3 100644 --- a/src/test/regress/sql/sqljson.sql +++ b/src/test/regress/sql/sqljson.sql @@ -480,3 +480,13 @@ SELECT JSON_OBJECTAGG(i: ('111' || i)::bytea FORMAT JSON WITH UNIQUE RETURNING v CREATE DOMAIN sqljson_char2 AS char(2) CHECK (VALUE NOT IN ('12')); SELECT JSON_SERIALIZE('123' RETURNING sqljson_char2); SELECT JSON_SERIALIZE('12' RETURNING sqljson_char2); + +-- Bug #18657: JsonValueExpr.raw_expr was accidentally processed by the +-- planner (preprocess_aggref()) causing the plan to carry Aggrefs containing +-- incorrect / out-of-sync value of aggtransno. One symptom of that is the +-- crash in ExecBuildAggTrans() as mentioned in the bug report: +-- https://postgr.es/m/18657-1b90ccce2b16bdb8@postgresql.org +CREATE FUNCTION volatile_one() RETURNS int AS $$ BEGIN RETURN 1; END; $$ LANGUAGE plpgsql VOLATILE; +EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': volatile_one() RETURNING text) FORMAT JSON); +SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': volatile_one() RETURNING text) FORMAT JSON); +DROP FUNCTION volatile_one; -- 2.43.0