Re: inconsistent behaviour of json_to_record and friends with embedded json - Mailing list pgsql-bugs
From | Tom Lane |
---|---|
Subject | Re: inconsistent behaviour of json_to_record and friends with embedded json |
Date | |
Msg-id | 19461.1559506457@sss.pgh.pa.us Whole thread Raw |
In response to | Re: inconsistent behaviour of json_to_record and friends withembedded json (Jeff Janes <jeff.janes@gmail.com>) |
Responses |
Re: inconsistent behaviour of json_to_record and friends with embedded json
|
List | pgsql-bugs |
Jeff Janes <jeff.janes@gmail.com> writes: > On Thu, May 30, 2019 at 10:41 AM Robert Vollmert <rob@vllmrt.net> wrote: >> 1. select * from json_to_record('{"out": "{\"key\": 1}"}') as (out json); >> Postgres 10/11: >> 1. gives >> ERROR: invalid input syntax for type json >> DETAIL: Token "key" is invalid. >> CONTEXT: JSON data, line 1: "{"key... > This is caused by commit: > commit cf35346e813e5a1373f308d397bb0a8f3f21d530 > Author: Andrew Dunstan <andrew@dunslane.net> > Date: Thu Apr 6 22:11:21 2017 -0400 > > Make json_populate_record and friends operate recursively > As far as I can tell, this was not an intended change, so I agree it is > probably a bug. I agree. It looks to me like the problem is this over-optimistic assumption: /* * Add quotes around string value (should be already escaped) if * converting to json/jsonb. */ No, it's *not* already escaped. Fixing the code to use escape_json() is a bit tedious, because for some reason that function wasn't designed to support non-null-terminated input, but with the attached patch we get what seems to me like sane behavior: regression=# select * from json_to_record('{"out": "{\"key\": 1}"}') as (out json); out ---------------- "{\"key\": 1}" (1 row) regression=# select * from json_to_record('{"out": {"key": 1}}') as (out json); out ------------ {"key": 1} (1 row) i.e. what you get is either a string or an object, the same as it was in the input (same for all combinations of json/jsonb in/out). Not terribly surprisingly, this patch changes no existing regression test cases. We should add some, but I've not done so here. Also, modulo this bug for the json-input case, this *is* an intentional behavioral change from 9.6, but it seems fairly poorly documented to me. Should we try to improve that? regards, tom lane diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index 28bdc72..9e7035c 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -2803,26 +2803,7 @@ populate_scalar(ScalarIOData *io, Oid typid, int32 typmod, JsValue *jsv) json = jsv->val.json.str; Assert(json); - - /* already done the hard work in the json case */ - if ((typid == JSONOID || typid == JSONBOID) && - jsv->val.json.type == JSON_TOKEN_STRING) - { - /* - * Add quotes around string value (should be already escaped) if - * converting to json/jsonb. - */ - - if (len < 0) - len = strlen(json); - - str = palloc(len + sizeof(char) * 3); - str[0] = '"'; - memcpy(&str[1], json, len); - str[len + 1] = '"'; - str[len + 2] = '\0'; - } - else if (len >= 0) + if (len >= 0) { /* Need to copy non-null-terminated string */ str = palloc(len + 1 * sizeof(char)); @@ -2830,7 +2811,21 @@ populate_scalar(ScalarIOData *io, Oid typid, int32 typmod, JsValue *jsv) str[len] = '\0'; } else - str = json; /* null-terminated string */ + str = json; /* string is already null-terminated */ + + /* If converting to json/jsonb, make string into valid JSON literal */ + if ((typid == JSONOID || typid == JSONBOID) && + jsv->val.json.type == JSON_TOKEN_STRING) + { + StringInfoData buf; + + initStringInfo(&buf); + escape_json(&buf, str); + /* free temporary buffer */ + if (str != json) + pfree(str); + str = buf.data; + } } else {
pgsql-bugs by date: