Re: Patch: Improve Boolean Predicate JSON Path Docs - Mailing list pgsql-hackers

On 2023-10-16 01:04 +0200, David E. Wheeler write:
> On Oct 14, 2023, at 19:51, Erik Wienhold <ewie@ewie.name> wrote:
> 
> > Thanks for putting this together.  See my review at the end.
> 
> Appreciate the speedy review!

You're welcome.

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

Your call but I'm not against including it in this patch because it
already touches the modes section.

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

Okay,  Not sure what the preferred style is but I saw <userinput> and
<computeroutput> used together in doc/src/sgml/ref/createuser.sgml.
But it's not applied consistently in the rest of the docs.

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

Right, you mentioned that idea in [1] (separate types).  Not sure what
the best strategy here is but it's likely to break existing queries.
Maybe deprecating unsupported path expressions in the next major release
and changing that to an error in the major release after that.

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

Hmm... I get no warnings on 1f89b73c4e.  Did you install all tools as
described in [2]?  The DTD needs to be installed as well.

[1] https://www.postgresql.org/message-id/BAF11F2D-5EDD-4DBB-87FA-4F35845029AE%40justatheory.com
[2] https://www.postgresql.org/docs/current/docguide-toolsets.html

-- 
Erik