Re: remaining sql/json patches

From: Thom Brown <thom(at)linux(dot)com>
To: Amit Langote <amitlangote09(at)gmail(dot)com>
Cc: jian he <jian(dot)universality(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Himanshu Upadhyaya <upadhyaya(dot)himanshu(at)gmail(dot)com>, Erik Rijkers <er(at)xs4all(dot)nl>, Andres Freund <andres(at)anarazel(dot)de>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: remaining sql/json patches
Date: 2024-05-15 23:49:44
Message-ID: CAA-aLv7Dfy9BMrhUZ1skcg=OdqysWKzObS7XiDXdotJNF0E44Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, 8 Apr 2024 at 10:09, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:

> On Mon, Apr 8, 2024 at 2:02 PM jian he <jian(dot)universality(at)gmail(dot)com>
> wrote:
> > On Mon, Apr 8, 2024 at 11:21 AM jian he <jian(dot)universality(at)gmail(dot)com>
> wrote:
> > >
> > > On Mon, Apr 8, 2024 at 12:34 AM jian he <jian(dot)universality(at)gmail(dot)com>
> wrote:
> > > >
> > > > On Sun, Apr 7, 2024 at 9:36 PM Amit Langote <amitlangote09(at)gmail(dot)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, 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 that if 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 within tables 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.

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

Thom

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2024-05-16 00:02:57 Re: Statistics Import and Export
Previous Message Tom Lane 2024-05-15 23:32:05 Re: Why does pgindent's README say to download typedefs.list from the buildfarm?