Re: json_query conditional wrapper bug - Mailing list pgsql-hackers

From Andrew Dunstan
Subject Re: json_query conditional wrapper bug
Date
Msg-id 30b32911-3a86-41ad-b433-c63d52090f87@dunslane.net
Whole thread Raw
In response to Re: json_query conditional wrapper bug  (Andrew Dunstan <andrew@dunslane.net>)
Responses Re: json_query conditional wrapper bug
List pgsql-hackers


On 2024-09-04 We 4:10 PM, Andrew Dunstan wrote:

On 2024-09-04 We 6:16 AM, Peter Eisentraut wrote:
On 28.08.24 11:21, Peter Eisentraut wrote:
These are ok:

select json_query('{"a": 1, "b": 42}'::jsonb, 'lax $.b' without wrapper);
  json_query
------------
  42

select json_query('{"a": 1, "b": 42}'::jsonb, 'lax $.b' with unconditional wrapper);
  json_query
------------
  [42]

But this appears to be wrong:

select json_query('{"a": 1, "b": 42}'::jsonb, 'lax $.b' with conditional wrapper);
  json_query
------------
  [42]

This should return an unwrapped 42.

If I make the code change illustrated in the attached patch, then I get the correct result here.  And various regression test results change, which, to me, all look more correct after this patch.  I don't know what the code I removed was supposed to accomplish, but it seems to be wrong somehow.  In the current implementation, the WITH CONDITIONAL WRAPPER clause doesn't appear to work correctly in any case I could identify.


Agree the code definitely looks wrong. If anything the test should probably be reversed:

        wrap = count > 1  || !(
            IsAJsonbScalar(singleton) ||
            (singleton->type == jbvBinary &&
JsonContainerIsScalar(singleton->val.binary.data)));

i.e. in the count = 1 case wrap unless it's a scalar or a binary wrapping a scalar. The code could do with a comment about the logic.

I know we're very close to release but we should fix this as it's a new feature.


I thought about this again.

I don't know what the spec says, but the Oracle docs say:

Specify WITH CONDITIONAL WRAPPER to include the array wrapper only if the path expression matches a single scalar value or multiple values of any type. If the path expression matches a single JSON object or JSON array, then the array wrapper is omitted.

So I now think the code that's there now is actually correct, and what you say appears wrong is also correct.



cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

pgsql-hackers by date:

Previous
From: Bertrand Drouvot
Date:
Subject: Re: per backend I/O statistics
Next
From: Karina Litskevich
Date:
Subject: Re: Invalid "trailing junk" error message when non-English letters are used