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

Ping (+ cc'ing Alexander who committed this)

On Wed, Nov 23, 2022 at 10:31 PM David G. Johnston <
david(dot)g(dot)johnston(at)gmail(dot)com> wrote:

> Hey,
>
> There is supposedly a recently submitted (i.e., in moderation) bug report
> from a Slack member on this as well, but I decided I didn't want to wait
> for it to post.
>
>
> 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);
>
> The corresponding:
>
> select jsonb_path_match('{"foo": true}'::jsonb, '$bar', '{}', false);
>
> produces the expected <ERROR: could not find jsonpath variable "bar">
>
> The responsible code seems to be (just did some code skimming here):
>
> src/backend/utils/adt/jsonpath_exec(dot)c(at)executeItemOptUnwrapTarget
>
> https://github.com/postgres/postgres/blob/a601366a460f68472bf70c4d94c57baa0a3ed1b2/src/backend/utils/adt/jsonpath_exec.c#L961
>
> case jpiVariable:
> {
> JsonbValue vbuf;
> JsonbValue *v;
> bool hasNext = jspGetNext(jsp, &elem);
>
> if (!hasNext && !found)
> {
> res = jperOk; /* skip evaluation */
> break;
> }
>
> v = hasNext ? &vbuf : palloc(sizeof(*v));
>
> baseObject = cxt->baseObject;
> getJsonPathItem(cxt, jsp, v);
>
> res = executeNextItem(cxt, jsp, &elem,
> v, found, hasNext);
> cxt->baseObject = baseObject;
> }
> break;
>
> Specifically, since exists doesn't care about values, just presence, found
> is false, and since the variable is the only thing present, hasNext is also
> false. Thus we simply return jperOK without ever checking to see what the
> variable actually is. This results in the exists code producing a true
> result.
>
> Looking at this more, it isn't just the variable case that ends up
> producing the wrong answer. Going by the principle that any function call
> of jsonb_path_exists that returns true should produce said match when
> executing jsonb_path_match, this is also broken for the rest (probably) of
> the matched types in the case group. And indeed, if the variable "bar" is
> defined the error in the match case just changes to "single boolean result
> is expected".
>
> select jsonb_path_exists('{"foo": true}'::jsonb, '"bar"', '{}', false); --
> true (bar in double quotes)
> 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.
>
> David J.
>
>