On 02/07/2024 07:49, David Rowley wrote:
I've attached a rebased set of patches. The previous set no longer applied.
I looked briefly at the first patch. Seems reasonable.
One little thing that caught my eye is that in populate_scalar(), you
sometimes make a temporary copy of the string to add the
null-terminator, but then call escape_json() which doesn't need the
null-terminator anymore. See attached patch to avoid that. However, it's
not clear to me how to reach that codepath, or if it reachable at all. I
tried to add a NOTICE there and ran the regression tests, but got no
failures.
--
Heikki Linnakangas
Neon (https://neon.tech)
diff --git a/src/backend/utils/adt/jsonfuncs.c
b/src/backend/utils/adt/jsonfuncs.c
index fd3c6b8f432..55c0fc4d73d 100644
--- 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
- {
- str = unconstify(char *, json);
- len = strlen(str);
- }
/* 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, len);
- /* free temporary buffer */
- if (str != json)
- pfree(str);
+ if (len >= 0)
+ escape_json(&buf, json, len);
+ else
+ escape_json_cstring(&buf, json);
str = buf.data;
}
+ else 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);
+ }
}
else
{
