From: Tom Lane Date: Sat, 13 Dec 2025 21:18:23 +0000 (-0500) Subject: Fix jsonb_object_agg crash after eliminating null-valued pairs. X-Git-Url: http://git.postgresql.org/gitweb/sta%3Cscript%20data-cfasync=?a=commitdiff_plain;h=HEAD;p=postgresql.git Fix jsonb_object_agg crash after eliminating null-valued pairs. In commit b61aa76e4 I added an assumption in jsonb_object_agg_finalfn that it'd be okay to apply uniqueifyJsonbObject repeatedly to a JsonbValue. I should have studied that code more closely first, because in skip_nulls mode it removed leading nulls by changing the "pairs" array start pointer. This broke the data structure's invariants in two ways: pairs no longer references a repalloc-able chunk, and the distance from pairs to the end of its array is less than parseState->size. So any subsequent addition of more pairs is at high risk of clobbering memory and/or causing repalloc to crash. Unfortunately, adding more pairs is exactly what will happen when the aggregate is being used as a window function. Fix by rewriting uniqueifyJsonbObject to not do that. The prior coding had little to recommend it anyway. Reported-by: Alexander Lakhin Author: Tom Lane Discussion: https://postgr.es/m/ec5e96fb-ee49-4e5f-8a09-3f72b4780538@gmail.com --- diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c index c4fe6e00dcd..dcf84c3fddc 100644 --- a/src/backend/utils/adt/jsonb.c +++ b/src/backend/utils/adt/jsonb.c @@ -1755,8 +1755,11 @@ jsonb_object_agg_finalfn(PG_FUNCTION_ARGS) * the stored JsonbValue data structure. Fortunately, the WJB_END_OBJECT * action will only destructively change fields in the JsonbInState struct * itself, so we can simply invoke pushJsonbValue on a local copy of that. - * (This technique results in running uniqueifyJsonbObject each time, but - * for now we won't bother trying to avoid 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. */ result = arg->pstate; diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c index 28e1ee18ce1..5911b7c4d57 100644 --- a/src/backend/utils/adt/jsonb_util.c +++ b/src/backend/utils/adt/jsonb_util.c @@ -2069,12 +2069,14 @@ lengthCompareJsonbPair(const void *a, const void *b, void *binequal) static void uniqueifyJsonbObject(JsonbValue *object, bool unique_keys, bool skip_nulls) { + JsonbPair *pairs = object->val.object.pairs; + int nPairs = object->val.object.nPairs; bool hasNonUniq = false; Assert(object->type == jbvObject); - if (object->val.object.nPairs > 1) - qsort_arg(object->val.object.pairs, object->val.object.nPairs, sizeof(JsonbPair), + if (nPairs > 1) + qsort_arg(pairs, nPairs, sizeof(JsonbPair), lengthCompareJsonbPair, &hasNonUniq); if (hasNonUniq && unique_keys) @@ -2084,36 +2086,25 @@ uniqueifyJsonbObject(JsonbValue *object, bool unique_keys, bool skip_nulls) if (hasNonUniq || skip_nulls) { - JsonbPair *ptr, - *res; + int nNewPairs = 0; - while (skip_nulls && object->val.object.nPairs > 0 && - object->val.object.pairs->value.type == jbvNull) + for (int i = 0; i < nPairs; i++) { - /* If skip_nulls is true, remove leading items with null */ - object->val.object.pairs++; - object->val.object.nPairs--; - } - - if (object->val.object.nPairs > 0) - { - ptr = object->val.object.pairs + 1; - res = object->val.object.pairs; - - while (ptr - object->val.object.pairs < object->val.object.nPairs) - { - /* Avoid copying over duplicate or null */ - if (lengthCompareJsonbStringValue(ptr, res) != 0 && - (!skip_nulls || ptr->value.type != jbvNull)) - { - res++; - if (ptr != res) - memcpy(res, ptr, sizeof(JsonbPair)); - } - ptr++; - } + JsonbPair *ptr = pairs + i; - object->val.object.nPairs = res + 1 - object->val.object.pairs; + /* Skip duplicate keys */ + if (nNewPairs > 0 && + lengthCompareJsonbStringValue(&pairs[nNewPairs - 1].key, + &ptr->key) == 0) + continue; + /* Skip null values, if told to */ + if (skip_nulls && ptr->value.type == jbvNull) + continue; + /* Emit this pair, but avoid no-op copy */ + if (i > nNewPairs) + pairs[nNewPairs] = *ptr; + nNewPairs++; } + object->val.object.nPairs = nNewPairs; } } diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out index 5a1eb18aba2..2ab7a0df075 100644 --- a/src/test/regress/expected/jsonb.out +++ b/src/test/regress/expected/jsonb.out @@ -1582,6 +1582,38 @@ SELECT jsonb_object_agg(1, NULL::jsonb); SELECT jsonb_object_agg(NULL, '{"a":1}'); ERROR: field name must not be null +SELECT jsonb_object_agg_unique(i, null) OVER (ORDER BY i) + FROM generate_series(1, 10) g(i); + jsonb_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 jsonb_object_agg_unique_strict(i, null) OVER (ORDER BY i) + FROM generate_series(1, 10) g(i); + jsonb_object_agg_unique_strict +-------------------------------- + {} + {} + {} + {} + {} + {} + {} + {} + {} + {} +(10 rows) + 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'); diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql index 57c11acddfe..1453f50ee99 100644 --- a/src/test/regress/sql/jsonb.sql +++ b/src/test/regress/sql/jsonb.sql @@ -402,6 +402,12 @@ SELECT jsonb_build_object('{1,2,3}'::int[], 3); SELECT jsonb_object_agg(1, NULL::jsonb); SELECT jsonb_object_agg(NULL, '{"a":1}'); +SELECT jsonb_object_agg_unique(i, null) OVER (ORDER BY i) + FROM generate_series(1, 10) g(i); + +SELECT jsonb_object_agg_unique_strict(i, null) OVER (ORDER BY i) + FROM generate_series(1, 10) g(i); + 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');