From 112fdb3528465cc14a2f1dff3dc27f100326d885 Mon Sep 17 00:00:00 2001 From: Andrew Dunstan Date: Wed, 13 Apr 2022 10:26:38 -0400 Subject: [PATCH] Fix finalization for json_objectagg and friends Commit f4fb45d15c misguidedly tried to free some state during aggregate finalization for json_objectagg. This resulted in attempts to access freed memory, especially when the function is used as a window function. Commit 4eb9798879 attempted to ameliorate that, but in fact it should just be ripped out, which is done here. Also add some regression tests for json_objectagg in various flavors as a window function. Original report from Jaime Casanova, diagnosis by Andres Freund. Discussion: https://postgr.es/m/YkfeMNYRCGhySKyg@ahch-to --- src/backend/utils/adt/json.c | 12 --------- src/test/regress/expected/sqljson.out | 35 +++++++++++++++++++++++++++ src/test/regress/sql/sqljson.sql | 18 ++++++++++++++ 3 files changed, 53 insertions(+), 12 deletions(-) diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c index 4cf01300d8..da3f1b9700 100644 --- a/src/backend/utils/adt/json.c +++ b/src/backend/utils/adt/json.c @@ -1002,13 +1002,6 @@ json_unique_builder_init(JsonUniqueBuilderState *cxt) cxt->skipped_keys.data = NULL; } -static void -json_unique_builder_clean(JsonUniqueBuilderState *cxt) -{ - if (cxt->skipped_keys.data) - resetStringInfo(&cxt->skipped_keys); -} - /* On-demand initialization of skipped_keys StringInfo structure */ static StringInfo json_unique_builder_get_skipped_keys(JsonUniqueBuilderState *cxt) @@ -1216,8 +1209,6 @@ json_object_agg_finalfn(PG_FUNCTION_ARGS) if (state == NULL) PG_RETURN_NULL(); - json_unique_builder_clean(&state->unique_check); - /* Else return state with appropriate object terminator added */ PG_RETURN_TEXT_P(catenate_stringinfo_string(state->str, " }")); } @@ -1324,9 +1315,6 @@ json_build_object_worker(int nargs, Datum *args, bool *nulls, Oid *types, appendStringInfoChar(result, '}'); - if (unique_keys) - json_unique_builder_clean(&unique_check); - return PointerGetDatum(cstring_to_text_with_len(result->data, result->len)); } diff --git a/src/test/regress/expected/sqljson.out b/src/test/regress/expected/sqljson.out index 6cadd87868..97a72be970 100644 --- a/src/test/regress/expected/sqljson.out +++ b/src/test/regress/expected/sqljson.out @@ -959,6 +959,41 @@ SELECT JSON_OBJECT('foo' : '1' FORMAT JSON, 'bar' : 'baz' RETURNING json); CREATE OR REPLACE VIEW public.json_object_view AS SELECT JSON_OBJECT('foo' : '1'::text FORMAT JSON, 'bar' : 'baz'::text RETURNING json) AS "json_object" DROP VIEW json_object_view; +SELECT to_json(a) AS a, JSON_OBJECTAGG(k : v WITH UNIQUE KEYS) OVER (ORDER BY k) +FROM (VALUES (1,1), (2,2)) a(k,v); + a | json_objectagg +---------------+---------------------- + {"k":1,"v":1} | { "1" : 1 } + {"k":2,"v":2} | { "1" : 1, "2" : 2 } +(2 rows) + +SELECT to_json(a) AS a, JSON_OBJECTAGG(k : v WITH UNIQUE KEYS) OVER (ORDER BY k) +FROM (VALUES (1,1), (1,2), (2,2)) a(k,v); +ERROR: duplicate JSON key "1" +SELECT to_json(a) AS a, JSON_OBJECTAGG(k : v ABSENT ON NULL WITH UNIQUE KEYS) + OVER (ORDER BY k) +FROM (VALUES (1,1), (1,null), (2,2)) a(k,v); +ERROR: duplicate JSON key "1" +SELECT to_json(a) AS a, JSON_OBJECTAGG(k : v ABSENT ON NULL) +OVER (ORDER BY k) +FROM (VALUES (1,1), (1,null), (2,2)) a(k,v); + a | json_objectagg +------------------+---------------------- + {"k":1,"v":1} | { "1" : 1 } + {"k":1,"v":null} | { "1" : 1 } + {"k":2,"v":2} | { "1" : 1, "2" : 2 } +(3 rows) + +SELECT to_json(a) AS a, JSON_OBJECTAGG(k : v ABSENT ON NULL) +OVER (ORDER BY k RANGE BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) +FROM (VALUES (1,1), (1,null), (2,2)) a(k,v); + a | json_objectagg +------------------+---------------------- + {"k":1,"v":1} | { "1" : 1, "2" : 2 } + {"k":1,"v":null} | { "1" : 1, "2" : 2 } + {"k":2,"v":2} | { "1" : 1, "2" : 2 } +(3 rows) + -- Test JSON_ARRAY deparsing EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_ARRAY('1' FORMAT JSON, 2 RETURNING json); diff --git a/src/test/regress/sql/sqljson.sql b/src/test/regress/sql/sqljson.sql index 51fc659b58..b422ded978 100644 --- a/src/test/regress/sql/sqljson.sql +++ b/src/test/regress/sql/sqljson.sql @@ -292,6 +292,24 @@ SELECT JSON_OBJECT('foo' : '1' FORMAT JSON, 'bar' : 'baz' RETURNING json); DROP VIEW json_object_view; +SELECT to_json(a) AS a, JSON_OBJECTAGG(k : v WITH UNIQUE KEYS) OVER (ORDER BY k) +FROM (VALUES (1,1), (2,2)) a(k,v); + +SELECT to_json(a) AS a, JSON_OBJECTAGG(k : v WITH UNIQUE KEYS) OVER (ORDER BY k) +FROM (VALUES (1,1), (1,2), (2,2)) a(k,v); + +SELECT to_json(a) AS a, JSON_OBJECTAGG(k : v ABSENT ON NULL WITH UNIQUE KEYS) + OVER (ORDER BY k) +FROM (VALUES (1,1), (1,null), (2,2)) a(k,v); + +SELECT to_json(a) AS a, JSON_OBJECTAGG(k : v ABSENT ON NULL) +OVER (ORDER BY k) +FROM (VALUES (1,1), (1,null), (2,2)) a(k,v); + +SELECT to_json(a) AS a, JSON_OBJECTAGG(k : v ABSENT ON NULL) +OVER (ORDER BY k RANGE BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) +FROM (VALUES (1,1), (1,null), (2,2)) a(k,v); + -- Test JSON_ARRAY deparsing EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_ARRAY('1' FORMAT JSON, 2 RETURNING json);