Horton

Thread

Commits

  1. SQL/JSON: Fix JSON_QUERY(... WITH CONDITIONAL WRAPPER)

  1. json_query conditional wrapper bug

    Peter Eisentraut <peter@eisentraut.org> — 2024-08-28T09:21:41Z

    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.

  2. Re: json_query conditional wrapper bug

    Peter Eisentraut <peter@eisentraut.org> — 2024-09-04T10:16:31Z

    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.

  3. Re: json_query conditional wrapper bug

    Andrew Dunstan <andrew@dunslane.net> — 2024-09-04T20:10:37Z

    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

  4. Re: json_query conditional wrapper bug

    Andrew Dunstan <andrew@dunslane.net> — 2024-09-05T15:01:49Z

    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

  5. Re: json_query conditional wrapper bug

    Peter Eisentraut <peter@eisentraut.org> — 2024-09-05T15:51:39Z

    On 05.09.24 17:01, Andrew Dunstan wrote:
> 
> 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,

Here is the relevant bit:

a) Case:
i) If the length of SEQ is 0 (zero), then let WRAPIT be False.
NOTE 479 — This ensures that the ON EMPTY behavior supersedes the 
WRAPPER behavior.
ii) If WRAPPER is WITHOUT ARRAY, then let WRAPIT be False.
iii) If WRAPPER is WITH UNCONDITIONAL ARRAY, then let WRAPIT be True.
iv) If WRAPPER is WITH CONDITIONAL ARRAY, then
Case:
1) If SEQ has a single SQL/JSON item, then let WRAPIT be False.
2) Otherwise, let WRAPIT be True.

 > 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.

I tested the above test expressions as well as the regression test case 
against Oracle and it agrees with my solution.  So it seems to me that 
this piece of documentation is wrong.

  6. Re: json_query conditional wrapper bug

    Andrew Dunstan <andrew@dunslane.net> — 2024-09-05T16:44:56Z

    

> On Sep 5, 2024, at 11:51 AM, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
> On 05.09.24 17:01, Andrew Dunstan wrote:
>>> 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,
> 
> Here is the relevant bit:
> 
> a) Case:
> i) If the length of SEQ is 0 (zero), then let WRAPIT be False.
> NOTE 479 — This ensures that the ON EMPTY behavior supersedes the WRAPPER behavior.
> ii) If WRAPPER is WITHOUT ARRAY, then let WRAPIT be False.
> iii) If WRAPPER is WITH UNCONDITIONAL ARRAY, then let WRAPIT be True.
> iv) If WRAPPER is WITH CONDITIONAL ARRAY, then
> Case:
> 1) If SEQ has a single SQL/JSON item, then let WRAPIT be False.
> 2) Otherwise, let WRAPIT be True.
> 
> > 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.
> 
> I tested the above test expressions as well as the regression test case against Oracle and it agrees with my solution.  So it seems to me that this piece of documentation is wrong.

Oh, odd. Then assuming a scalar is an SQL/JSON item your patch appears correct.

Cheers

Andrew

  7. Re: json_query conditional wrapper bug

    Amit Langote <amitlangote09@gmail.com> — 2024-09-10T08:00:17Z

    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?

-- 
Thanks, Amit Langote

  8. Re: json_query conditional wrapper bug

    Peter Eisentraut <peter@eisentraut.org> — 2024-09-10T20:15:55Z

    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?

  9. Re: json_query conditional wrapper bug

    Amit Langote <amitlangote09@gmail.com> — 2024-09-11T07:51:18Z

    On Wed, Sep 11, 2024 at 5:15 AM Peter Eisentraut <peter@eisentraut.org> wrote:
> 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?

The result of JSON_QUERY() in the CHECK constraint changes, so the
constraint that previously failed now succeeds after this change,
because the comparison looked like this before and after:

-- before
postgres=# select jsonb '[10]' < jsonb '[10]';
 ?column?
----------
 f
(1 row)

-- after
postgres=# select jsonb '10' < jsonb '[10]';
 ?column?
----------
 t
(1 row)

That causes the next constraint to be evaluated and its failure
reported instead.

In the attached, I've adjusted the constraint for the test case to be
a bit more relevant and removed a nearby somewhat redundant test,
mainly because its output changes after the adjustment.

--
Thanks, Amit Langote

  10. Re: json_query conditional wrapper bug

    Peter Eisentraut <peter@eisentraut.org> — 2024-09-11T09:57:14Z

    On 11.09.24 09:51, Amit Langote wrote:
>>> 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?
> 
> The result of JSON_QUERY() in the CHECK constraint changes, so the
> constraint that previously failed now succeeds after this change,
> because the comparison looked like this before and after:
> 
> -- before
> postgres=# select jsonb '[10]' < jsonb '[10]';
>   ?column?
> ----------
>   f
> (1 row)
> 
> -- after
> postgres=# select jsonb '10' < jsonb '[10]';
>   ?column?
> ----------
>   t
> (1 row)
> 
> That causes the next constraint to be evaluated and its failure
> reported instead.
> 
> In the attached, I've adjusted the constraint for the test case to be
> a bit more relevant and removed a nearby somewhat redundant test,
> mainly because its output changes after the adjustment.

Ok, that looks good.  Good that we could clear that up a bit.

  11. Re: json_query conditional wrapper bug

    Amit Langote <amitlangote09@gmail.com> — 2024-09-11T11:25:08Z

    On Wed, Sep 11, 2024 at 6:57 PM Peter Eisentraut <peter@eisentraut.org> wrote:
> On 11.09.24 09:51, Amit Langote wrote:
> >>> 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?
> >
> > The result of JSON_QUERY() in the CHECK constraint changes, so the
> > constraint that previously failed now succeeds after this change,
> > because the comparison looked like this before and after:
> >
> > -- before
> > postgres=# select jsonb '[10]' < jsonb '[10]';
> >   ?column?
> > ----------
> >   f
> > (1 row)
> >
> > -- after
> > postgres=# select jsonb '10' < jsonb '[10]';
> >   ?column?
> > ----------
> >   t
> > (1 row)
> >
> > That causes the next constraint to be evaluated and its failure
> > reported instead.
> >
> > In the attached, I've adjusted the constraint for the test case to be
> > a bit more relevant and removed a nearby somewhat redundant test,
> > mainly because its output changes after the adjustment.
>
> Ok, that looks good.  Good that we could clear that up a bit.

Thanks for checking.  Would you like me to commit it?

-- 
Thanks, Amit Langote

  12. Re: json_query conditional wrapper bug

    Peter Eisentraut <peter@eisentraut.org> — 2024-09-11T11:56:23Z

    On 11.09.24 13:25, Amit Langote wrote:
> On Wed, Sep 11, 2024 at 6:57 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>> On 11.09.24 09:51, Amit Langote wrote:
>>>>> 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?
>>>
>>> The result of JSON_QUERY() in the CHECK constraint changes, so the
>>> constraint that previously failed now succeeds after this change,
>>> because the comparison looked like this before and after:
>>>
>>> -- before
>>> postgres=# select jsonb '[10]' < jsonb '[10]';
>>>    ?column?
>>> ----------
>>>    f
>>> (1 row)
>>>
>>> -- after
>>> postgres=# select jsonb '10' < jsonb '[10]';
>>>    ?column?
>>> ----------
>>>    t
>>> (1 row)
>>>
>>> That causes the next constraint to be evaluated and its failure
>>> reported instead.
>>>
>>> In the attached, I've adjusted the constraint for the test case to be
>>> a bit more relevant and removed a nearby somewhat redundant test,
>>> mainly because its output changes after the adjustment.
>>
>> Ok, that looks good.  Good that we could clear that up a bit.
> 
> Thanks for checking.  Would you like me to commit it?

Please do.

  13. Re: json_query conditional wrapper bug

    Amit Langote <amitlangote09@gmail.com> — 2024-09-12T02:24:52Z

    On Wed, Sep 11, 2024 at 8:56 PM Peter Eisentraut <peter@eisentraut.org> wrote:
> On 11.09.24 13:25, Amit Langote wrote:
> > On Wed, Sep 11, 2024 at 6:57 PM Peter Eisentraut <peter@eisentraut.org> wrote:
> >> On 11.09.24 09:51, Amit Langote wrote:
> >>>>> 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?
> >>>
> >>> The result of JSON_QUERY() in the CHECK constraint changes, so the
> >>> constraint that previously failed now succeeds after this change,
> >>> because the comparison looked like this before and after:
> >>>
> >>> -- before
> >>> postgres=# select jsonb '[10]' < jsonb '[10]';
> >>>    ?column?
> >>> ----------
> >>>    f
> >>> (1 row)
> >>>
> >>> -- after
> >>> postgres=# select jsonb '10' < jsonb '[10]';
> >>>    ?column?
> >>> ----------
> >>>    t
> >>> (1 row)
> >>>
> >>> That causes the next constraint to be evaluated and its failure
> >>> reported instead.
> >>>
> >>> In the attached, I've adjusted the constraint for the test case to be
> >>> a bit more relevant and removed a nearby somewhat redundant test,
> >>> mainly because its output changes after the adjustment.
> >>
> >> Ok, that looks good.  Good that we could clear that up a bit.
> >
> > Thanks for checking.  Would you like me to commit it?
>
> Please do.

Done.  Thanks for the report and the patch.

-- 
Thanks, Amit Langote