Re: json_query conditional wrapper bug

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