From a331512dec25807c4c93e3d537ac7734ee901641 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 24 Jun 2014 21:22:43 -0700
Subject: [PATCH] Fix handling of nested JSON objects in
json_populate_recordset and friends.
populate_recordset_object_start() improperly created a new hash table
(overwriting the link to the existing one) if called at nest levels
greater than one. This resulted in previous fields not appearing in
the final output, as reported by Matti Hameister in bug #10728.
In 9.4 the problem also affects json_to_recordset.
This perhaps missed detection earlier because the default behavior is to
throw an error for nested objects: you have to pass use_json_as_text = true
to see the problem.
In addition, fix query-lifespan leakage of the hashtable created by
json_populate_record(). This is pretty much the same problem recently
fixed in dblink: creating an intended-to-be-temporary context underneath
the executor's per-tuple context isn't enough to make it go away at the
end of the tuple cycle, because MemoryContextReset is not
MemoryContextResetAndDeleteChildren.
Michael Paquier and Tom Lane
---
src/backend/utils/adt/jsonfuncs.c | 28 ++++++++++++++++++++++------
src/test/regress/expected/json.out | 15 +++++++++++++++
src/test/regress/expected/json_1.out | 15 +++++++++++++++
src/test/regress/sql/json.sql | 6 ++++++
4 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 71179f6551..f303860fdb 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -2075,8 +2075,10 @@ populate_record_worker(FunctionCallInfo fcinfo, bool have_record_arg)
* with domain nulls.
*/
if (hash_get_num_entries(json_hash) == 0 && rec)
+ {
+ hash_destroy(json_hash);
PG_RETURN_POINTER(rec);
-
+ }
}
else
{
@@ -2250,6 +2252,9 @@ populate_record_worker(FunctionCallInfo fcinfo, bool have_record_arg)
ReleaseTupleDesc(tupdesc);
+ if (json_hash)
+ hash_destroy(json_hash);
+
PG_RETURN_DATUM(HeapTupleGetDatum(rettuple));
}
@@ -2735,16 +2740,23 @@ populate_recordset_object_start(void *state)
int lex_level = _state->lex->lex_level;
HASHCTL ctl;
+ /* Reject object at top level: we must have an array at level 0 */
if (lex_level == 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("cannot call json_populate_recordset on an object")));
- else if (lex_level > 1 && !_state->use_json_as_text)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("cannot call json_populate_recordset with nested objects")));
- /* set up a new hash for this entry */
+ /* Nested objects, if allowed, require no special processing */
+ if (lex_level > 1)
+ {
+ if (!_state->use_json_as_text)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot call json_populate_recordset with nested objects")));
+ return;
+ }
+
+ /* Object at level 1: set up a new hash table for this object */
memset(&ctl, 0, sizeof(ctl));
ctl.keysize = NAMEDATALEN;
ctl.entrysize = sizeof(JsonHashEntry);
@@ -2771,9 +2783,11 @@ populate_recordset_object_end(void *state)
HeapTupleHeader rec = _state->rec;
HeapTuple rettuple;
+ /* Nested objects require no special processing */
if (_state->lex->lex_level > 1)
return;
+ /* Otherwise, construct and return a tuple based on this level-1 object */
values = (Datum *) palloc(ncolumns * sizeof(Datum));
nulls = (bool *) palloc(ncolumns * sizeof(bool));
@@ -2865,7 +2879,9 @@ populate_recordset_object_end(void *state)
tuplestore_puttuple(_state->tuple_store, rettuple);
+ /* Done with hash for this object */
hash_destroy(json_hash);
+ _state->json_hash = NULL;
}
static void
diff --git a/src/test/regress/expected/json.out b/src/test/regress/expected/json.out
index 8b8556b2e0..d1e32a19a5 100644
--- a/src/test/regress/expected/json.out
+++ b/src/test/regress/expected/json.out
@@ -1007,6 +1007,13 @@ select * from json_populate_recordset(row('def',99,null)::jpop,'[{"a":[100,200,3
select * from json_populate_recordset(row('def',99,null)::jpop,'[{"c":[100,200,300],"x":43.2},{"a":{"z":true},"b":3,"c":"2012-01-20 10:42:53"}]',true) q;
ERROR: invalid input syntax for type timestamp: "[100,200,300]"
+create type jpop2 as (a int, b json, c int, d int);
+select * from json_populate_recordset(null::jpop2, '[{"a":2,"c":3,"b":{"z":4},"d":6}]',true) q;
+ a | b | c | d
+---+---------+---+---
+ 2 | {"z":4} | 3 | 6
+(1 row)
+
-- using the default use_json_as_text argument
select * from json_populate_recordset(null::jpop,'[{"a":"blurfl","x":43.2},{"b":3,"c":"2012-01-20 10:42:53"}]') q;
a | b | c
@@ -1223,3 +1230,11 @@ select * from json_to_recordset('[{"a":1,"b":"foo","d":false},{"a":2,"b":"bar","
2 | bar | t
(2 rows)
+select * from json_to_recordset('[{"a":1,"b":{"d":"foo"},"c":true},{"a":2,"c":false,"b":{"d":"bar"}}]', true)
+ as x(a int, b json, c boolean);
+ a | b | c
+---+-------------+---
+ 1 | {"d":"foo"} | t
+ 2 | {"d":"bar"} | f
+(2 rows)
+
diff --git a/src/test/regress/expected/json_1.out b/src/test/regress/expected/json_1.out
index b32c3ed09f..93cb693b2f 100644
--- a/src/test/regress/expected/json_1.out
+++ b/src/test/regress/expected/json_1.out
@@ -1007,6 +1007,13 @@ select * from json_populate_recordset(row('def',99,null)::jpop,'[{"a":[100,200,3
select * from json_populate_recordset(row('def',99,null)::jpop,'[{"c":[100,200,300],"x":43.2},{"a":{"z":true},"b":3,"c":"2012-01-20 10:42:53"}]',true) q;
ERROR: invalid input syntax for type timestamp: "[100,200,300]"
+create type jpop2 as (a int, b json, c int, d int);
+select * from json_populate_recordset(null::jpop2, '[{"a":2,"c":3,"b":{"z":4},"d":6}]',true) q;
+ a | b | c | d
+---+---------+---+---
+ 2 | {"z":4} | 3 | 6
+(1 row)
+
-- using the default use_json_as_text argument
select * from json_populate_recordset(null::jpop,'[{"a":"blurfl","x":43.2},{"b":3,"c":"2012-01-20 10:42:53"}]') q;
a | b | c
@@ -1219,3 +1226,11 @@ select * from json_to_recordset('[{"a":1,"b":"foo","d":false},{"a":2,"b":"bar","
2 | bar | t
(2 rows)
+select * from json_to_recordset('[{"a":1,"b":{"d":"foo"},"c":true},{"a":2,"c":false,"b":{"d":"bar"}}]', true)
+ as x(a int, b json, c boolean);
+ a | b | c
+---+-------------+---
+ 1 | {"d":"foo"} | t
+ 2 | {"d":"bar"} | f
+(2 rows)
+
diff --git a/src/test/regress/sql/json.sql b/src/test/regress/sql/json.sql
index 3d5ed50126..bc8bb62978 100644
--- a/src/test/regress/sql/json.sql
+++ b/src/test/regress/sql/json.sql
@@ -325,6 +325,9 @@ select * from json_populate_recordset(row('def',99,null)::jpop,'[{"a":"blurfl","
select * from json_populate_recordset(row('def',99,null)::jpop,'[{"a":[100,200,300],"x":43.2},{"a":{"z":true},"b":3,"c":"2012-01-20 10:42:53"}]',true) q;
select * from json_populate_recordset(row('def',99,null)::jpop,'[{"c":[100,200,300],"x":43.2},{"a":{"z":true},"b":3,"c":"2012-01-20 10:42:53"}]',true) q;
+create type jpop2 as (a int, b json, c int, d int);
+select * from json_populate_recordset(null::jpop2, '[{"a":2,"c":3,"b":{"z":4},"d":6}]',true) q;
+
-- using the default use_json_as_text argument
select * from json_populate_recordset(null::jpop,'[{"a":"blurfl","x":43.2},{"b":3,"c":"2012-01-20 10:42:53"}]') q;
@@ -447,3 +450,6 @@ select * from json_to_record('{"a":1,"b":"foo","c":"bar"}',true)
select * from json_to_recordset('[{"a":1,"b":"foo","d":false},{"a":2,"b":"bar","c":true}]',false)
as x(a int, b text, c boolean);
+
+select * from json_to_recordset('[{"a":1,"b":{"d":"foo"},"c":true},{"a":2,"c":false,"b":{"d":"bar"}}]', true)
+ as x(a int, b json, c boolean);
--
2.49.0