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

From: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
To: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Cc: PostgreSQL Bug List <pgsql-bugs(at)lists(dot)postgresql(dot)org>
Subject: Re: Bug in jsonb_path_exists (maybe _match) one-element scalar/variable jsonpath handling
Date: 2022-12-02 12:18:46
Message-ID: CAPpHfdvuHTjZLOWy1BEr0b1qw=mNXqw0OEndnLsdaw85ftDc-A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Hi, David!

Thank you for the report.

On Thu, Nov 24, 2022 at 8:31 AM David G. Johnston
<david(dot)g(dot)johnston(at)gmail(dot)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);

Yes, this definitely looks incorrect.

> 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".

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)
> 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.
BTW, do you mean something like this: jsonb_path_match() equivalent to
jsonb_path_match() expression?

select jsonb_path_match('{"foo": true}'::jsonb, 'exists($bar)',
'{"bar":"foo"}', false);

Draft patch fixing the issue is attached. Let me know what you think
about this.

------
Regards,
Alexander Korotkov

Attachment Content-Type Size
fix_jsonb_variable.patch application/octet-stream 3.2 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message David G. Johnston 2022-12-02 14:24:20 Re: Bug in jsonb_path_exists (maybe _match) one-element scalar/variable jsonpath handling
Previous Message Amit Kapila 2022-12-02 09:20:44 Re: Incorrect messages emitted from pgoutput when using column lists