MIME-Version: 1.0
References: 
 <CAKFQuwbeytffJkVnEqDyLZ=rQsznoTh1OgDoOF3VmOMkxcTMjA@mail.gmail.com>
 <CAPpHfdvuHTjZLOWy1BEr0b1qw=mNXqw0OEndnLsdaw85ftDc-A@mail.gmail.com>
 <CAKFQuwawT6=kvAe_YCiozY3TWoE2nPA9zFsbJUBhmO23RMR8LA@mail.gmail.com>
In-Reply-To: 
 <CAKFQuwawT6=kvAe_YCiozY3TWoE2nPA9zFsbJUBhmO23RMR8LA@mail.gmail.com>
From: Alexander Korotkov <aekorotkov@gmail.com>
Date: Fri, 2 Dec 2022 17:57:56 +0300
Message-ID: 
 <CAPpHfdvj+2b5tkEWhLjeLEPvrKQFobqvmVD1VE7ijdwRMoqaAw@mail.gmail.com>
Subject: Re: Bug in jsonb_path_exists (maybe _match) one-element
 scalar/variable jsonpath handling
To: "David G. Johnston" <david.g.johnston@gmail.com>
Cc: PostgreSQL Bug List <pgsql-bugs@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAPpHfdvj%2B2b5tkEWhLjeLEPvrKQFobqvmVD1VE7ijdwRMoqaAw%40mail.gmail.com>
Precedence: bulk

On Fri, Dec 2, 2022 at 5:24 PM David G. Johnston
<david.g.johnston@gmail.com> wrote:
> On Fri, Dec 2, 2022 at 5:18 AM Alexander Korotkov <aekorotkov@gmail.com> =
wrote:
>>
>> On Thu, Nov 24, 2022 at 8:31 AM David G. Johnston
>> <david.g.johnston@gmail.com> wrote:
>> > The following query produces an incorrect result.  It should error (or=
 at worse produce "false"), but it instead produces "true" (this applies to=
 @? too)
>> >
>> > select jsonb_path_exists('{"foo": true}'::jsonb, '$bar', '{}', false);
>>
>> Variable case is definitely broken, but I don't think other cases are
>> broken.  If we're checking for existence and there is a constant, we
>> can immediately return true because constant exists indeed.  That
>> logic doesn't work for variable, which could be non-existent.
>>
>> > select jsonb_path_exists('{"foo": true}'::jsonb, '"bar"', '{}', false)=
; -- true (bar in double quotes)
>
>
> I think my issue with the constant is that the function itself is said to=
 return whether or not the provided path matches the input json.  It is imp=
ossible to match the input json if there is no reference to the input json =
in the jsonpath expression.  As the existing wording promises: "Checks whet=
her the JSON path returns any item for the specified JSON value" - the word=
 item is rightly taken to mean that the path at minimum references the root=
 (i.e., mandatory $) - and that any true result from exists will, if the ex=
pression is used for _match, produce the "item for the specified JSON value=
" that was found.
>
> So I'll stand by my conclusion that the behavior of constants is buggy - =
though I suppose fixing the bug is probably most readily accomplished by ch=
anging the definition of what behavior we are promising and fixing up the d=
ocumentation to express that change.  In short, it is really an error to no=
t specify "$" in your expression - but if you don't you will simply get a t=
rue outcome for the existence test - for backward compatibility reasons.

Thank you for explaining your point, but I can't agree with that.
Constant jsonpath expression is always returning item for the input
JSON value.  Even despite the input value is ignored.  This is
redundant case, but still correct.

>> > select jsonb_path_match('{"foo": true}'::jsonb, '"bar"', '{}', false);=
 -- ERROR:  single boolean result is expected
>> > select jsonb_path_match('{"foo": true}'::jsonb, '$bar', '{"bar":"foo"}=
', false); -- same error as above, as expected
>> >
>> > I expect the missing variable specification to produce jperError and t=
he rest of the block to produce jperNotFound.  The "single boolean result e=
xpected" error seems incorrect though I'm not sure where that is coming fro=
m.  But I'm also not considering, or am even aware of, what the standard we=
 are guided by here says should actually happen.
>>
>> I think jsonb_path_match() behaves correctly, it expects jsonpatch
>> expression to return single boolend and throws an error otherwise.
>
> Yeah, I may have mis-interpreted the meaning of the error message.  Somet=
hing like: "jsonpath expression must produce a single boolean result" would=
 be a bit more clear.

OK, I'm not a native English speaker and can't judge about this.  I
propose this should be considered separately.

BTW, what do you think about the patch?

------
Regards,
Alexander Korotkov