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.g.johnston@gmail.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.c@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.
>
>