On Oct 14, 2023, at 19:51, Erik Wienhold <e...@ewie.name> wrote: > Thanks for putting this together. See my review at the end.
Appreciate the speedy review!
> Nice. This really does help to make some sense of it. I checked all
> queries and they do work out except for two queries where the path
> expression string is not properly quoted (but the intended output is
> still correct).
🤦🏻‍♂️
>> Follow-ups I’d like to make:
>>
>> 1. Expand the modes section to show how the types of results can vary
>> depending on the mode, thanks to the flattening. Examples:
>>
>> david=# select jsonb_path_query('{"a":[1,2,3,4,5]}', '$.a ?(@[*] > 2)');
>> jsonb_path_query
>> ------------------
>> 3
>> 4
>> 5
>> (3 rows)
>>
>> david=# select jsonb_path_query('{"a":[1,2,3,4,5]}', 'strict $.a ?(@[*] >
>> 2)');
>> jsonb_path_query
>> ------------------
>> [1, 2, 3, 4, 5]
>>
>> 2. Improve the descriptions and examples for @?/jsonb_path_exists()
>> and @@/jsonb_path_match().
>
> +1
I planned to submit these changes in a separate patch, based on Tom Lane’s
suggestion[1]. Would it be preferred to add them to this patch?
> Perhaps make it explicit that the reader must run this in psql in order
> to use \set and :'json' in the ensuing samples? Some of the existing
> examples already use psql output but they do not rely on any psql
> features.
Good call, done.
> This should use <screen>, <userinput>, and <computeroutput> if it shows
> a psql session, e.g.:
>
> <screen>
> <userinput>select jsonb_path_query(:'json', '$.track.segments');</userinput>
> <computeroutput>
>
> jsonb_path_query
> -------------------------------------------------------------------------------------------------------------------------------------------------------------------
> [{"HR": 73, "location": [47.763, 13.4034], "start time": "2018-10-14
> 10:05:14"}, {"HR": 135, "location": [47.706, 13.2635], "start time":
> "2018-10-14 10:39:21"}]
> </computeroutput>
> </screen>
I pokwds around, and it appears the computeroutput bit is used for function
output. So I followed the precedent in queries.sgml[2] and omitted the
computeroutput tags but added prompt, e.g.,
<screen>
<prompt>=></prompt> <userinput>select jsonb_path_query(:'json', 'strict
$.**.HR');</userinput>
jsonb_path_query
------------------
73
135
</screen>
> Also the cast to jsonb is not necessary and only adds clutter IMO.
Right, removed them all in function calls.
>> + <para>
>> + Predicate-only path expressions are necessary for implementation of
>> the
>> + <literal>@@</literal> operator (and the
>> + <function>jsonb_path_match</function> function), and should not be
>> used
>> + with the <literal>@?</literal> operator (or
>> + <function>jsonb_path_exists</function> function).
>> + </para>
>> +
>> + <para>
>> + Conversely, non-predicate <type>jsonpath</type> expressions should
>> not be
>> + used with the <literal>@@</literal> operator (or the
>> + <function>jsonb_path_match</function> function).
>> + </para>
>> + </sect4>
>
> Both paras should be wrapped in a single <note> so that they stand out
> from the rest of the text. Maybe even <warning>, but <note> is already
> used on this page for things that I'd consider warnings.
Agreed. Would be good if we could teach these functions and operators to reject
path expressions they don’t support.
>> + <sect4 id="jsonpath-regular-expression-deviation">
>> + <title>Regular Expression Interpretation</title>
>> + <para>
>> + There are minor differences in the interpretation of regular
>> + expression patterns used in <literal>like_regex</literal> filters, as
>> + described in <xref linkend="jsonpath-regular-expressions"/>.
>> + </para>
>> + </sect4>
>
> <sect3 id="devations-from-the-standard"> should be closed here,
> otherwise the docs won't build. This can be checked with
> `make -C doc/src/sgml check`.
Thanks. That produces a bunch of warnings for postgres.sgml and legal.sgml (and
a failure to load the docbook DTD), but func.sgml is clean now.
> `git diff --check` shows a couple of lines with trailing whitespace
> (mostly psql output).
I must’ve cleaned those after I sent the patch, good now. Updated patch
attached, this time created by `git format-patch -v2`.
Best,
David
[1] https://www.postgresql.org/message-id/1229727.1680535592%40sss.pgh.pa.us
[2]
https://www.postgresql.org/docs/current/queries-table-expressions.html#QUERIES-JOIN
v2-0001-Improve-boolean-predicate-JSON-Path-docs.patch
Description: Binary data
