On Wed, Sep 11, 2024 at 5:15 AM Peter Eisentraut <pe...@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 <pe...@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
v3-0001-WIP-Fix-JSON_QUERY-WITH-CONDITIONAL-WRAPPER.patch
Description: Binary data
