public inbox for [email protected]  
help / color / mirror / Atom feed
From: Andrew Dunstan <[email protected]>
To: Peter Eisentraut <[email protected]>
To: pgsql-hackers <[email protected]>
Subject: Re: json_query conditional wrapper bug
Date: Wed, 4 Sep 2024 16:10:37 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>


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.


cheers


andrew



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







view thread (5+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: json_query conditional wrapper bug
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox