Re: remaining sql/json patches

Hi Thom,

On Thu, May 16, 2024 at 8:50 AM Thom Brown <thom@linux.com> wrote:
> On Mon, 8 Apr 2024 at 10:09, Amit Langote <amitlangote09@gmail.com> wrote:
>>
>> On Mon, Apr 8, 2024 at 2:02 PM jian he <jian.universality@gmail.com> wrote:
>> > On Mon, Apr 8, 2024 at 11:21 AM jian he <jian.universality@gmail.com> wrote:
>> > >
>> > > On Mon, Apr 8, 2024 at 12:34 AM jian he <jian.universality@gmail.com> wrote:
>> > > >
>> > > > On Sun, Apr 7, 2024 at 9:36 PM Amit Langote <amitlangote09@gmail.com> wrote:
>> > > > > 0002 needs an expanded commit message but I've run out of energy today.
>> > > > >
>> > >
>> > > other than that, it looks good to me.
>> >
>> > one more tiny issue.
>> > +EXPLAIN (COSTS OFF, VERBOSE) SELECT * FROM jsonb_table_view1;
>> > +ERROR:  relation "jsonb_table_view1" does not exist
>> > +LINE 1: EXPLAIN (COSTS OFF, VERBOSE) SELECT * FROM jsonb_table_view1...
>> > +                                                   ^
>> > maybe you want
>> > EXPLAIN (COSTS OFF, VERBOSE) SELECT * FROM jsonb_table_view7;
>> > at the end of the sqljson_jsontable.sql.
>> > I guess it will be fine, but the format json explain's out is quite big.
>> >
>> > you also need to `drop table s cascade;` at the end of the test?
>>
>> Pushed after fixing this and other issues.  Thanks a lot for your
>> careful reviews.
>>
>> I've marked the CF entry for this as committed now:
>> https://commitfest.postgresql.org/47/4377/
>>
>> Let's work on the remaining PLAN clause with a new entry in the next
>> CF, possibly in a new email thread.
>
>
> I've just taken a look at the doc changes,

Thanks for taking a look.

> and I think we need to either remove the leading "select" keyword, or uppercase it in the examples.
>
> For example (on https://www.postgresql.org/docs/devel/functions-json.html#SQLJSON-QUERY-FUNCTIONS):
>
> json_exists ( context_item, path_expression [ PASSING { value AS varname } [, ...]] [ { TRUE | FALSE | UNKNOWN |
ERROR} ON ERROR ]) 
>
> Returns true if the SQL/JSON path_expression applied to the context_item using the PASSING values yields any items.
> The ON ERROR clause specifies the behavior if an error occurs; the default is to return the boolean FALSE value. Note
thatif the path_expression is strict and ON ERROR behavior is ERROR, an error is generated if it yields no items. 
> Examples:
> select json_exists(jsonb '{"key1": [1,2,3]}', 'strict $.key1[*] ? (@ > 2)') → t
> select json_exists(jsonb '{"a": [1,2,3]}', 'lax $.a[5]' ERROR ON ERROR) → f
> select json_exists(jsonb '{"a": [1,2,3]}', 'strict $.a[5]' ERROR ON ERROR) →
> ERROR:  jsonpath array subscript is out of bounds
>
> Examples are more difficult to read when keywords appear to be at the same level as predicates.  Plus other examples
withintables on the same page don't start with "select", and further down, block examples uppercase keywords.  Either
way,I don't like it as it is. 

I agree that the leading SELECT should be removed from these examples.
Also, the function names should be capitalized both in the syntax
description and in the examples, even though other functions appearing
on this page aren't.

> Separate from this, I think these tables don't scan well (see json_query for an example of what I'm referring to).
Thereis no clear separation of the syntax definition, the description, and the example. This is more a matter for the
websitemailing list, but I'm expressing it here to check whether others agree. 

Hmm, yes, I think I forgot to put <synopsis> around the syntax like
it's done for a few other functions listed on the page.

How about the attached?  Other than the above points, it removes the
<para> tags from the description text of each function to turn it into
a single paragraph, because the multi-paragraph style only seems to
appear in this table and it's looking a bit weird now.  Though it's
also true that the functions in this table have the longest
descriptions.

--
Thanks, Amit Langote