Fix JSON aggregates to work properly when final function is re-executed.

tglsfdc · tglsfdc · commit 8571ecb24f57 · 2014-12-02T15:02:43.000-05:00
Davide S. reported that json_agg() sometimes produced multiple trailing
right brackets.  This turns out to be because json_agg_finalfn() attaches
the final right bracket, and was doing so by modifying the aggregate state
in-place.  That's verboten, though unfortunately it seems there's no way
for nodeAgg.c to check for such mistakes.

Fix that back to 9.3 where the broken code was introduced.  In 9.4 and
HEAD, likewise fix json_object_agg(), which had copied the erroneous logic.
Make some cosmetic cleanups as well.
diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c
@@ -85,6 +85,7 @@ static void json_categorize_type(Oid typoid,
 					 Oid *outfuncoid);
 static void datum_to_json(Datum val, bool is_null, StringInfo result,
 			  JsonTypeCategory tcategory, Oid outfuncoid);
+static text *catenate_stringinfo_string(StringInfo buffer, const char *addon);
 
 /* the null action object used for pure validation */
 static JsonSemAction nullSemAction =
@@ -166,7 +167,7 @@ lex_expect(JsonParseContext ctx, JsonLexContext *lex, JsonTokenType token)
 
 /* utility function to check if a string is a valid JSON number */
 extern bool
-IsValidJsonNumber(const char * str, int len)
+IsValidJsonNumber(const char *str, int len)
 {
 	bool		numeric_error;
 	JsonLexContext dummy_lex;
@@ -191,7 +192,7 @@ IsValidJsonNumber(const char * str, int len)
 
 	json_lex_number(&dummy_lex, dummy_lex.input, &numeric_error);
 
-	return ! numeric_error;
+	return !numeric_error;
 }
 
 /*
@@ -1359,6 +1360,7 @@ datum_to_json(Datum val, bool is_null, StringInfo result,
 			break;
 		case JSONTYPE_NUMERIC:
 			outputstr = OidOutputFunctionCall(outfuncoid, val);
+
 			/*
 			 * Don't call escape_json for a non-key if it's a valid JSON
 			 * number.
@@ -1646,6 +1648,8 @@ to_json(PG_FUNCTION_ARGS)
 
 /*
  * json_agg transition function
+ *
+ * aggregate input column as a json array value.
  */
 Datum
 json_agg_transfn(PG_FUNCTION_ARGS)
@@ -1732,12 +1736,32 @@ json_agg_finalfn(PG_FUNCTION_ARGS)
 
 	state = PG_ARGISNULL(0) ? NULL : (StringInfo) PG_GETARG_POINTER(0);
 
+	/* NULL result for no rows in, as is standard with aggregates */
 	if (state == NULL)
 		PG_RETURN_NULL();
 
-	appendStringInfoChar(state, ']');
+	/* Else return state with appropriate array terminator added */
+	PG_RETURN_TEXT_P(catenate_stringinfo_string(state, "]"));
+}
+
+/*
+ * Helper function for aggregates: return given StringInfo's contents plus
+ * specified trailing string, as a text datum.  We need this because aggregate
+ * final functions are not allowed to modify the aggregate state.
+ */
+static text *
+catenate_stringinfo_string(StringInfo buffer, const char *addon)
+{
+	/* custom version of cstring_to_text_with_len */
+	int			buflen = buffer->len;
+	int			addlen = strlen(addon);
+	text	   *result = (text *) palloc(buflen + addlen + VARHDRSZ);
+
+	SET_VARSIZE(result, buflen + addlen + VARHDRSZ);
+	memcpy(VARDATA(result), buffer->data, buflen);
+	memcpy(VARDATA(result) + buflen, addon, addlen);
 
-	PG_RETURN_TEXT_P(cstring_to_text(state->data));
+	return result;
 }
 
 /*
