pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Alexander Korotkov <aekorotkov@gmail.com>
To: David G. Johnston <david.g.johnston@gmail.com>
Cc: PostgreSQL Bug List <pgsql-bugs@lists.postgresql.org>
Subject: Re: Bug in jsonb_path_exists (maybe _match) one-element scalar/variable jsonpath handling
Date: Fri, 2 Dec 2022 17:57:56 +0300
Message-ID: <CAPpHfdvj+2b5tkEWhLjeLEPvrKQFobqvmVD1VE7ijdwRMoqaAw@mail.gmail.com> (raw)
In-Reply-To: <CAKFQuwawT6=kvAe_YCiozY3TWoE2nPA9zFsbJUBhmO23RMR8LA@mail.gmail.com>
References: <CAKFQuwbeytffJkVnEqDyLZ=rQsznoTh1OgDoOF3VmOMkxcTMjA@mail.gmail.com>
	<CAPpHfdvuHTjZLOWy1BEr0b1qw=mNXqw0OEndnLsdaw85ftDc-A@mail.gmail.com>
	<CAKFQuwawT6=kvAe_YCiozY3TWoE2nPA9zFsbJUBhmO23RMR8LA@mail.gmail.com>

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 impossible to match the input json if there is no reference to the input json in the jsonpath expression.  As the existing wording promises: "Checks whether 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 expression 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 changing the definition of what behavior we are promising and fixing up the documentation to express that change.  In short, it is really an error to not specify "$" in your expression - but if you don't you will simply get a true 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 the rest of the block to produce jperNotFound.  The "single boolean result expected" error seems incorrect though I'm not sure where that is coming from.  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.  Something 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





view thread (13+ messages)  latest in thread

Message-ID: <CAPpHfdvj+2b5tkEWhLjeLEPvrKQFobqvmVD1VE7ijdwRMoqaAw@mail.gmail.com>
Permalink:  ../CAPpHfdvj+2b5tkEWhLjeLEPvrKQFobqvmVD1VE7ijdwRMoqaAw@mail.gmail.com/
Also on:    postgresql.org/message-id/CAPpHfdvj+2b5tkEWhLjeLEPvrKQFobqvmVD1VE7ijdwRMoqaAw@mail.gmail.com

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: aekorotkov@gmail.com, david.g.johnston@gmail.com, pgsql-bugs@lists.postgresql.org
  Subject: Re: Bug in jsonb_path_exists (maybe _match) one-element scalar/variable jsonpath handling
  In-Reply-To: <CAPpHfdvj+2b5tkEWhLjeLEPvrKQFobqvmVD1VE7ijdwRMoqaAw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox