Обсуждение: BUG #13996: json_to_record() returns non-NULL, malformed value for omitted key under some circumstances

The following bug has been logged on the website:

Bug reference:      13996
Logged by:          Johann Visagie
Email address:      johann@visagie.za.net
PostgreSQL version: 9.5.1
Operating system:   OS X 10.11.3
Description:

In the following call to json_to_record(), the target row format `t`
specifies a text column `c` which does *not* appear as a key in the JSON
object used as the function’s argument.

SELECT t.*
FROM json_to_record('{"a":1, "b":{"c":16, "d":2}, "x":8}'::json)
AS t(a int, b json, c text, x int);

By the definition of the function in section 9.15 of the manual, the
statement should return a NULL value for the column `c`.  This is made
explicit in the second Note at the bottom of that manual page, which states
i.a.:

>JSON fields that do not appear in the target row type will be omitted from
the output, and target columns that do not match any JSON field will simply
be NULL.

However, if (as in this example) another key in the JSON object — `b` in
this case — refers to a nested JSON object which *does* contain a key `c`,
the function returns a malformed JSON string as (text)  value for column
`c`:

 a |       b        |    c    | x
---+----------------+---------+---
 1 | {"c":16,"d":2} | {"c":16 | 8
(1 row)

I have not tested whether json_ro_recordset() exhibits the same
misbehaviour.

johann@visagie.za.net writes:
> SELECT t.*
> FROM json_to_record('{"a":1, "b":{"c":16, "d":2}, "x":8}'::json)
> AS t(a int, b json, c text, x int);

> However, if (as in this example) another key in the JSON object - `b` in
> this case - refers to a nested JSON object which *does* contain a key `c`,
> the function returns a malformed JSON string as (text)  value for column
> `c`:

>  a |       b        |    c    | x
> ---+----------------+---------+---
>  1 | {"c":16,"d":2} | {"c":16 | 8
> (1 row)


AFAICT this is a simple thinko in the hash_object_field_end() callback,
as per attached patch that fixes this and doesn't break any existing
regression test cases.  Andrew, do you concur that this is correct,
or is there something I'm missing about the tracking of lex_level?

            regards, tom lane

diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 88225aa..363afa7 100644
*** a/src/backend/utils/adt/jsonfuncs.c
--- b/src/backend/utils/adt/jsonfuncs.c
*************** hash_object_field_end(void *state, char
*** 2438,2444 ****
      /*
       * Ignore nested fields.
       */
!     if (_state->lex->lex_level > 2)
          return;

      /*
--- 2438,2444 ----
      /*
       * Ignore nested fields.
       */
!     if (_state->lex->lex_level > 1)
          return;

      /*

On 03/02/2016 08:04 PM, Tom Lane wrote:
> johann@visagie.za.net writes:
>> SELECT t.*
>> FROM json_to_record('{"a":1, "b":{"c":16, "d":2}, "x":8}'::json)
>> AS t(a int, b json, c text, x int);
>> However, if (as in this example) another key in the JSON object - `b` in
>> this case - refers to a nested JSON object which *does* contain a key `c`,
>> the function returns a malformed JSON string as (text)  value for column
>> `c`:
>>   a |       b        |    c    | x
>> ---+----------------+---------+---
>>   1 | {"c":16,"d":2} | {"c":16 | 8
>> (1 row)
>
> AFAICT this is a simple thinko in the hash_object_field_end() callback,
> as per attached patch that fixes this and doesn't break any existing
> regression test cases.  Andrew, do you concur that this is correct,
> or is there something I'm missing about the tracking of lex_level?
>
>


Looks like you're right. lex_level is incremented at object/array start
and decremented at object/array end. So keys of the outermost object
will be at lex_level 1, and we shouldn't be using anything at a higher
level. I guess we should add something like this as an extra regression
test.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:
> On 03/02/2016 08:04 PM, Tom Lane wrote:
>> AFAICT this is a simple thinko in the hash_object_field_end() callback,
>> as per attached patch that fixes this and doesn't break any existing
>> regression test cases.  Andrew, do you concur that this is correct,
>> or is there something I'm missing about the tracking of lex_level?

> Looks like you're right. lex_level is incremented at object/array start
> and decremented at object/array end. So keys of the outermost object
> will be at lex_level 1, and we shouldn't be using anything at a higher
> level. I guess we should add something like this as an extra regression
> test.

Yeah, I was going to adopt exactly this test case ...

            regards, tom lane