From d9a356ff2e6bb7ed5fb1145af49fa3e51e68a98a Mon Sep 17 00:00:00 2001
From: Andrew Dunstan <andrew@dunslane.net>
Date: Fri, 24 Jul 2015 09:40:46 -0400
Subject: [PATCH] Fix treatment of nulls in jsonb_agg and jsonb_object_agg

The wrong is_null flag was being passed to datum_to_json. Also, null
object key values are not permitted, and this was not being checked
for. Add regression tests covering these cases, and also add those tests
to the json set, even though it was doing the right thing.

Fixes bug #13514, initially diagnosed by Tom Lane.
---
 src/backend/utils/adt/jsonb.c         | 12 +++++++++---
 src/test/regress/expected/json.out    | 21 ++++++++++++++++++++-
 src/test/regress/expected/json_1.out  | 21 ++++++++++++++++++++-
 src/test/regress/expected/jsonb.out   | 19 ++++++++++++++++++-
 src/test/regress/expected/jsonb_1.out | 19 ++++++++++++++++++-
 src/test/regress/sql/json.sql         | 13 +++++++++++--
 src/test/regress/sql/jsonb.sql        | 12 +++++++++++-
 7 files changed, 107 insertions(+), 10 deletions(-)

diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index e68972221ab..154bc3626c9 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -705,6 +705,7 @@ datum_to_jsonb(Datum val, bool is_null, JsonbInState *result,
 
 	if (is_null)
 	{
+		Assert(!key_scalar);
 		jb.type = jbvNull;
 	}
 	else if (key_scalar &&
@@ -1606,7 +1607,7 @@ jsonb_agg_transfn(PG_FUNCTION_ARGS)
 
 	memset(&elem, 0, sizeof(JsonbInState));
 
-	datum_to_jsonb(val, false, &elem, tcategory, outfuncoid, false);
+	datum_to_jsonb(val, PG_ARGISNULL(1), &elem, tcategory, outfuncoid, false);
 
 	jbelem = JsonbValueToJsonb(elem.res);
 
@@ -1752,7 +1753,12 @@ jsonb_object_agg_transfn(PG_FUNCTION_ARGS)
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 				 errmsg("could not determine input data type")));
 
-	val = PG_ARGISNULL(1) ? (Datum) 0 : PG_GETARG_DATUM(1);
+	if (PG_ARGISNULL(1))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("field name must not be null")));
+
+	val = PG_GETARG_DATUM(1);
 
 	jsonb_categorize_type(val_type,
 						  &tcategory, &outfuncoid);
@@ -1777,7 +1783,7 @@ jsonb_object_agg_transfn(PG_FUNCTION_ARGS)
 
 	memset(&elem, 0, sizeof(JsonbInState));
 
-	datum_to_jsonb(val, false, &elem, tcategory, outfuncoid, false);
+	datum_to_jsonb(val, PG_ARGISNULL(2), &elem, tcategory, outfuncoid, false);
 
 	jbval = JsonbValueToJsonb(elem.res);
 
diff --git a/src/test/regress/expected/json.out b/src/test/regress/expected/json.out
index 43ca67dddfa..eb6b26b241e 100644
--- a/src/test/regress/expected/json.out
+++ b/src/test/regress/expected/json.out
@@ -465,7 +465,7 @@ SELECT json_agg(q)
   {"b":"a2","c":5,"z":[{"f1":2,"f2":[1,2,3]},{"f1":5,"f2":[4,5,6]}]}]
 (1 row)
 
-SELECT json_agg(q)
+SELECT json_agg(q ORDER BY x, y)
   FROM rows q;
        json_agg        
 -----------------------
@@ -474,6 +474,16 @@ SELECT json_agg(q)
   {"x":3,"y":"txt3"}]
 (1 row)
 
+UPDATE rows SET x = NULL WHERE x = 1;
+SELECT json_agg(q ORDER BY x NULLS FIRST, y)
+  FROM rows q;
+         json_agg         
+--------------------------
+ [{"x":null,"y":"txt1"}, +
+  {"x":2,"y":"txt2"},    +
+  {"x":3,"y":"txt3"}]
+(1 row)
+
 -- non-numeric output
 SELECT row_to_json(q)
 FROM (SELECT 'NaN'::float8 AS "float8field") q;
@@ -1574,6 +1584,15 @@ FROM foo;
  {"turbines" : { "847001" : {"name" : "t15", "type" : "GE1043"}, "847002" : {"name" : "t16", "type" : "GE1043"}, "847003" : {"name" : "sub-alpha", "type" : "GESS90"} }}
 (1 row)
 
+SELECT json_object_agg(name, type) FROM foo;
+                        json_object_agg                         
+----------------------------------------------------------------
+ { "t15" : "GE1043", "t16" : "GE1043", "sub-alpha" : "GESS90" }
+(1 row)
+
+INSERT INTO foo VALUES (999999, NULL, 'bar');
+SELECT json_object_agg(name, type) FROM foo;
+ERROR:  field name must not be null
 -- json_object
 -- one dimension
 SELECT json_object('{a,1,b,2,3,NULL,"d e f","a b c"}');
diff --git a/src/test/regress/expected/json_1.out b/src/test/regress/expected/json_1.out
index 155f414ea4c..48543e8c356 100644
--- a/src/test/regress/expected/json_1.out
+++ b/src/test/regress/expected/json_1.out
@@ -465,7 +465,7 @@ SELECT json_agg(q)
   {"b":"a2","c":5,"z":[{"f1":2,"f2":[1,2,3]},{"f1":5,"f2":[4,5,6]}]}]
 (1 row)
 
-SELECT json_agg(q)
+SELECT json_agg(q ORDER BY x, y)
   FROM rows q;
        json_agg        
 -----------------------
@@ -474,6 +474,16 @@ SELECT json_agg(q)
   {"x":3,"y":"txt3"}]
 (1 row)
 
+UPDATE rows SET x = NULL WHERE x = 1;
+SELECT json_agg(q ORDER BY x NULLS FIRST, y)
+  FROM rows q;
+         json_agg         
+--------------------------
+ [{"x":null,"y":"txt1"}, +
+  {"x":2,"y":"txt2"},    +
+  {"x":3,"y":"txt3"}]
+(1 row)
+
 -- non-numeric output
 SELECT row_to_json(q)
 FROM (SELECT 'NaN'::float8 AS "float8field") q;
@@ -1570,6 +1580,15 @@ FROM foo;
  {"turbines" : { "847001" : {"name" : "t15", "type" : "GE1043"}, "847002" : {"name" : "t16", "type" : "GE1043"}, "847003" : {"name" : "sub-alpha", "type" : "GESS90"} }}
 (1 row)
 
+SELECT json_object_agg(name, type) FROM foo;
+                        json_object_agg                         
+----------------------------------------------------------------
+ { "t15" : "GE1043", "t16" : "GE1043", "sub-alpha" : "GESS90" }
+(1 row)
+
+INSERT INTO foo VALUES (999999, NULL, 'bar');
+SELECT json_object_agg(name, type) FROM foo;
+ERROR:  field name must not be null
 -- json_object
 -- one dimension
 SELECT json_object('{a,1,b,2,3,NULL,"d e f","a b c"}');
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index 0ccc0f7a795..17656d4413a 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -369,13 +369,21 @@ SELECT jsonb_agg(q)
  [{"b": "a1", "c": 4, "z": [{"f1": 1, "f2": [1, 2, 3]}, {"f1": 4, "f2": [4, 5, 6]}]}, {"b": "a1", "c": 5, "z": [{"f1": 1, "f2": [1, 2, 3]}, {"f1": 5, "f2": [4, 5, 6]}]}, {"b": "a2", "c": 4, "z": [{"f1": 2, "f2": [1, 2, 3]}, {"f1": 4, "f2": [4, 5, 6]}]}, {"b": "a2", "c": 5, "z": [{"f1": 2, "f2": [1, 2, 3]}, {"f1": 5, "f2": [4, 5, 6]}]}]
 (1 row)
 
-SELECT jsonb_agg(q)
+SELECT jsonb_agg(q ORDER BY x, y)
   FROM rows q;
                                jsonb_agg                               
 -----------------------------------------------------------------------
  [{"x": 1, "y": "txt1"}, {"x": 2, "y": "txt2"}, {"x": 3, "y": "txt3"}]
 (1 row)
 
+UPDATE rows SET x = NULL WHERE x = 1;
+SELECT jsonb_agg(q ORDER BY x NULLS FIRST, y)
+  FROM rows q;
+                                jsonb_agg                                 
+--------------------------------------------------------------------------
+ [{"x": null, "y": "txt1"}, {"x": 2, "y": "txt2"}, {"x": 3, "y": "txt3"}]
+(1 row)
+
 -- jsonb extraction functions
 CREATE TEMP TABLE test_jsonb (
        json_type text,
@@ -1393,6 +1401,15 @@ FROM foo;
  {"turbines": {"847001": {"name": "t15", "type": "GE1043"}, "847002": {"name": "t16", "type": "GE1043"}, "847003": {"name": "sub-alpha", "type": "GESS90"}}}
 (1 row)
 
+SELECT jsonb_object_agg(name, type) FROM foo;
+                     jsonb_object_agg                      
+-----------------------------------------------------------
+ {"t15": "GE1043", "t16": "GE1043", "sub-alpha": "GESS90"}
+(1 row)
+
+INSERT INTO foo VALUES (999999, NULL, 'bar');
+SELECT jsonb_object_agg(name, type) FROM foo;
+ERROR:  field name must not be null
 -- jsonb_object
 -- one dimension
 SELECT jsonb_object('{a,1,b,2,3,NULL,"d e f","a b c"}');
diff --git a/src/test/regress/expected/jsonb_1.out b/src/test/regress/expected/jsonb_1.out
index 7b23a993574..86b1162ac29 100644
--- a/src/test/regress/expected/jsonb_1.out
+++ b/src/test/regress/expected/jsonb_1.out
@@ -369,13 +369,21 @@ SELECT jsonb_agg(q)
  [{"b": "a1", "c": 4, "z": [{"f1": 1, "f2": [1, 2, 3]}, {"f1": 4, "f2": [4, 5, 6]}]}, {"b": "a1", "c": 5, "z": [{"f1": 1, "f2": [1, 2, 3]}, {"f1": 5, "f2": [4, 5, 6]}]}, {"b": "a2", "c": 4, "z": [{"f1": 2, "f2": [1, 2, 3]}, {"f1": 4, "f2": [4, 5, 6]}]}, {"b": "a2", "c": 5, "z": [{"f1": 2, "f2": [1, 2, 3]}, {"f1": 5, "f2": [4, 5, 6]}]}]
 (1 row)
 
-SELECT jsonb_agg(q)
+SELECT jsonb_agg(q ORDER BY x, y)
   FROM rows q;
                                jsonb_agg                               
 -----------------------------------------------------------------------
  [{"x": 1, "y": "txt1"}, {"x": 2, "y": "txt2"}, {"x": 3, "y": "txt3"}]
 (1 row)
 
+UPDATE rows SET x = NULL WHERE x = 1;
+SELECT jsonb_agg(q ORDER BY x NULLS FIRST, y)
+  FROM rows q;
+                                jsonb_agg                                 
+--------------------------------------------------------------------------
+ [{"x": null, "y": "txt1"}, {"x": 2, "y": "txt2"}, {"x": 3, "y": "txt3"}]
+(1 row)
+
 -- jsonb extraction functions
 CREATE TEMP TABLE test_jsonb (
        json_type text,
@@ -1393,6 +1401,15 @@ FROM foo;
  {"turbines": {"847001": {"name": "t15", "type": "GE1043"}, "847002": {"name": "t16", "type": "GE1043"}, "847003": {"name": "sub-alpha", "type": "GESS90"}}}
 (1 row)
 
+SELECT jsonb_object_agg(name, type) FROM foo;
+                     jsonb_object_agg                      
+-----------------------------------------------------------
+ {"t15": "GE1043", "t16": "GE1043", "sub-alpha": "GESS90"}
+(1 row)
+
+INSERT INTO foo VALUES (999999, NULL, 'bar');
+SELECT jsonb_object_agg(name, type) FROM foo;
+ERROR:  field name must not be null
 -- jsonb_object
 -- one dimension
 SELECT jsonb_object('{a,1,b,2,3,NULL,"d e f","a b c"}');
diff --git a/src/test/regress/sql/json.sql b/src/test/regress/sql/json.sql
index 8c3b73f5b3e..f631480f967 100644
--- a/src/test/regress/sql/json.sql
+++ b/src/test/regress/sql/json.sql
@@ -126,7 +126,12 @@ SELECT json_agg(q)
          FROM generate_series(1,2) x,
               generate_series(4,5) y) q;
 
-SELECT json_agg(q)
+SELECT json_agg(q ORDER BY x, y)
+  FROM rows q;
+
+UPDATE rows SET x = NULL WHERE x = 1;
+
+SELECT json_agg(q ORDER BY x NULLS FIRST, y)
   FROM rows q;
 
 -- non-numeric output
@@ -442,7 +447,6 @@ SELECT json_build_object(
        'd', json_build_object('e',array[9,8,7]::int[],
            'f', (select row_to_json(r) from ( select relkind, oid::regclass as name from pg_class where relname = 'pg_class') r)));
 
-
 -- empty objects/arrays
 SELECT json_build_array();
 
@@ -468,6 +472,11 @@ INSERT INTO foo VALUES (847003,'sub-alpha','GESS90');
 SELECT json_build_object('turbines',json_object_agg(serial_num,json_build_object('name',name,'type',type)))
 FROM foo;
 
+SELECT json_object_agg(name, type) FROM foo;
+
+INSERT INTO foo VALUES (999999, NULL, 'bar');
+SELECT json_object_agg(name, type) FROM foo;
+
 -- json_object
 
 -- one dimension
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 3d2d8abfc1d..83ed4ebd93f 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -93,7 +93,12 @@ SELECT jsonb_agg(q)
          FROM generate_series(1,2) x,
               generate_series(4,5) y) q;
 
-SELECT jsonb_agg(q)
+SELECT jsonb_agg(q ORDER BY x, y)
+  FROM rows q;
+
+UPDATE rows SET x = NULL WHERE x = 1;
+
+SELECT jsonb_agg(q ORDER BY x NULLS FIRST, y)
   FROM rows q;
 
 -- jsonb extraction functions
@@ -334,6 +339,11 @@ INSERT INTO foo VALUES (847003,'sub-alpha','GESS90');
 SELECT jsonb_build_object('turbines',jsonb_object_agg(serial_num,jsonb_build_object('name',name,'type',type)))
 FROM foo;
 
+SELECT jsonb_object_agg(name, type) FROM foo;
+
+INSERT INTO foo VALUES (999999, NULL, 'bar');
+SELECT jsonb_object_agg(name, type) FROM foo;
+
 -- jsonb_object
 
 -- one dimension
-- 
GitLab