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
