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

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
index ef5608deb827d242ccd464d7781ec32f5d21b965..aa197f74142c2460509bc4b65e9a9f4635fb59d0 100644 (file)
--- 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;
 }
 
 /*