v4-0001-SQL-JSON-Fix-some-oversights-in-commit-b6e1157e7.patch

From 27d45cbf9c3cdbb8efef344b43dd44cfaf8f4cee Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Thu, 17 Oct 2024 15:15:08 +0900
Subject: [PATCH v4] SQL/JSON: Fix some oversights in commit b6e1157e7

Initialize JsonValueExpr.raw_expr in ExecInitExprRec() even if it

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.

Outputs of some tests in sqljson.out is modified to account for the
fact that raw_expr remains unchanged during

Bug: #18657
Reported-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.org
Backpatch-through: 16
---
 src/backend/executor/execExpr.c       |  2 ++
 src/backend/optimizer/util/clauses.c  |  3 +-
 src/test/regress/expected/sqljson.out | 50 +++++++++++++++++++++++++++
 src/test/regress/sql/sqljson.sql      | 15 ++++++++
 4 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index c8077aa57b..b8d69536ac 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -2307,6 +2307,8 @@ ExecInitExprRec(Expr *node, ExprState *state,
 			{
 				JsonValueExpr *jve = (JsonValueExpr *) node;
 
+				Assert(jve->raw_expr != NULL);
+				ExecInitExprRec(jve->raw_expr, state, resv, resnull);
 				Assert(jve->formatted_expr != NULL);
 				ExecInitExprRec(jve->formatted_expr, state, resv, resnull);
 				break;
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index b4e085e9d4..07edc02df3 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -2917,9 +2917,10 @@ eval_const_expressions_mutator(Node *node,
 				JsonValueExpr *jve = (JsonValueExpr *) node;
 				Node	   *formatted;
 
+				Assert(jve->formatted_expr);
 				formatted = eval_const_expressions_mutator((Node *) jve->formatted_expr,
 														   context);
-				if (formatted && IsA(formatted, Const))
+				if (IsA(formatted, Const))
 					return formatted;
 				break;
 			}
diff --git a/src/test/regress/expected/sqljson.out b/src/test/regress/expected/sqljson.out
index 9adfbe15f6..5fe5968b42 100644
--- a/src/test/regress/expected/sqljson.out
+++ b/src/test/regress/expected/sqljson.out
@@ -1300,3 +1300,53 @@ 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;
+CREATE FUNCTION stable_one() RETURNS int AS $$ BEGIN RETURN 1; END; $$ LANGUAGE plpgsql STABLE;
+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)
+
+EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': stable_one() RETURNING text) FORMAT JSON);
+                                                QUERY PLAN                                                 
+-----------------------------------------------------------------------------------------------------------
+ Aggregate
+   Output: JSON_OBJECT('a' : JSON_OBJECTAGG('b' : stable_one() RETURNING text) FORMAT JSON RETURNING json)
+   ->  Result
+(3 rows)
+
+SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': stable_one() RETURNING text) FORMAT JSON);
+     json_object     
+---------------------
+ {"a" : { "b" : 1 }}
+(1 row)
+
+EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': 1 RETURNING text) FORMAT JSON);
+                                           QUERY PLAN                                           
+------------------------------------------------------------------------------------------------
+ Aggregate
+   Output: JSON_OBJECT('a' : JSON_OBJECTAGG('b' : 1 RETURNING text) FORMAT JSON RETURNING json)
+   ->  Result
+(3 rows)
+
+SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': 1 RETURNING text) FORMAT JSON);
+     json_object     
+---------------------
+ {"a" : { "b" : 1 }}
+(1 row)
+
+DROP FUNCTION volatile_one, stable_one;
diff --git a/src/test/regress/sql/sqljson.sql b/src/test/regress/sql/sqljson.sql
index 42dbec26d6..031cab7365 100644
--- a/src/test/regress/sql/sqljson.sql
+++ b/src/test/regress/sql/sqljson.sql
@@ -480,3 +480,18 @@ 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;
+CREATE FUNCTION stable_one() RETURNS int AS $$ BEGIN RETURN 1; END; $$ LANGUAGE plpgsql STABLE;
+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);
+EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': stable_one() RETURNING text) FORMAT JSON);
+SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': stable_one() RETURNING text) FORMAT JSON);
+EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': 1 RETURNING text) FORMAT JSON);
+SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': 1 RETURNING text) FORMAT JSON);
+DROP FUNCTION volatile_one, stable_one;
-- 
2.43.0