diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c index 864c5ac1c85..8d15331920f 100644 --- a/src/backend/utils/adt/jsonb.c +++ b/src/backend/utils/adt/jsonb.c @@ -1708,6 +1708,7 @@ jsonb_object_agg_finalfn(PG_FUNCTION_ARGS) { JsonbAggState *arg; JsonbInState result; + JsonbParseState copyps; Jsonb *out; /* cannot be called directly because of internal-type argument */ @@ -1725,11 +1726,28 @@ jsonb_object_agg_finalfn(PG_FUNCTION_ARGS) * itself, so we can simply invoke pushJsonbValue on a local copy of that. * Note that this will run uniqueifyJsonbObject each time; that's hard to * avoid, since duplicate pairs may have been added since the previous - * finalization. We assume uniqueifyJsonbObject can be applied repeatedly - * (with the same unique_keys/skip_nulls options) without damaging the - * data structure. + * finalization. uniqueifyJsonbObject can be applied repeatedly (with the + * same unique_keys/skip_nulls options) without breaking the data + * structure invariants, except that it removes null values if told to. + * That's problematic when unique_keys is also set, since that might + * prevent detection of a later-added key that duplicates an existing one + * with null value. Therefore, when both options are set, make a copy of + * the pairs array and uniqueify that, keeping the original unmodified. */ result = arg->pstate; + if (result.parseState->unique_keys && result.parseState->skip_nulls) + { + JsonbParseState *oldps = result.parseState; + int npairs = oldps->contVal.val.object.nPairs; + + Assert(oldps->next == NULL); + copyps = *oldps; + copyps.contVal.val.object.pairs = palloc_array(JsonbPair, npairs); + memcpy(copyps.contVal.val.object.pairs, + oldps->contVal.val.object.pairs, + npairs * sizeof(JsonbPair)); + result.parseState = ©ps; + } pushJsonbValue(&result, WJB_END_OBJECT, NULL); diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c index 91fb9ea09bf..e8611715775 100644 --- a/src/backend/utils/adt/jsonb_util.c +++ b/src/backend/utils/adt/jsonb_util.c @@ -2032,15 +2032,17 @@ lengthCompareJsonbString(const char *val1, int len1, const char *val2, int len2) } /* - * qsort_arg() comparator to compare JsonbPair values. + * qsort_arg() comparator to compare JsonbPair keys. * - * Third argument 'binequal' may point to a bool. If it's set, *binequal is set - * to true iff a and b have full binary equality, since some callers have an - * interest in whether the two values are equal or merely equivalent. + * Third argument 'binequal' may point to a bool. If it's not NULL, and + * the two pairs have equal keys, *binequal is set to true. This allows + * uniqueifyJsonbObject to cheaply detect whether there were duplicate keys. * * N.B: String comparisons here are "length-wise" * - * Pairs with equals keys are ordered such that the order field is respected. + * Pairs with equal keys are sorted by descending order fields. Since + * uniqueifyJsonbObject will keep the first pair of any group with equal + * keys, this ensures "last one wins" behavior as desired. */ static int lengthCompareJsonbPair(const void *a, const void *b, void *binequal) @@ -2053,10 +2055,7 @@ lengthCompareJsonbPair(const void *a, const void *b, void *binequal) if (res == 0 && binequal) *((bool *) binequal) = true; - /* - * Guarantee keeping order of equal pair. Unique algorithm will prefer - * first element as value. - */ + /* If equal key strings, sort by descending "order" */ if (res == 0) res = (pa->order > pb->order) ? -1 : 1; @@ -2103,6 +2102,13 @@ uniqueifyJsonbObject(JsonbValue *object, bool unique_keys, bool skip_nulls) /* Emit this pair, but avoid no-op copy */ if (i > nNewPairs) pairs[nNewPairs] = *ptr; + + /* + * Renumber so that order fields again match pairs' array indexes. + * This ensures that if any more pairs are added later, they will + * correctly be seen as later than these. + */ + pairs[nNewPairs].order = nNewPairs; nNewPairs++; } object->val.object.nPairs = nNewPairs; diff --git a/src/test/regress/expected/json.out b/src/test/regress/expected/json.out index 04b478cb468..799fb204d65 100644 --- a/src/test/regress/expected/json.out +++ b/src/test/regress/expected/json.out @@ -2244,6 +2244,51 @@ SELECT json_build_object(json '{"a":1,"b":2}', 3); ERROR: key value must be scalar, not array, composite, or json SELECT json_build_object('{1,2,3}'::int[], 3); ERROR: key value must be scalar, not array, composite, or json +-- handling of NULL values +SELECT json_object_agg(1, NULL::json); + json_object_agg +----------------- + { "1" : null } +(1 row) + +SELECT json_object_agg(NULL, '{"a":1}'); +ERROR: null value not allowed for object key +SELECT json_object_agg_unique(i, null) OVER (ORDER BY i) + FROM generate_series(1, 10) g(i); + json_object_agg_unique +----------------------------------------------------------------------------------------------------------------------------- + { "1" : null } + { "1" : null, "2" : null } + { "1" : null, "2" : null, "3" : null } + { "1" : null, "2" : null, "3" : null, "4" : null } + { "1" : null, "2" : null, "3" : null, "4" : null, "5" : null } + { "1" : null, "2" : null, "3" : null, "4" : null, "5" : null, "6" : null } + { "1" : null, "2" : null, "3" : null, "4" : null, "5" : null, "6" : null, "7" : null } + { "1" : null, "2" : null, "3" : null, "4" : null, "5" : null, "6" : null, "7" : null, "8" : null } + { "1" : null, "2" : null, "3" : null, "4" : null, "5" : null, "6" : null, "7" : null, "8" : null, "9" : null } + { "1" : null, "2" : null, "3" : null, "4" : null, "5" : null, "6" : null, "7" : null, "8" : null, "9" : null, "10" : null } +(10 rows) + +SELECT json_object_agg_unique_strict(i, null) OVER (ORDER BY i) + FROM generate_series(1, 10) g(i); + json_object_agg_unique_strict +------------------------------- + { } + { } + { } + { } + { } + { } + { } + { } + { } + { } +(10 rows) + +-- a NULL-valued pair must still count as a duplicate key +SELECT i, json_object_agg_unique_strict(k, v) OVER (ORDER BY i) + FROM (VALUES (1, 'a', null::int), (2, 'a', 1)) t(i, k, v); +ERROR: duplicate JSON object key value: "a" CREATE TEMP TABLE foo (serial_num int, name text, type text); INSERT INTO foo VALUES (847001,'t15','GE1043'); INSERT INTO foo VALUES (847002,'t16','GE1043'); @@ -2264,6 +2309,26 @@ SELECT json_object_agg(name, type) FROM foo; INSERT INTO foo VALUES (999999, NULL, 'bar'); SELECT json_object_agg(name, type) FROM foo; ERROR: null value not allowed for object key +-- the final function runs once per row, but later pairs must still win +SELECT g, json_object_agg(k, v) OVER (ORDER BY g) + FROM (VALUES (1,'a',1),(1,'a',2),(1,'a',3),(1,'b',5),(2,'b',6),(3,'a',7)) t(g,k,v); + g | json_object_agg +---+---------------------------------------------------------- + 1 | { "a" : 1, "a" : 2, "a" : 3, "b" : 5 } + 1 | { "a" : 1, "a" : 2, "a" : 3, "b" : 5 } + 1 | { "a" : 1, "a" : 2, "a" : 3, "b" : 5 } + 1 | { "a" : 1, "a" : 2, "a" : 3, "b" : 5 } + 2 | { "a" : 1, "a" : 2, "a" : 3, "b" : 5, "b" : 6 } + 3 | { "a" : 1, "a" : 2, "a" : 3, "b" : 5, "b" : 6, "a" : 7 } +(6 rows) + +-- edge case for parser +SELECT json_object_agg(DISTINCT 'a', 'abc'); + json_object_agg +----------------- + { "a" : "abc" } +(1 row) + -- json_object -- empty object, one dimension SELECT json_object('{}'); diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out index 8803d6ed528..fc27adb913b 100644 --- a/src/test/regress/expected/jsonb.out +++ b/src/test/regress/expected/jsonb.out @@ -1614,6 +1614,10 @@ SELECT jsonb_object_agg_unique_strict(i, null) OVER (ORDER BY i) {} (10 rows) +-- a NULL-valued pair must still count as a duplicate key +SELECT i, jsonb_object_agg_unique_strict(k, v) OVER (ORDER BY i) + FROM (VALUES (1, 'a', null::int), (2, 'a', 1)) t(i, k, v); +ERROR: duplicate JSON object key value CREATE TEMP TABLE foo (serial_num int, name text, type text); INSERT INTO foo VALUES (847001,'t15','GE1043'); INSERT INTO foo VALUES (847002,'t16','GE1043'); @@ -1634,6 +1638,19 @@ SELECT jsonb_object_agg(name, type) FROM foo; INSERT INTO foo VALUES (999999, NULL, 'bar'); SELECT jsonb_object_agg(name, type) FROM foo; ERROR: field name must not be null +-- the final function runs once per row, but later pairs must still win +SELECT g, jsonb_object_agg(k, v) OVER (ORDER BY g) + FROM (VALUES (1,'a',1),(1,'a',2),(1,'a',3),(1,'b',5),(2,'b',6),(3,'a',7)) t(g,k,v); + g | jsonb_object_agg +---+------------------ + 1 | {"a": 3, "b": 5} + 1 | {"a": 3, "b": 5} + 1 | {"a": 3, "b": 5} + 1 | {"a": 3, "b": 5} + 2 | {"a": 3, "b": 6} + 3 | {"a": 7, "b": 6} +(6 rows) + -- edge case for parser SELECT jsonb_object_agg(DISTINCT 'a', 'abc'); jsonb_object_agg @@ -1707,6 +1724,9 @@ select jsonb_object('{a,b,"","d e f"}','{1,2,3,"a b c"}'); {"": "3", "a": "1", "b": "2", "d e f": "a b c"} (1 row) +-- jsonb_object_agg_unique requires unique keys +select jsonb_object_agg_unique(mod(i,100), i) from generate_series(0, 199) i; +ERROR: duplicate JSON object key value -- extract_path, extract_path_as_text SELECT jsonb_extract_path('{"f2":{"f3":1},"f4":{"f5":99,"f6":"stringy"}}','f4','f6'); jsonb_extract_path diff --git a/src/test/regress/sql/json.sql b/src/test/regress/sql/json.sql index e9800b21ffe..f53d4c62e1c 100644 --- a/src/test/regress/sql/json.sql +++ b/src/test/regress/sql/json.sql @@ -695,6 +695,20 @@ SELECT json_build_object(json '{"a":1,"b":2}', 3); SELECT json_build_object('{1,2,3}'::int[], 3); +-- handling of NULL values +SELECT json_object_agg(1, NULL::json); +SELECT json_object_agg(NULL, '{"a":1}'); + +SELECT json_object_agg_unique(i, null) OVER (ORDER BY i) + FROM generate_series(1, 10) g(i); + +SELECT json_object_agg_unique_strict(i, null) OVER (ORDER BY i) + FROM generate_series(1, 10) g(i); + +-- a NULL-valued pair must still count as a duplicate key +SELECT i, json_object_agg_unique_strict(k, v) OVER (ORDER BY i) + FROM (VALUES (1, 'a', null::int), (2, 'a', 1)) t(i, k, v); + CREATE TEMP TABLE foo (serial_num int, name text, type text); INSERT INTO foo VALUES (847001,'t15','GE1043'); INSERT INTO foo VALUES (847002,'t16','GE1043'); @@ -708,6 +722,13 @@ SELECT json_object_agg(name, type) FROM foo; INSERT INTO foo VALUES (999999, NULL, 'bar'); SELECT json_object_agg(name, type) FROM foo; +-- the final function runs once per row, but later pairs must still win +SELECT g, json_object_agg(k, v) OVER (ORDER BY g) + FROM (VALUES (1,'a',1),(1,'a',2),(1,'a',3),(1,'b',5),(2,'b',6),(3,'a',7)) t(g,k,v); + +-- edge case for parser +SELECT json_object_agg(DISTINCT 'a', 'abc'); + -- json_object -- empty object, one dimension diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql index 4b346c8977a..a13e904a9e2 100644 --- a/src/test/regress/sql/jsonb.sql +++ b/src/test/regress/sql/jsonb.sql @@ -408,6 +408,10 @@ SELECT jsonb_object_agg_unique(i, null) OVER (ORDER BY i) SELECT jsonb_object_agg_unique_strict(i, null) OVER (ORDER BY i) FROM generate_series(1, 10) g(i); +-- a NULL-valued pair must still count as a duplicate key +SELECT i, jsonb_object_agg_unique_strict(k, v) OVER (ORDER BY i) + FROM (VALUES (1, 'a', null::int), (2, 'a', 1)) t(i, k, v); + CREATE TEMP TABLE foo (serial_num int, name text, type text); INSERT INTO foo VALUES (847001,'t15','GE1043'); INSERT INTO foo VALUES (847002,'t16','GE1043'); @@ -421,6 +425,10 @@ SELECT jsonb_object_agg(name, type) FROM foo; INSERT INTO foo VALUES (999999, NULL, 'bar'); SELECT jsonb_object_agg(name, type) FROM foo; +-- the final function runs once per row, but later pairs must still win +SELECT g, jsonb_object_agg(k, v) OVER (ORDER BY g) + FROM (VALUES (1,'a',1),(1,'a',2),(1,'a',3),(1,'b',5),(2,'b',6),(3,'a',7)) t(g,k,v); + -- edge case for parser SELECT jsonb_object_agg(DISTINCT 'a', 'abc'); @@ -471,6 +479,9 @@ select jsonb_object('{a,b,NULL,"d e f"}','{1,2,3,"a b c"}'); select jsonb_object('{a,b,"","d e f"}','{1,2,3,"a b c"}'); +-- jsonb_object_agg_unique requires unique keys +select jsonb_object_agg_unique(mod(i,100), i) from generate_series(0, 199) i; + -- extract_path, extract_path_as_text