Re: Patch bug: Fix jsonpath .* on Arrays

From: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
To: Степан Неретин <fenixrnd(at)mail(dot)ru>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Subject: Re: Patch bug: Fix jsonpath .* on Arrays
Date: 2024-06-25 17:48:45
Message-ID: 4538CB54-AB0A-4AE0-967F-6AFD88C31B8B@justatheory.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Jun 25, 2024, at 12:46 AM, Степан Неретин <fenixrnd(at)mail(dot)ru> wrote:

> Hi! Looks good to me, but I have several comments.

Thanks for your review!

> Your patch improves tests, but why did you change formatting in jsonpath_exec.c? What's the motivation?

It’s not just formatting. From the commit message:

> While at it, teach `executeAnyItem()` to return `jperOk` when `found`
> exist, not because it will be used (the result and `found` are inspected
> by different functions), but because it seems like the proper thing to
> return from `executeAnyItem()` when considered in isolation.

I have since realized it’s not a complete fix for the issue, and hacked around it in my Go version. Would be fine to remove that bit, but IIRC this was the only execution function that would return `jperNotFound` when it in fact adds items to the `found` list. The current implementation only looks at one or the other, so it’s not super important, but I found the inconsistency annoying and sometimes confusing.

> [1] select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*');
> I propose adding a similar test with explicitly specified lax mode: select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'lax $.*'); to show what lax mode is set by default.

Very few of the other tests do so; I can add it if it’s important for this case, though.

> Odd empty result for the test: select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'strict $.*';
> I expected an error like in test [1]. This behavior is not obvious to me.

@? suppresses a number of errors. Perhaps I should add a variant of the error-raising query that passes the silent arg, which would also suppress the error.

> Everything else is cool. Thanks to the patch and the discussion above, I began to understand better how wildcards in JSON work.

Yeah, it’s kind of wild TBH.

Best,

David

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David E. Wheeler 2024-06-25 17:55:17 Re: Proposal: Document ABI Compatibility
Previous Message Bruce Momjian 2024-06-25 17:48:43 Re: Proposal for Updating CRC32C with AVX-512 Algorithm.