On Jun 25, 2024, at 12:46 AM, Степан Неретин <fenixrnd@mail.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
removethat bit, but IIRC this was the only execution function that would return `jperNotFound` when it in fact adds
itemsto the `found` list. The current implementation only looks at one or the other, so it’s not super important, but I
foundthe 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,
whichwould also suppress the error.
> Everything else is cool. Thanks to the patch and the discussion above, I began to understand better how wildcards in
JSONwork.
Yeah, it’s kind of wild TBH.
Best,
David
