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

application/octet-stream

Filename: v2-0001-SQL-JSON-Fix-some-oversights-in-commit-b6e1157e7.patch
Type: application/octet-stream
Part: 0
Message: Re: BUG #18657: Using JSON_OBJECTAGG with volatile function leads to segfault

Patch

Format: format-patch
Series: patch v2-0001
Subject: SQL/JSON: Fix some oversights in commit b6e1157e7
File+
src/backend/nodes/nodeFuncs.c 0 3
src/test/regress/expected/sqljson.out 33 12
src/test/regress/sql/sqljson.sql 10 0
From 7d69cb42d276afc78db78375bbe5cf871b80aad3 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
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 <exclusion@gmail.com>
Diagnosed-by: Tender Wang <tndrwang@gmail.com>
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