Feedback-ID: ie0a040ee:Fastmail
Message-ID: <749b08c1-a28d-4b40-82ab-e7219cff623f@eisentraut.org>
Date: Tue, 10 Sep 2024 22:15:55 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: json_query conditional wrapper bug
To: Amit Langote <amitlangote09@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
References: <8022e067-818b-45d3-8fab-6e0d94d03626@eisentraut.org>
 <0919c21e-af83-458b-8e40-1228429eaaa4@eisentraut.org>
 <CA+HiwqFARj1L-HPa+ESbK3ZvUTYt0MpFdrE2vjVjhF2oxXSK-g@mail.gmail.com>
Content-Language: en-US
From: Peter Eisentraut <peter@eisentraut.org>
In-Reply-To: <CA+HiwqFARj1L-HPa+ESbK3ZvUTYt0MpFdrE2vjVjhF2oxXSK-g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/749b08c1-a28d-4b40-82ab-e7219cff623f%40eisentraut.org>
Precedence: bulk

On 10.09.24 10:00, Amit Langote wrote:
> Sorry for missing this report and thanks Andrew for the offlist heads up.
> 
> On Wed, Sep 4, 2024 at 7:16 PM Peter Eisentraut <peter@eisentraut.org> 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.
> 
> Agreed that this looks wrong.
> 
> I've wondered why the condition was like that but left it as-is,
> because I thought at one point that that's needed to ensure that the
> returned single scalar SQL/JSON item is valid jsonb.
> 
> I've updated your patch to include updated test outputs and a nearby
> code comment expanded.  Do you intend to commit it or do you prefer
> that I do?

This change looks unrelated:

-ERROR:  new row for relation "test_jsonb_constraints" violates check 
constraint "test_jsonb_constraint4"
+ERROR:  new row for relation "test_jsonb_constraints" violates check 
constraint "test_jsonb_constraint5"

Is this some randomness in the way these constraints are evaluated?