1
0
mirror of https://github.com/postgres/postgres.git synced 2026-10-23 13:29:09 +03:00

Fix jsonb_object_agg() and related aggs used as window functions.

When being used as a window function, these aggregates must run the
finalization step repeatedly on their accumulated state.  During that,
discarding null-valued pairs isn't okay if we want to reject duplicate
keys, since per SQL spec a null-valued pair still participates in
duplicate key checking.  Hence, when both those options are selected
(jsonb_object_agg_unique_strict) run uniqueifyJsonbObject on a copy of
the pairs array, keeping the original data structure unmodified.
We don't want to do that when unique_keys isn't specified, because
repeat de-duplication can be very expensive when there are many
duplicates, but fortunately we don't have to.

uniqueifyJsonbObject did not renumber the pairs it kept, so a pair
added later could lose to an older one with the same key.  Renumber
them to ensure that we preserve last-one-wins behavior across
multiple finalizations.  Also improve related comments.

While adding test cases to cover these mistakes, I chose to sync up
all the json[b]_object_agg tests in json.sql and jsonb.sql.  This
exposed that json_object_agg() fails to de-duplicate keys in its
output, which seems like a pre-existing bug; but for this commit
let's just memorialize its behavior.

Author: shihao zhong <zhong950419@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAGRkXqTCfSu=aEUCvKDTGroEWOcyz3MCv4Xi8x1PZ1T1rEmG=Q@mail.gmail.com
Backpatch-through: 19
This commit is contained in:
Tom Lane
2026-10-14 13:30:24 -04:00
parent 8457258902
commit 56b463f373
6 changed files with 153 additions and 12 deletions

View File

@@ -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 = &copyps;
}
pushJsonbValue(&result, WJB_END_OBJECT, NULL);

View File

@@ -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;

View File

@@ -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('{}');

View File

@@ -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

View File

@@ -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

View File

@@ -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