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

On Sun, Jan 8, 2023 at 2:19 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Thu, Dec 8, 2022 at 1:52 AM David G. Johnston
> <david.g.johnston@gmail.com> wrote:
> > On Mon, Dec 5, 2022 at 4:58 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> >>
> >> On Fri, Dec 2, 2022 at 3:18 PM Alexander Korotkov <aekorotkov@gmail.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.

Pushed and backpatched to 12, where jsonpath first appeared.

------
Regards,
Alexander Korotkov