From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
---|---|
To: | jian he <jian(dot)universality(at)gmail(dot)com> |
Cc: | "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Doc Rework: Section 9.16.13 SQL/JSON Query Functions |
Date: | 2024-07-08 12:57:00 |
Message-ID: | CA+HiwqFCetbYuqvhJBw7sKWJkrkXTL7wnWHQvRjEOBD1vct8Xg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thanks for the readthrough.
On Sat, Jul 6, 2024 at 11:56 AM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
> json_exists
> "Returns true if the SQL/JSON path_expression applied to the
> context_item using the PASSING values yields any items."
> now you changed to
> <<
> Returns true if the SQL/JSON path_expression applied to the
> context_item. path_expression can reference variables named in the
> PASSING clause.
> <<
> Is it wrong?
Yes, I mistakenly dropped "doesn't yield any items."
> For both <literal>ON EMPTY</literal>and <literal>ON ERROR</literal>,
> specifying <literal>ERROR</literal> will cause an error to be
> thrown with
> the appropriate message. Other options include returning a SQL NULL, an
> need one more whitespace. should be:
> For both <literal>ON EMPTY</literal> and <literal>ON ERROR</literal>,
Fixed.
> Note that an error will be thrown if multiple values are returned and
> <literal>WITHOUT WRAPPER</literal> is specified.
> since not specify error on error, then no error will be thrown. maybe
> rephrase to
> It will be evaulated as error if multiple values are returned and
> <literal>WITHOUT WRAPPER</literal> is specified.
Done.
> <para>
> For both <literal>ON EMPTY</literal> and <literal>ON ERROR</literal>,
> specifying <literal>ERROR</literal> will cause an error to be
> thrown with
> the appropriate message. Other options include returning a SQL NULL, an
> empty array or object (array by default), or a user-specified expression
> that can be coerced to jsonb or the type specified in
> <literal>RETURNING</literal>.
> The default when <literal>ON EMPTY</literal> or <literal>ON
> ERROR</literal>
> is not specified is to return a SQL NULL value when the respective
> situation occurs.
> </para>
> in here, "empty array or object (array by default)",
> I don't think people can understand the meaning of "(array by default)" .
>
> "or a user-specified expression"
> maybe we can change to
> "or a user-specified
> <literal>DEFAULT</literal> <replaceable>expression</replaceable>"
> I think "user-specified expression" didn't have much link with
> "<literal>DEFAULT</literal> <replaceable>expression</replaceable>"
Yes, specifying each option's syntax in parenthesis makes sense.
> <replaceable>path_expression</replaceable> can reference variables named
> in the <literal>PASSING</literal> clause.
> do we need "The <replaceable>path_expression</replaceable>"?
Fixed.
> also maybe we can add
> + In <literal>PASSING</literal> clause, <replaceable>varname</replaceable> is
> + the variables name, <replaceable>value</replaceable> is the
> + variables' value.
Instead of expanding the description of the PASSING clause in each
function's description, I've moved its description to the top
paragraph with slightly different text.
> we can add a PASSING clause example from sqljson_queryfuncs.sql ,
> since all three functions can use it.
Done.
> JSON_VALUE:
> <<an error is thrown if that's not the case (though see the discussion
> of ON ERROR below).
> then
> << The ON ERROR and ON EMPTY clauses have similar semantics as
> mentioned in the description of JSON_QUERY, except the set of values
> returned in lieu of throwing an error is different.
>
> you first refer "below", then director to JSON_QUERY on error, on
> empty description.
> is the correct usage of "below"?
> "(though see the discussion of ON ERROR below)."
> i am not sure the meaning of "though" even watched this
> https://www.youtube.com/watch?v=r-LphuCKQ0Q
I've replaced the sentence with "(though see the discussion of ON
ERROR below)" with this:
"getting multiple values will be treated as an error."
No need to reference the ON ERROR clause with that wording.
> + <replaceable>context_item</replaceable> can be any character string that
> + can be succesfully cast to <type>jsonb</type>.
>
> typo: "succesfully", should be "successfully"
Fixed.
> maybe rephrase it to:
> + <replaceable>context_item</replaceable> can be jsonb type or any
> character string that
> + can be successfully cast to <type>jsonb</type>.
Done.
> + <literal>ON EMPTY</literal> expression (that is caused by empty result
> + of <replaceable>path_expression</replaceable>evaluation).
> need extra white space, should be
> + of <replaceable>path_expression</replaceable> evaluation).
Fixed.
> + The default when <literal>ON EMPTY</literal> or <literal>ON
> ERROR</literal>
> + is not specified is to return a SQL NULL value when the respective
> + situation occurs.
> Correct me if I'm wrong.
> we can just say:
> + The default when <literal>ON EMPTY</literal> or <literal>ON
> ERROR</literal>
> + is not specified is to return an SQL NULL value.
Agreed.
> another tiny issue.
>
> - <literal>select json_query(jsonb '{"a": "[1, 2]"}', 'lax $.a'
> OMIT QUOTES);</literal>
> + <literal>JSON_QUERY(jsonb '{"a": "[1, 2]"}', 'lax $.a' OMIT
> QUOTES);</literal>
> <returnvalue>[1, 2]</returnvalue>
> </para>
> <para>
> - <literal>select json_query(jsonb '{"a": "[1, 2]"}', 'lax $.a'
> RETURNING int[] OMIT QUOTES ERROR ON ERROR);</literal>
> + <literal>JSON_QUERY(jsonb '{"a": "[1, 2]"}', 'lax $.a'
> RETURNING int[] OMIT QUOTES ERROR ON ERROR);</literal>
>
> These two example queries don't need semicolons at the end?
Fixed.
Also, I've removed the following sentence in the description of
JSON_EXISTS, because a) it seems out of place
- <para>
- Note that if the <replaceable>path_expression</replaceable> is
- <literal>strict</literal> and <literal>ON ERROR</literal> behavior is
- <literal>ERROR</literal>, an error is generated if it yields no items.
- </para>
and b) does not seem correct:
SELECT JSON_EXISTS(jsonb '{"key1": [1,2,3]}', 'strict $.key1[*] ? (@ >
3)' ERROR ON ERROR);
json_exists
-------------
f
(1 row)
path_expression being strict or lax only matters inside
jsonpath_exec.c, not in ExecEvalJsonPathExpr().
Updated patch attached.
--
Thanks, Amit Langote
Attachment | Content-Type | Size |
---|---|---|
v4-0001-SQL-JSON-Various-improvements-to-SQL-JSON-query-f.patch | application/octet-stream | 21.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Langote | 2024-07-08 13:16:33 | Re: a potential typo in comments of pg_parse_json |
Previous Message | Andrei Lepikhov | 2024-07-08 12:45:15 | Re: MergeJoin beats HashJoin in the case of multiple hash clauses |