Skip to content

Commit 75ef435

Browse files
tglsfdctglsfdc
committed
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.
1 parent 1511521 commit 75ef435

File tree

1 file changed

+34
-11
lines changed

1 file changed

+34
-11
lines changed

‎src/backend/utils/adt/json.c

+34-11
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ static void datum_to_json(Datum val, bool is_null, StringInfo result,
9494
bool key_scalar);
9595
static void add_json(Datum val, bool is_null, StringInfo result,
9696
Oid val_type, bool key_scalar);
97+
static text *catenate_stringinfo_string(StringInfo buffer, const char *addon);
9798

9899
/* the null action object used for pure validation */
99100
static JsonSemAction nullSemAction =
@@ -175,7 +176,7 @@ lex_expect(JsonParseContext ctx, JsonLexContext *lex, JsonTokenType token)
175176

176177
/* utility function to check if a string is a valid JSON number */
177178
extern bool
178-
IsValidJsonNumber(const char * str, int len)
179+
IsValidJsonNumber(const char *str, int len)
179180
{
180181
bool numeric_error;
181182
JsonLexContext dummy_lex;
@@ -200,7 +201,7 @@ IsValidJsonNumber(const char * str, int len)
200201

201202
json_lex_number(&dummy_lex, dummy_lex.input, &numeric_error);
202203

203-
return ! numeric_error;
204+
return !numeric_error;
204205
}
205206

206207
/*
@@ -1370,7 +1371,7 @@ datum_to_json(Datum val, bool is_null, StringInfo result,
13701371
text *jsontext;
13711372

13721373
/* callers are expected to ensure that null keys are not passed in */
1373-
Assert( ! (key_scalar && is_null));
1374+
Assert(!(key_scalar && is_null));
13741375

13751376
if (is_null)
13761377
{
@@ -1404,6 +1405,7 @@ datum_to_json(Datum val, bool is_null, StringInfo result,
14041405
break;
14051406
case JSONTYPE_NUMERIC:
14061407
outputstr = OidOutputFunctionCall(outfuncoid, val);
1408+
14071409
/*
14081410
* Don't call escape_json for a non-key if it's a valid JSON
14091411
* number.
@@ -1798,6 +1800,8 @@ to_json(PG_FUNCTION_ARGS)
17981800

17991801
/*
18001802
* json_agg transition function
1803+
*
1804+
* aggregate input column as a json array value.
18011805
*/
18021806
Datum
18031807
json_agg_transfn(PG_FUNCTION_ARGS)
@@ -1884,18 +1888,18 @@ json_agg_finalfn(PG_FUNCTION_ARGS)
18841888

18851889
state = PG_ARGISNULL(0) ? NULL : (StringInfo) PG_GETARG_POINTER(0);
18861890

1891+
/* NULL result for no rows in, as is standard with aggregates */
18871892
if (state == NULL)
18881893
PG_RETURN_NULL();
18891894

1890-
appendStringInfoChar(state, ']');
1891-
1892-
PG_RETURN_TEXT_P(cstring_to_text_with_len(state->data, state->len));
1895+
/* Else return state with appropriate array terminator added */
1896+
PG_RETURN_TEXT_P(catenate_stringinfo_string(state, "]"));
18931897
}
18941898

18951899
/*
18961900
* json_object_agg transition function.
18971901
*
1898-
* aggregate two input columns as a single json value.
1902+
* aggregate two input columns as a single json object value.
18991903
*/
19001904
Datum
19011905
json_object_agg_transfn(PG_FUNCTION_ARGS)
@@ -1909,7 +1913,7 @@ json_object_agg_transfn(PG_FUNCTION_ARGS)
19091913
if (!AggCheckCallContext(fcinfo, &aggcontext))
19101914
{
19111915
/* cannot be called directly because of internal-type argument */
1912-
elog(ERROR, "json_agg_transfn called in non-aggregate context");
1916+
elog(ERROR, "json_object_agg_transfn called in non-aggregate context");
19131917
}
19141918

19151919
if (PG_ARGISNULL(0))
@@ -1976,7 +1980,6 @@ json_object_agg_transfn(PG_FUNCTION_ARGS)
19761980

19771981
/*
19781982
* json_object_agg final function.
1979-
*
19801983
*/
19811984
Datum
19821985
json_object_agg_finalfn(PG_FUNCTION_ARGS)
@@ -1988,12 +1991,32 @@ json_object_agg_finalfn(PG_FUNCTION_ARGS)
19881991

19891992
state = PG_ARGISNULL(0) ? NULL : (StringInfo) PG_GETARG_POINTER(0);
19901993

1994+
/* NULL result for no rows in, as is standard with aggregates */
19911995
if (state == NULL)
19921996
PG_RETURN_NULL();
19931997

1994-
appendStringInfoString(state, " }");
1998+
/* Else return state with appropriate object terminator added */
1999+
PG_RETURN_TEXT_P(catenate_stringinfo_string(state, " }"));
2000+
}
2001+
2002+
/*
2003+
* Helper function for aggregates: return given StringInfo's contents plus
2004+
* specified trailing string, as a text datum. We need this because aggregate
2005+
* final functions are not allowed to modify the aggregate state.
2006+
*/
2007+
static text *
2008+
catenate_stringinfo_string(StringInfo buffer, const char *addon)
2009+
{
2010+
/* custom version of cstring_to_text_with_len */
2011+
int buflen = buffer->len;
2012+
int addlen = strlen(addon);
2013+
text *result = (text *) palloc(buflen + addlen + VARHDRSZ);
2014+
2015+
SET_VARSIZE(result, buflen + addlen + VARHDRSZ);
2016+
memcpy(VARDATA(result), buffer->data, buflen);
2017+
memcpy(VARDATA(result) + buflen, addon, addlen);
19952018

1996-
PG_RETURN_TEXT_P(cstring_to_text_with_len(state->data, state->len));
2019+
return result;
19972020
}
19982021

19992022
/*

0 commit comments

Comments
 (0)