Re: Bug in jsonb_path_exists (maybe _match) one-element scalar/variable jsonpath handling

From: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
To: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Bug List <pgsql-bugs(at)lists(dot)postgresql(dot)org>
Subject: Re: Bug in jsonb_path_exists (maybe _match) one-element scalar/variable jsonpath handling
Date: 2023-01-08 11:19:30
Message-ID: CAPpHfdubq+FXyh3YndM6xP1tReQF76BckU7EDc-F_b2RTnUVhw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On Thu, Dec 8, 2022 at 1:52 AM David G. Johnston
<david(dot)g(dot)johnston(at)gmail(dot)com> wrote:
> On Mon, Dec 5, 2022 at 4:58 PM Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
>>
>> On Fri, Dec 2, 2022 at 3:18 PM Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
>> > Draft patch fixing the issue is attached. Let me know what you think
>> > about this.
>>
>> Revised patch is attached, wrong pfree() is fixed. I was intended to
>> backpatch it. But the behavior change makes me uneasy.
>>
>> select * from jsonb_path_query('{"a": 10}', '$ ? (@.a < $value)');
>>
>> Currently, this query generates an error because of missing "value"
>> variable. The patch suppress this error. I'm not sure this error
>> should be suppressed. Especially, I'm sure this should be
>> backpatched.
>>
>> Should we fix only existence checking behaviour and let other cases
>> throw an error? Thoughts?
>>
>
> I've attached some additional regression test changes to formally document what it is we are affecting here. The "false" ones seems like it can stand-in for all of the types left behind when the variable one got moved to its own case.
>
> The regressions.diffs file is the changes made by the 0001 patch.
>
> Instead of making everything that today correctly produces a "could not find jsonpath variable" error behave in a non-error way we need to make _exists produce the exact same error. Aside from seemingly being correct on its own merits, it is superior to turning what was a true outcome to a false outcome, which is much more likely to go unnoticed and cause people grief.

This makes sense to me. See the attached patch implementing this.
I'm going to push and backpatch it if no objections.

> The behavior of the introduced constant false jsonpath expression seems internally consistent. Fixing the documentation to make it clear how such an unusual but acceptable jsonpath expression behaves is material for a separate patch.

I would appreciate if you could work on such patch. If so, feel free
to post it.

------
Regards,
Alexander Korotkov

Attachment Content-Type Size
0001-Fix-jsonpath-existense-checking-of-missing-variab-v3.patch application/octet-stream 6.1 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message PG Bug reporting form 2023-01-08 16:28:24 BUG #17739: postgres ts_headline function is not returning matches it should during full text search
Previous Message Dino Maric 2023-01-08 10:13:57 Re: Confusion about the range types