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 <exclusion@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/ec5e96fb-ee49-4e5f-8a09-3f72b4780538@gmail.com

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
+	 * Note that this will run uniqueifyJsonbObject each time; that's hard to
-	 * for now we won't bother trying to avoid that.)
+	 * 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;
 

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)
+	if (nPairs > 1)
-		qsort_arg(object->val.object.pairs, object->val.object.nPairs, sizeof(JsonbPair),
+		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,
+		int			nNewPairs = 0;
-				   *res;
 
-		while (skip_nulls && object->val.object.nPairs > 0 &&
+		for (int i = 0; i < nPairs; i++)
-			   object->val.object.pairs->value.type == jbvNull)
 		{
-			/* If skip_nulls is true, remove leading items with null */
+			JsonbPair  *ptr = pairs + i;
-			object->val.object.pairs++;
-			object->val.object.nPairs--;
-		}
 
-		if (object->val.object.nPairs > 0)
+			/* Skip duplicate keys */
-		{
+			if (nNewPairs > 0 &&
-			ptr = object->val.object.pairs + 1;
+				lengthCompareJsonbStringValue(&pairs[nNewPairs - 1].key,
-			res = object->val.object.pairs;
+											  &ptr->key) == 0)
-
+				continue;
-			while (ptr - object->val.object.pairs < object->val.object.nPairs)
+			/* Skip null values, if told to */
-			{
+			if (skip_nulls && ptr->value.type == jbvNull)
-				/* Avoid copying over duplicate or null */
+				continue;
-				if (lengthCompareJsonbStringValue(ptr, res) != 0 &&
+			/* Emit this pair, but avoid no-op copy */
-					(!skip_nulls || ptr->value.type != jbvNull))
+			if (i > nNewPairs)
-				{
+				pairs[nNewPairs] = *ptr;
-					res++;
+			nNewPairs++;
-					if (ptr != res)
-						memcpy(res, ptr, sizeof(JsonbPair));
-				}
-				ptr++;
-			}
-
-			object->val.object.nPairs = res + 1 - object->val.object.pairs;
 		}
+		object->val.object.nPairs = nNewPairs;
 	}
 }

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');

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');