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