Optimize escaping of JSON strings

There were quite a few places where we either had a non-NUL-terminated
string or a text Datum which we needed to call escape_json() on.  Many of
these places required that a temporary string was created due to the fact
that escape_json() needs a NUL-terminated cstring.  For text types, those
first had to be converted to cstring before calling escape_json() on them.

Here we introduce two new functions to make escaping JSON more optimal:

escape_json_text() can be given a text Datum to append onto the given
buffer.  This is more optimal as it foregoes the need to convert the text
Datum into a cstring.  A temporary allocation is only required if the text
Datum needs to be detoasted.

escape_json_with_len() can be used when the length of the cstring is
already known or the given string isn't NUL-terminated.  Having this
allows various places which were creating a temporary NUL-terminated
string to just call escape_json_with_len() without any temporary memory
allocations.

Discussion: https://postgr.es/m/CAApHDvpLXwMZvbCKcdGfU9XQjGCDm7tFpRdTXuB9PVgpNUYfEQ@mail.gmail.com
Reviewed-by: Melih Mutlu, Heikki Linnakangas

diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
index 999ddad76d977a0b4ec7d7f46172f56dd6d952b1..2125436e40cde79cc23108c8391f2ecf7e95237e 100644 (file)
--- a/contrib/hstore/hstore_io.c
+++ b/contrib/hstore/hstore_io.c
@@ -1343,23 +1343,20 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
        int                     count = HS_COUNT(in);
        char       *base = STRPTR(in);
        HEntry     *entries = ARRPTR(in);
-       StringInfoData tmp,
-                               dst;
+       StringInfoData dst;
 
        if (count == 0)
                PG_RETURN_TEXT_P(cstring_to_text_with_len("{}", 2));
 
-       initStringInfo(&tmp);
        initStringInfo(&dst);
 
        appendStringInfoChar(&dst, '{');
 
        for (i = 0; i < count; i++)
        {
-               resetStringInfo(&tmp);
-               appendBinaryStringInfo(&tmp, HSTORE_KEY(entries, base, i),
-                                                          HSTORE_KEYLEN(entries, i));
-               escape_json(&dst, tmp.data);
+               escape_json_with_len(&dst,
+                                                        HSTORE_KEY(entries, base, i),
+                                                        HSTORE_KEYLEN(entries, i));
                appendStringInfoString(&dst, ": ");
                if (HSTORE_VALISNULL(entries, i))
                        appendStringInfoString(&dst, "null");
@@ -1372,13 +1369,13 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
                        appendStringInfoString(&dst, "false");
                else
                {
-                       resetStringInfo(&tmp);
-                       appendBinaryStringInfo(&tmp, HSTORE_VAL(entries, base, i),
-                                                                  HSTORE_VALLEN(entries, i));
-                       if (IsValidJsonNumber(tmp.data, tmp.len))
-                               appendBinaryStringInfo(&dst, tmp.data, tmp.len);
+                       char       *str = HSTORE_VAL(entries, base, i);
+                       int                     len = HSTORE_VALLEN(entries, i);
+
+                       if (IsValidJsonNumber(str, len))
+                               appendBinaryStringInfo(&dst, str, len);
                        else
-                               escape_json(&dst, tmp.data);
+                               escape_json_with_len(&dst, str, len);
                }
 
                if (i + 1 != count)
@@ -1398,32 +1395,28 @@ hstore_to_json(PG_FUNCTION_ARGS)
        int                     count = HS_COUNT(in);
        char       *base = STRPTR(in);
        HEntry     *entries = ARRPTR(in);
-       StringInfoData tmp,
-                               dst;
+       StringInfoData dst;
 
        if (count == 0)
                PG_RETURN_TEXT_P(cstring_to_text_with_len("{}", 2));
 
-       initStringInfo(&tmp);
        initStringInfo(&dst);
 
        appendStringInfoChar(&dst, '{');
 
        for (i = 0; i < count; i++)
        {
-               resetStringInfo(&tmp);
-               appendBinaryStringInfo(&tmp, HSTORE_KEY(entries, base, i),
-                                                          HSTORE_KEYLEN(entries, i));
-               escape_json(&dst, tmp.data);
+               escape_json_with_len(&dst,
+                                                        HSTORE_KEY(entries, base, i),
+                                                        HSTORE_KEYLEN(entries, i));
                appendStringInfoString(&dst, ": ");
                if (HSTORE_VALISNULL(entries, i))
                        appendStringInfoString(&dst, "null");
                else
                {
-                       resetStringInfo(&tmp);
-                       appendBinaryStringInfo(&tmp, HSTORE_VAL(entries, base, i),
-                                                                  HSTORE_VALLEN(entries, i));
-                       escape_json(&dst, tmp.data);
+                       escape_json_with_len(&dst,
+                                                                HSTORE_VAL(entries, base, i),
+                                                                HSTORE_VALLEN(entries, i));
                }
 
                if (i + 1 != count)

diff --git a/src/backend/backup/backup_manifest.c b/src/backend/backup/backup_manifest.c
index b360a135472e9dc159740fb9c9a32975ec36b1bd..4357cfa31de6631b7eb552174e666f5ade663234 100644 (file)
--- a/src/backend/backup/backup_manifest.c
+++ b/src/backend/backup/backup_manifest.c
@@ -148,7 +148,7 @@ AddFileToBackupManifest(backup_manifest_info *manifest, Oid spcoid,
                pg_verify_mbstr(PG_UTF8, pathname, pathlen, true))
        {
                appendStringInfoString(&buf, "{ \"Path\": ");
-               escape_json(&buf, pathname);
+               escape_json_with_len(&buf, pathname, pathlen);
                appendStringInfoString(&buf, ", ");
        }
        else

diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c
index d719a61f16badd450d37278680f9200b109f6a12..be7bc46038f67c8580362a8edf15c816ef57d960 100644 (file)
--- a/src/backend/utils/adt/json.c
+++ b/src/backend/utils/adt/json.c
@@ -23,6 +23,7 @@
 #include "utils/builtins.h"
 #include "utils/date.h"
 #include "utils/datetime.h"
+#include "utils/fmgroids.h"
 #include "utils/json.h"
 #include "utils/jsonfuncs.h"
 #include "utils/lsyscache.h"
@@ -285,9 +286,16 @@ datum_to_json_internal(Datum val, bool is_null, StringInfo result,
                        pfree(jsontext);
                        break;
                default:
-                       outputstr = OidOutputFunctionCall(outfuncoid, val);
-                       escape_json(result, outputstr);
-                       pfree(outputstr);
+                       /* special-case text types to save useless palloc/memcpy cycles */
+                       if (outfuncoid == F_TEXTOUT || outfuncoid == F_VARCHAROUT ||
+                               outfuncoid == F_BPCHAROUT)
+                               escape_json_text(result, (text *) DatumGetPointer(val));
+                       else
+                       {
+                               outputstr = OidOutputFunctionCall(outfuncoid, val);
+                               escape_json(result, outputstr);
+                               pfree(outputstr);
+                       }
                        break;
        }
 }
@@ -1391,7 +1399,6 @@ json_object(PG_FUNCTION_ARGS)
                                count,
                                i;
        text       *rval;
-       char       *v;
 
        switch (ndims)
        {
@@ -1434,19 +1441,16 @@ json_object(PG_FUNCTION_ARGS)
                                        (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
                                         errmsg("null value not allowed for object key")));
 
-               v = TextDatumGetCString(in_datums[i * 2]);
                if (i > 0)
                        appendStringInfoString(&result, ", ");
-               escape_json(&result, v);
+               escape_json_text(&result, (text *) DatumGetPointer(in_datums[i * 2]));
                appendStringInfoString(&result, " : ");
-               pfree(v);
                if (in_nulls[i * 2 + 1])
                        appendStringInfoString(&result, "null");
                else
                {
-                       v = TextDatumGetCString(in_datums[i * 2 + 1]);
-                       escape_json(&result, v);
-                       pfree(v);
+                       escape_json_text(&result,
+                                                        (text *) DatumGetPointer(in_datums[i * 2 + 1]));
                }
        }
 
@@ -1483,7 +1487,6 @@ json_object_two_arg(PG_FUNCTION_ARGS)
                                val_count,
                                i;
        text       *rval;
-       char       *v;
 
        if (nkdims > 1 || nkdims != nvdims)
                ereport(ERROR,
@@ -1512,20 +1515,15 @@ json_object_two_arg(PG_FUNCTION_ARGS)
                                        (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
                                         errmsg("null value not allowed for object key")));
 
-               v = TextDatumGetCString(key_datums[i]);
                if (i > 0)
                        appendStringInfoString(&result, ", ");
-               escape_json(&result, v);
+               escape_json_text(&result, (text *) DatumGetPointer(key_datums[i]));
                appendStringInfoString(&result, " : ");
-               pfree(v);
                if (val_nulls[i])
                        appendStringInfoString(&result, "null");
                else
-               {
-                       v = TextDatumGetCString(val_datums[i]);
-                       escape_json(&result, v);
-                       pfree(v);
-               }
+                       escape_json_text(&result,
+                                                        (text *) DatumGetPointer(val_datums[i]));
        }
 
        appendStringInfoChar(&result, '}');
@@ -1541,50 +1539,100 @@ json_object_two_arg(PG_FUNCTION_ARGS)
        PG_RETURN_TEXT_P(rval);
 }
 
+/*
+ * escape_json_char
+ *             Inline helper function for escape_json* functions
+ */
+static pg_attribute_always_inline void
+escape_json_char(StringInfo buf, char c)
+{
+       switch (c)
+       {
+               case '\b':
+                       appendStringInfoString(buf, "\\b");
+                       break;
+               case '\f':
+                       appendStringInfoString(buf, "\\f");
+                       break;
+               case '\n':
+                       appendStringInfoString(buf, "\\n");
+                       break;
+               case '\r':
+                       appendStringInfoString(buf, "\\r");
+                       break;
+               case '\t':
+                       appendStringInfoString(buf, "\\t");
+                       break;
+               case '"':
+                       appendStringInfoString(buf, "\\\"");
+                       break;
+               case '\\':
+                       appendStringInfoString(buf, "\\\\");
+                       break;
+               default:
+                       if ((unsigned char) c < ' ')
+                               appendStringInfo(buf, "\\u%04x", (int) c);
+                       else
+                               appendStringInfoCharMacro(buf, c);
+                       break;
+       }
+}
 
 /*
- * Produce a JSON string literal, properly escaping characters in the text.
+ * escape_json
+ *             Produce a JSON string literal, properly escaping the NUL-terminated
+ *             cstring.
  */
 void
 escape_json(StringInfo buf, const char *str)
 {
-       const char *p;
+       appendStringInfoCharMacro(buf, '"');
+
+       for (; *str != '\0'; str++)
+               escape_json_char(buf, *str);
 
        appendStringInfoCharMacro(buf, '"');
-       for (p = str; *p; p++)
-       {
-               switch (*p)
-               {
-                       case '\b':
-                               appendStringInfoString(buf, "\\b");
-                               break;
-                       case '\f':
-                               appendStringInfoString(buf, "\\f");
-                               break;
-                       case '\n':
-                               appendStringInfoString(buf, "\\n");
-                               break;
-                       case '\r':
-                               appendStringInfoString(buf, "\\r");
-                               break;
-                       case '\t':
-                               appendStringInfoString(buf, "\\t");
-                               break;
-                       case '"':
-                               appendStringInfoString(buf, "\\\"");
-                               break;
-                       case '\\':
-                               appendStringInfoString(buf, "\\\\");
-                               break;
-                       default:
-                               if ((unsigned char) *p < ' ')
-                                       appendStringInfo(buf, "\\u%04x", (int) *p);
-                               else
-                                       appendStringInfoCharMacro(buf, *p);
-                               break;
-               }
-       }
+}
+
+/*
+ * escape_json_with_len
+ *             Produce a JSON string literal, properly escaping the possibly not
+ *             NUL-terminated characters in 'str'.  'len' defines the number of bytes
+ *             from 'str' to process.
+ */
+void
+escape_json_with_len(StringInfo buf, const char *str, int len)
+{
        appendStringInfoCharMacro(buf, '"');
+
+       for (int i = 0; i < len; i++)
+               escape_json_char(buf, str[i]);
+
+       appendStringInfoCharMacro(buf, '"');
+}
+
+/*
+ * escape_json_text
+ *             Append 'txt' onto 'buf' and escape using escape_json_with_len.
+ *
+ * This is more efficient than calling text_to_cstring and appending the
+ * result as that could require an additional palloc and memcpy.
+ */
+void
+escape_json_text(StringInfo buf, const text *txt)
+{
+       /* must cast away the const, unfortunately */
+       text       *tunpacked = pg_detoast_datum_packed(unconstify(text *, txt));
+       int                     len = VARSIZE_ANY_EXHDR(tunpacked);
+       char       *str;
+
+       str = VARDATA_ANY(tunpacked);
+
+       escape_json_with_len(buf, str, len);
+
+       /* pfree any detoasted values */
+       if (tunpacked != txt)
+               pfree(tunpacked);
 }
 
 /* Semantic actions for key uniqueness check */

diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index e4562b3c6cea765d0b1717a4aad0392c537ea17f..928552d5514065d8fa8c20aab5606b4a0ea4cf30 100644 (file)
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -354,7 +354,7 @@ jsonb_put_escaped_value(StringInfo out, JsonbValue *scalarVal)
                        appendBinaryStringInfo(out, "null", 4);
                        break;
                case jbvString:
-                       escape_json(out, pnstrdup(scalarVal->val.string.val, scalarVal->val.string.len));
+                       escape_json_with_len(out, scalarVal->val.string.val, scalarVal->val.string.len);
                        break;
                case jbvNumeric:
                        appendStringInfoString(out,

diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 1b681eff5f1c02e3443385b9e899f4601c946117..7076b344b7be192be63ad809e2d66365d2fd4a6c 100644 (file)
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -3133,18 +3133,6 @@ populate_scalar(ScalarIOData *io, Oid typid, int32 typmod, JsValue *jsv,
 
                json = jsv->val.json.str;
                Assert(json);
-               if (len >= 0)
-               {
-                       /* Need to copy non-null-terminated string */
-                       str = palloc(len + 1 * sizeof(char));
-                       memcpy(str, json, len);
-                       str[len] = '\0';
-               }
-               else
-               {
-                       /* string is already null-terminated */
-                       str = unconstify(char *, json);
-               }
 
                /* If converting to json/jsonb, make string into valid JSON literal */
                if ((typid == JSONOID || typid == JSONBOID) &&
@@ -3153,12 +3141,24 @@ populate_scalar(ScalarIOData *io, Oid typid, int32 typmod, JsValue *jsv,
                        StringInfoData buf;
 
                        initStringInfo(&buf);
-                       escape_json(&buf, str);
-                       /* free temporary buffer */
-                       if (str != json)
-                               pfree(str);
+                       if (len >= 0)
+                               escape_json_with_len(&buf, json, len);
+                       else
+                               escape_json(&buf, json);
                        str = buf.data;
                }
+               else if (len >= 0)
+               {
+                       /* create a NUL-terminated version */
+                       str = palloc(len + 1);
+                       memcpy(str, json, len);
+                       str[len] = '\0';
+               }
+               else
+               {
+                       /* string is already NUL-terminated */
+                       str = unconstify(char *, json);
+               }
        }
        else
        {
@@ -5936,7 +5936,7 @@ transform_string_values_scalar(void *state, char *token, JsonTokenType tokentype
        {
                text       *out = _state->action(_state->action_state, token, strlen(token));
 
-               escape_json(_state->strval, text_to_cstring(out));
+               escape_json_text(_state->strval, out);
        }
        else
                appendStringInfoString(_state->strval, token);

diff --git a/src/backend/utils/adt/jsonpath.c b/src/backend/utils/adt/jsonpath.c
index 11e6193e96473263ce07eb7e5f0434fc523da176..0f691bc5f0f9a6d8b48b523ac47ab9fa8cf42602 100644 (file)
--- a/src/backend/utils/adt/jsonpath.c
+++ b/src/backend/utils/adt/jsonpath.c
@@ -523,6 +523,8 @@ printJsonPathItem(StringInfo buf, JsonPathItem *v, bool inKey,
 {
        JsonPathItem elem;
        int                     i;
+       int32           len;
+       char       *str;
 
        check_stack_depth();
        CHECK_FOR_INTERRUPTS();
@@ -533,7 +535,8 @@ printJsonPathItem(StringInfo buf, JsonPathItem *v, bool inKey,
                        appendStringInfoString(buf, "null");
                        break;
                case jpiString:
-                       escape_json(buf, jspGetString(v, NULL));
+                       str = jspGetString(v, &len);
+                       escape_json_with_len(buf, str, len);
                        break;
                case jpiNumeric:
                        if (jspHasNext(v))
@@ -662,7 +665,8 @@ printJsonPathItem(StringInfo buf, JsonPathItem *v, bool inKey,
                case jpiKey:
                        if (inKey)
                                appendStringInfoChar(buf, '.');
-                       escape_json(buf, jspGetString(v, NULL));
+                       str = jspGetString(v, &len);
+                       escape_json_with_len(buf, str, len);
                        break;
                case jpiCurrent:
                        Assert(!inKey);
@@ -674,7 +678,8 @@ printJsonPathItem(StringInfo buf, JsonPathItem *v, bool inKey,
                        break;
                case jpiVariable:
                        appendStringInfoChar(buf, '$');
-                       escape_json(buf, jspGetString(v, NULL));
+                       str = jspGetString(v, &len);
+                       escape_json_with_len(buf, str, len);
                        break;
                case jpiFilter:
                        appendStringInfoString(buf, "?(");
@@ -732,7 +737,9 @@ printJsonPathItem(StringInfo buf, JsonPathItem *v, bool inKey,
 
                        appendStringInfoString(buf, " like_regex ");
 
-                       escape_json(buf, v->content.like_regex.pattern);
+                       escape_json_with_len(buf,
+                                                                v->content.like_regex.pattern,
+                                                                v->content.like_regex.patternlen);
 
                        if (v->content.like_regex.flags)
                        {

diff --git a/src/include/utils/json.h b/src/include/utils/json.h
index 6d7f1b387d1edadf00e5616da80810a19ca4affd..79c1062e1bf2c985eb578866575242eef0e11c4c 100644 (file)
--- a/src/include/utils/json.h
+++ b/src/include/utils/json.h
@@ -18,6 +18,8 @@
 
 /* functions in json.c */
 extern void escape_json(StringInfo buf, const char *str);
+extern void escape_json_with_len(StringInfo buf, const char *str, int len);
+extern void escape_json_text(StringInfo buf, const text *txt);
 extern char *JsonEncodeDateTime(char *buf, Datum value, Oid typid,
                                                                const int *tzp);
 extern bool to_json_is_immutable(Oid typoid);