Re: Patch: Improve Boolean Predicate JSON Path Docs

On Oct 14, 2023, at 19:51, Erik Wienhold <ewie(at)ewie(dot)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>=&gt;</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