On Thu, Dec 07, 2023 at 10:28:50PM -0500, Tom Lane wrote: > Nathan Bossart <nathandboss...@gmail.com> writes: >> I did both of these in v2, although I opted to test that the first >> character after the optional '-' was a digit instead of testing that it was >> _not_ an 'I' or 'N'. > > Yeah, I thought about that too after sending my message. This version > LGTM, although maybe the comment could be slightly more verbose with > explicit reference to Inf/NaN as being the cases we need to quote.
Done. >> I think there are some similar improvements that we can make for >> JSONTYPE_BOOL and JSONTYPE_CAST, but I haven't tested them yet. > > I am suspicious of using > > appendStringInfo(result, "\"%s\"", ...); > > in each of these paths; snprintf is not a terribly cheap thing. > It might be worth expanding that to appendStringInfoChar/ > appendStringInfoString/appendStringInfoChar. WFM. I'll tackle JSONTYPE_BOOL and JSONTYPE_CAST next... -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
>From 48ef8db324c007eef8d5e5fe015e1294e6e410b4 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Thu, 7 Dec 2023 16:45:45 -0600 Subject: [PATCH v3 1/1] avoid some strlen() calls in json.c --- src/backend/utils/adt/json.c | 37 ++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c index cb4311e75f..f975526f33 100644 --- a/src/backend/utils/adt/json.c +++ b/src/backend/utils/adt/json.c @@ -218,13 +218,22 @@ datum_to_json_internal(Datum val, bool is_null, StringInfo result, outputstr = OidOutputFunctionCall(outfuncoid, val); /* - * Don't call escape_json for a non-key if it's a valid JSON - * number. + * Don't quote a non-key if it's a valid JSON number (i.e., not + * "Infinity", "-Infinity", or "NaN"). Since we know this is a + * numeric data type's output, we simplify and open-code the + * validation for better performance. */ - if (!key_scalar && IsValidJsonNumber(outputstr, strlen(outputstr))) + if (!key_scalar && + ((*outputstr >= '0' && *outputstr <= '9') || + (*outputstr == '-' && + (outputstr[1] >= '0' && outputstr[1] <= '9')))) appendStringInfoString(result, outputstr); else - escape_json(result, outputstr); + { + appendStringInfoChar(result, '"'); + appendStringInfoString(result, outputstr); + appendStringInfoChar(result, '"'); + } pfree(outputstr); break; case JSONTYPE_DATE: @@ -232,7 +241,9 @@ datum_to_json_internal(Datum val, bool is_null, StringInfo result, char buf[MAXDATELEN + 1]; JsonEncodeDateTime(buf, val, DATEOID, NULL); - appendStringInfo(result, "\"%s\"", buf); + appendStringInfoChar(result, '"'); + appendStringInfoString(result, buf); + appendStringInfoChar(result, '"'); } break; case JSONTYPE_TIMESTAMP: @@ -240,7 +251,9 @@ datum_to_json_internal(Datum val, bool is_null, StringInfo result, char buf[MAXDATELEN + 1]; JsonEncodeDateTime(buf, val, TIMESTAMPOID, NULL); - appendStringInfo(result, "\"%s\"", buf); + appendStringInfoChar(result, '"'); + appendStringInfoString(result, buf); + appendStringInfoChar(result, '"'); } break; case JSONTYPE_TIMESTAMPTZ: @@ -248,7 +261,9 @@ datum_to_json_internal(Datum val, bool is_null, StringInfo result, char buf[MAXDATELEN + 1]; JsonEncodeDateTime(buf, val, TIMESTAMPTZOID, NULL); - appendStringInfo(result, "\"%s\"", buf); + appendStringInfoChar(result, '"'); + appendStringInfoString(result, buf); + appendStringInfoChar(result, '"'); } break; case JSONTYPE_JSON: @@ -502,8 +517,14 @@ composite_to_json(Datum composite, StringInfo result, bool use_line_feeds) int i; bool needsep = false; const char *sep; + int seplen; + /* + * We can avoid expensive strlen() calls by precalculating the separator + * length. + */ sep = use_line_feeds ? ",\n " : ","; + seplen = use_line_feeds ? sizeof(",\n ") - 1 : sizeof(",") - 1; td = DatumGetHeapTupleHeader(composite); @@ -532,7 +553,7 @@ composite_to_json(Datum composite, StringInfo result, bool use_line_feeds) continue; if (needsep) - appendStringInfoString(result, sep); + appendBinaryStringInfo(result, sep, seplen); needsep = true; attname = NameStr(att->attname); -- 2.25.1