Re: json_query conditional wrapper bug

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