Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit11c8721

Browse files
committed
SQL/JSON: Fix some oversights in commitb6e1157
The decision inb6e1157 to ignore raw_expr when evaluating aJsonValueExpr was incorrect. While its value is not ultimatelyused (since formatted_expr's value is), failing to initialize itcan lead to problems, for instance, when the expression tree inraw_expr contains Aggref nodes, which must be initialized toensure the parent Agg node works correctly.Also, optimize eval_const_expressions_mutator()'s handling ofJsonValueExpr a bit. Currently, when formatted_expr cannot be foldedinto a constant, we end up processing it twice -- once directly ineval_const_expressions_mutator() and again recursively viaece_generic_processing(). This recursive processing is required tohandle raw_expr. To avoid the redundant processing of formatted_expr,we now process raw_expr directly in eval_const_expressions_mutator().Finally, update the comment of JsonValueExpr to describe the roles ofraw_expr and formatted_expr more clearly.Bug: #18657Reported-by: Alexander Lakhin <exclusion@gmail.com>Diagnosed-by: Fabio R. Sluzala <fabio3rs@gmail.com>Diagnosed-by: Tender Wang <tndrwang@gmail.com>Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>Discussion:https://postgr.es/m/18657-1b90ccce2b16bdb8@postgresql.orgBackpatch-through: 16
1 parent52475b4 commit11c8721

File tree

5 files changed

+92
-11
lines changed

5 files changed

+92
-11
lines changed

‎src/backend/executor/execExpr.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2307,6 +2307,8 @@ ExecInitExprRec(Expr *node, ExprState *state,
23072307
{
23082308
JsonValueExpr*jve= (JsonValueExpr*)node;
23092309

2310+
Assert(jve->raw_expr!=NULL);
2311+
ExecInitExprRec(jve->raw_expr,state,resv,resnull);
23102312
Assert(jve->formatted_expr!=NULL);
23112313
ExecInitExprRec(jve->formatted_expr,state,resv,resnull);
23122314
break;

‎src/backend/optimizer/util/clauses.c

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2915,13 +2915,25 @@ eval_const_expressions_mutator(Node *node,
29152915
caseT_JsonValueExpr:
29162916
{
29172917
JsonValueExpr*jve= (JsonValueExpr*)node;
2918-
Node*formatted;
2918+
Node*raw_expr= (Node*)jve->raw_expr;
2919+
Node*formatted_expr= (Node*)jve->formatted_expr;
29192920

2920-
formatted=eval_const_expressions_mutator((Node*)jve->formatted_expr,
2921-
context);
2922-
if (formatted&&IsA(formatted,Const))
2923-
returnformatted;
2924-
break;
2921+
/*
2922+
* If we can fold formatted_expr to a constant, we can elide
2923+
* the JsonValueExpr altogether. Otherwise we must process
2924+
* raw_expr too. But JsonFormat is a flat node and requires
2925+
* no simplification, only copying.
2926+
*/
2927+
formatted_expr=eval_const_expressions_mutator(formatted_expr,
2928+
context);
2929+
if (formatted_expr&&IsA(formatted_expr,Const))
2930+
returnformatted_expr;
2931+
2932+
raw_expr=eval_const_expressions_mutator(raw_expr,context);
2933+
2934+
return (Node*)makeJsonValueExpr((Expr*)raw_expr,
2935+
(Expr*)formatted_expr,
2936+
copyObject(jve->format));
29252937
}
29262938

29272939
caseT_SubPlan:

‎src/include/nodes/primnodes.h

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1669,15 +1669,19 @@ typedef struct JsonReturning
16691669
* JsonValueExpr -
16701670
*representation of JSON value expression (expr [FORMAT JsonFormat])
16711671
*
1672-
* The actual value is obtained by evaluating formatted_expr. raw_expr is
1673-
* only there for displaying the original user-written expression and is not
1674-
* evaluated by ExecInterpExpr() and eval_const_expressions_mutator().
1672+
* raw_expr is the user-specified value, while formatted_expr is the value
1673+
* obtained by coercing raw_expr to the type required by either the FORMAT
1674+
* clause or an enclosing node's RETURNING clause.
1675+
*
1676+
* When deparsing a JsonValueExpr, get_rule_expr() prints raw_expr. However,
1677+
* during the evaluation of a JsonValueExpr, the value of formatted_expr
1678+
* takes precedence over that of raw_expr.
16751679
*/
16761680
typedefstructJsonValueExpr
16771681
{
16781682
NodeTagtype;
1679-
Expr*raw_expr;/*raw expression */
1680-
Expr*formatted_expr;/* formatted expression */
1683+
Expr*raw_expr;/*user-specified expression */
1684+
Expr*formatted_expr;/*coercedformatted expression */
16811685
JsonFormat*format;/* FORMAT clause, if specified */
16821686
}JsonValueExpr;
16831687

‎src/test/regress/expected/sqljson.out

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,3 +1300,52 @@ SELECT JSON_SERIALIZE('123' RETURNING sqljson_char2);
13001300
ERROR: value too long for type character(2)
13011301
SELECT JSON_SERIALIZE('12' RETURNING sqljson_char2);
13021302
ERROR: value for domain sqljson_char2 violates check constraint "sqljson_char2_check"
1303+
-- Bug #18657: JsonValueExpr.raw_expr was not initialized in ExecInitExprRec()
1304+
-- causing the Aggrefs contained in it to also not be initialized, which led
1305+
-- to a crash in ExecBuildAggTrans() as mentioned in the bug report:
1306+
-- https://postgr.es/m/18657-1b90ccce2b16bdb8@postgresql.org
1307+
CREATE FUNCTION volatile_one() RETURNS int AS $$ BEGIN RETURN 1; END; $$ LANGUAGE plpgsql VOLATILE;
1308+
CREATE FUNCTION stable_one() RETURNS int AS $$ BEGIN RETURN 1; END; $$ LANGUAGE plpgsql STABLE;
1309+
EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': volatile_one() RETURNING text) FORMAT JSON);
1310+
QUERY PLAN
1311+
-------------------------------------------------------------------------------------------------------------
1312+
Aggregate
1313+
Output: JSON_OBJECT('a' : JSON_OBJECTAGG('b' : volatile_one() RETURNING text) FORMAT JSON RETURNING json)
1314+
-> Result
1315+
(3 rows)
1316+
1317+
SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': volatile_one() RETURNING text) FORMAT JSON);
1318+
json_object
1319+
---------------------
1320+
{"a" : { "b" : 1 }}
1321+
(1 row)
1322+
1323+
EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': stable_one() RETURNING text) FORMAT JSON);
1324+
QUERY PLAN
1325+
-----------------------------------------------------------------------------------------------------------
1326+
Aggregate
1327+
Output: JSON_OBJECT('a' : JSON_OBJECTAGG('b' : stable_one() RETURNING text) FORMAT JSON RETURNING json)
1328+
-> Result
1329+
(3 rows)
1330+
1331+
SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': stable_one() RETURNING text) FORMAT JSON);
1332+
json_object
1333+
---------------------
1334+
{"a" : { "b" : 1 }}
1335+
(1 row)
1336+
1337+
EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': 1 RETURNING text) FORMAT JSON);
1338+
QUERY PLAN
1339+
------------------------------------------------------------------------------------------------
1340+
Aggregate
1341+
Output: JSON_OBJECT('a' : JSON_OBJECTAGG('b' : 1 RETURNING text) FORMAT JSON RETURNING json)
1342+
-> Result
1343+
(3 rows)
1344+
1345+
SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': 1 RETURNING text) FORMAT JSON);
1346+
json_object
1347+
---------------------
1348+
{"a" : { "b" : 1 }}
1349+
(1 row)
1350+
1351+
DROP FUNCTION volatile_one, stable_one;

‎src/test/regress/sql/sqljson.sql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,3 +480,17 @@ SELECT JSON_OBJECTAGG(i: ('111' || i)::bytea FORMAT JSON WITH UNIQUE RETURNING v
480480
CREATEDOMAINsqljson_char2ASchar(2)CHECK (VALUE NOTIN ('12'));
481481
SELECT JSON_SERIALIZE('123' RETURNING sqljson_char2);
482482
SELECT JSON_SERIALIZE('12' RETURNING sqljson_char2);
483+
484+
-- Bug #18657: JsonValueExpr.raw_expr was not initialized in ExecInitExprRec()
485+
-- causing the Aggrefs contained in it to also not be initialized, which led
486+
-- to a crash in ExecBuildAggTrans() as mentioned in the bug report:
487+
-- https://postgr.es/m/18657-1b90ccce2b16bdb8@postgresql.org
488+
CREATEFUNCTIONvolatile_one() RETURNSintAS $$BEGIN RETURN1; END; $$ LANGUAGE plpgsql VOLATILE;
489+
CREATEFUNCTIONstable_one() RETURNSintAS $$BEGIN RETURN1; END; $$ LANGUAGE plpgsql STABLE;
490+
EXPLAIN (VERBOSE, COSTS OFF)SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': volatile_one() RETURNINGtext) FORMAT JSON);
491+
SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': volatile_one() RETURNINGtext) FORMAT JSON);
492+
EXPLAIN (VERBOSE, COSTS OFF)SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': stable_one() RETURNINGtext) FORMAT JSON);
493+
SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': stable_one() RETURNINGtext) FORMAT JSON);
494+
EXPLAIN (VERBOSE, COSTS OFF)SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b':1 RETURNINGtext) FORMAT JSON);
495+
SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b':1 RETURNINGtext) FORMAT JSON);
496+
DROPFUNCTION volatile_one, stable_one;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp