Horton

Thread

Commits

  1. SQL/JSON: Prevent ON EMPTY for EXISTS columns in JSON_TABLE()

  2. SQL/JSON: Validate values in ON ERROR/EMPTY clauses

  3. SQL/JSON: Improve some error messages

  1. SQL/JSON query functions context_item doc entry and type requirement

    jian he <jian.universality@gmail.com> — 2024-06-03T15:11:01Z

    hi
based on gram.y and function transformJsonValueExpr.

gram.y:
| JSON_QUERY '('
json_value_expr ',' a_expr json_passing_clause_opt
json_returning_clause_opt
json_wrapper_behavior
json_quotes_clause_opt
json_behavior_clause_opt
')'

| JSON_EXISTS '('
json_value_expr ',' a_expr json_passing_clause_opt
json_on_error_clause_opt
')'

| JSON_VALUE '('
json_value_expr ',' a_expr json_passing_clause_opt
json_returning_clause_opt
json_behavior_clause_opt
')'

json_format_clause_opt contains:
| FORMAT_LA JSON
{
$$ = (Node *) makeJsonFormat(JS_FORMAT_JSON, JS_ENC_DEFAULT, @1);
}


That means, all the context_item can specify "FORMAT JSON" options,
in the meantime, do we need to update these functions
synopsis/signature in the doc?

some examples:
create table a(b jsonb);
create table a1(b int4range);
select json_value(b format json, 'strict $[*]' DEFAULT 9 ON ERROR) from a;
select json_value(b format json, 'strict $[*]' DEFAULT 9 ON ERROR) from a1;
select json_value(text '"1"' format json, 'strict $[*]' DEFAULT 9 ON ERROR);

------------------------------------------------
transformJsonValueExpr

/* Try to coerce to the target type. */
coerced = coerce_to_target_type(pstate, expr, exprtype,
targettype, -1,
COERCION_EXPLICIT,
COERCE_EXPLICIT_CAST,
location);

based on the function transformJsonValueExpr and subfunction
coerce_to_target_type,
for SQL/JSON query functions (JSON_EXISTS, JSON_QUERY, and JSON_VALUE)
the context_item requirement is any data type that not error out while
explicitly casting to jsonb in coerce_to_target_type.

I played around with it, I think these types can be used in context_item.
{char,text,bpchar,character varying } and these types of associated domains.
bytea data type too, but need specify "ENCODING UTF8".
e.g.
select json_value(bytea '"1"' format json ENCODING UTF8, 'strict $[*]'
DEFAULT 9 ON ERROR);


Maybe we can add some brief explanation in this para to explain more
about "context_item"
{
SQL/JSON functions JSON_EXISTS(), JSON_QUERY(), and JSON_VALUE()
described in Table 9.52 can be used to query JSON documents. Each of
these functions apply a path_expression (the query) to a context_item
(the document); see Section 9.16.2 for more details on what
path_expression can contain.
}

  2. Re: SQL/JSON query functions context_item doc entry and type requirement

    Amit Langote <amitlangote09@gmail.com> — 2024-06-17T06:43:28Z

    Hi,

On Tue, Jun 4, 2024 at 12:11 AM jian he <jian.universality@gmail.com> wrote:
>
> hi
> based on gram.y and function transformJsonValueExpr.
>
> gram.y:
> | JSON_QUERY '('
> json_value_expr ',' a_expr json_passing_clause_opt
> json_returning_clause_opt
> json_wrapper_behavior
> json_quotes_clause_opt
> json_behavior_clause_opt
> ')'
>
> | JSON_EXISTS '('
> json_value_expr ',' a_expr json_passing_clause_opt
> json_on_error_clause_opt
> ')'
>
> | JSON_VALUE '('
> json_value_expr ',' a_expr json_passing_clause_opt
> json_returning_clause_opt
> json_behavior_clause_opt
> ')'
>
> json_format_clause_opt contains:
> | FORMAT_LA JSON
> {
> $$ = (Node *) makeJsonFormat(JS_FORMAT_JSON, JS_ENC_DEFAULT, @1);
> }
>
>
> That means, all the context_item can specify "FORMAT JSON" options,
> in the meantime, do we need to update these functions
> synopsis/signature in the doc?
>
> some examples:
> create table a(b jsonb);
> create table a1(b int4range);
> select json_value(b format json, 'strict $[*]' DEFAULT 9 ON ERROR) from a;
> select json_value(b format json, 'strict $[*]' DEFAULT 9 ON ERROR) from a1;
> select json_value(text '"1"' format json, 'strict $[*]' DEFAULT 9 ON ERROR);
>
> ------------------------------------------------
> transformJsonValueExpr
>
> /* Try to coerce to the target type. */
> coerced = coerce_to_target_type(pstate, expr, exprtype,
> targettype, -1,
> COERCION_EXPLICIT,
> COERCE_EXPLICIT_CAST,
> location);
>
> based on the function transformJsonValueExpr and subfunction
> coerce_to_target_type,
> for SQL/JSON query functions (JSON_EXISTS, JSON_QUERY, and JSON_VALUE)
> the context_item requirement is any data type that not error out while
> explicitly casting to jsonb in coerce_to_target_type.
>
> I played around with it, I think these types can be used in context_item.
> {char,text,bpchar,character varying } and these types of associated domains.
> bytea data type too, but need specify "ENCODING UTF8".
> e.g.
> select json_value(bytea '"1"' format json ENCODING UTF8, 'strict $[*]'
> DEFAULT 9 ON ERROR);
>
>
> Maybe we can add some brief explanation in this para to explain more
> about "context_item"
> {
> SQL/JSON functions JSON_EXISTS(), JSON_QUERY(), and JSON_VALUE()
> described in Table 9.52 can be used to query JSON documents. Each of
> these functions apply a path_expression (the query) to a context_item
> (the document); see Section 9.16.2 for more details on what
> path_expression can contain.
> }

If I understand correctly, you're suggesting that we add a line to the
above paragraph to mention which types are appropriate for
context_item.  How about we add the following:

<replaceable>context_item</replaceable> expression can be a value of
any type that can be cast to <type>jsonb</type>. This includes types
such as <type>char</type>,  <type>text</type>, <type>bpchar</type>,
<type>character varying</type>, and <type>bytea</type> (with
<code>ENCODING UTF8</code>), as well as any domains over these types.

-- 
Thanks, Amit Langote

  3. Re: SQL/JSON query functions context_item doc entry and type requirement

    jian he <jian.universality@gmail.com> — 2024-06-17T09:47:37Z

    On Mon, Jun 17, 2024 at 2:43 PM Amit Langote <amitlangote09@gmail.com> wrote:
>
> Hi,
>
> On Tue, Jun 4, 2024 at 12:11 AM jian he <jian.universality@gmail.com> wrote:
> >
> > hi
> > based on gram.y and function transformJsonValueExpr.
> >
> > gram.y:
> > | JSON_QUERY '('
> > json_value_expr ',' a_expr json_passing_clause_opt
> > json_returning_clause_opt
> > json_wrapper_behavior
> > json_quotes_clause_opt
> > json_behavior_clause_opt
> > ')'
> >
> > | JSON_EXISTS '('
> > json_value_expr ',' a_expr json_passing_clause_opt
> > json_on_error_clause_opt
> > ')'
> >
> > | JSON_VALUE '('
> > json_value_expr ',' a_expr json_passing_clause_opt
> > json_returning_clause_opt
> > json_behavior_clause_opt
> > ')'
> >
> > json_format_clause_opt contains:
> > | FORMAT_LA JSON
> > {
> > $$ = (Node *) makeJsonFormat(JS_FORMAT_JSON, JS_ENC_DEFAULT, @1);
> > }
> >
> >
> > That means, all the context_item can specify "FORMAT JSON" options,
> > in the meantime, do we need to update these functions
> > synopsis/signature in the doc?
> >

>
> If I understand correctly, you're suggesting that we add a line to the
> above paragraph to mention which types are appropriate for
> context_item.  How about we add the following:
>
> <replaceable>context_item</replaceable> expression can be a value of
> any type that can be cast to <type>jsonb</type>. This includes types
> such as <type>char</type>,  <type>text</type>, <type>bpchar</type>,
> <type>character varying</type>, and <type>bytea</type> (with
> <code>ENCODING UTF8</code>), as well as any domains over these types.

your wording looks ok to me. I want to add two sentences. so it becomes:

+   The <replaceable>context_item</replaceable> expression can be a value of
+    any type that can be cast to <type>jsonb</type>. This includes types
+   such as <type>char</type>,  <type>text</type>, <type>bpchar</type>,
+    <type>character varying</type>, and <type>bytea</type> (with
+    <code>ENCODING UTF8</code>), as well as any domains over these types.
+    The <replaceable>context_item</replaceable> expression can also
be followed with
+    <literal>FORMAT JSON</literal>, <literal>ENCODING UTF8</literal>.
+    These two options currently don't have actual meaning.
+    <literal>ENCODING UTF8</literal> can only be specified when
<replaceable>context_item</replaceable> type is <type>bytea</type>.

imho, "These two options currently don't have actual meaning." is accurate,
but still does not explain why we allow "FORMAT JSON ENCODING UTF8".
I think we may need an explanation for  "FORMAT JSON ENCODING UTF8".
because json_array, json_object, json_serialize, json all didn't
mention the meaning of "[ FORMAT JSON [ ENCODING UTF8 ] ] ".


I added "[ FORMAT JSON [ ENCODING UTF8 ] ] " to the function
signature/synopsis of json_exists, json_query, json_value.

  4. Re: SQL/JSON query functions context_item doc entry and type requirement

    Chapman Flack <jcflack@acm.org> — 2024-06-17T13:05:42Z

    Hi,

On 06/17/24 02:43, Amit Langote wrote:
> <replaceable>context_item</replaceable> expression can be a value of
> any type that can be cast to <type>jsonb</type>. This includes types
> such as <type>char</type>,  <type>text</type>, <type>bpchar</type>,
> <type>character varying</type>, and <type>bytea</type> (with
> <code>ENCODING UTF8</code>), as well as any domains over these types.

Reading this message in conjunction with [0] makes me think that we are
really talking about a function that takes a first parameter of type jsonb,
and behaves exactly that way (so any cast required is applied by the system
ahead of the call). Under those conditions, this seems like an unusual
sentence to add in the docs, at least until we have also documented that
tan's argument can be of any type that can be cast to double precision.

On the other hand, if the behavior of the functions were to be changed
(perhaps using prosupport rewriting as suggested in [1]?) so that it was
not purely describable as a function accepting exactly jsonb with a
possible system-applied cast in front, then in that case such an added
explanation in the docs might be very fitting.

Regards,
-Chap


[0]
https://www.postgresql.org/message-id/CA%2BHiwqGuqLfAEP-FwW3QHByfQOoUpyj6YZG6R6bScpQswvNYDA%40mail.gmail.com
[1] https://www.postgresql.org/message-id/66703054.6040109%40acm.org

  5. Re: SQL/JSON query functions context_item doc entry and type requirement

    jian he <jian.universality@gmail.com> — 2024-06-19T15:29:39Z

    On Mon, Jun 17, 2024 at 9:05 PM Chapman Flack <jcflack@acm.org> wrote:
>
> Hi,
>
> On 06/17/24 02:43, Amit Langote wrote:
> > <replaceable>context_item</replaceable> expression can be a value of
> > any type that can be cast to <type>jsonb</type>. This includes types
> > such as <type>char</type>,  <type>text</type>, <type>bpchar</type>,
> > <type>character varying</type>, and <type>bytea</type> (with
> > <code>ENCODING UTF8</code>), as well as any domains over these types.
>
> Reading this message in conjunction with [0] makes me think that we are
> really talking about a function that takes a first parameter of type jsonb,
> and behaves exactly that way (so any cast required is applied by the system
> ahead of the call). Under those conditions, this seems like an unusual
> sentence to add in the docs, at least until we have also documented that
> tan's argument can be of any type that can be cast to double precision.
>

I guess it would be fine to add an unusual sentence to the docs.

imagine a function: array_avg(anyarray) returns anyelement.
array_avg calculate an array's elements's avg. like
array('{1,2,3}'::int[]) returns 2.
but array_avg won't make sense if the input argument is a date array.
so mentioning in the doc: array_avg can accept anyarray, but anyarray
cannot date array.
seems ok.


> On the other hand, if the behavior of the functions were to be changed
> (perhaps using prosupport rewriting as suggested in [1]?) so that it was
> not purely describable as a function accepting exactly jsonb with a
> possible system-applied cast in front, then in that case such an added
> explanation in the docs might be very fitting.
>

prosupport won't work, I think.
because json_exists, json_value, json_query, json_table don't have
pg_proc entries.
These are more like expressions.

  6. Re: SQL/JSON query functions context_item doc entry and type requirement

    David G. Johnston <david.g.johnston@gmail.com> — 2024-06-19T16:03:15Z

    On Wed, Jun 19, 2024 at 8:29 AM jian he <jian.universality@gmail.com> wrote:

> On Mon, Jun 17, 2024 at 9:05 PM Chapman Flack <jcflack@acm.org> wrote:
> >
> > Hi,
> >
> > On 06/17/24 02:43, Amit Langote wrote:
> > > <replaceable>context_item</replaceable> expression can be a value of
> > > any type that can be cast to <type>jsonb</type>. This includes types
> > > such as <type>char</type>,  <type>text</type>, <type>bpchar</type>,
> > > <type>character varying</type>, and <type>bytea</type> (with
> > > <code>ENCODING UTF8</code>), as well as any domains over these types.
> >
> > Reading this message in conjunction with [0] makes me think that we are
> > really talking about a function that takes a first parameter of type
> jsonb,
> > and behaves exactly that way (so any cast required is applied by the
> system
> > ahead of the call). Under those conditions, this seems like an unusual
> > sentence to add in the docs, at least until we have also documented that
> > tan's argument can be of any type that can be cast to double precision.
> >
>
> I guess it would be fine to add an unusual sentence to the docs.
>
> imagine a function: array_avg(anyarray) returns anyelement.
> array_avg calculate an array's elements's avg. like
> array('{1,2,3}'::int[]) returns 2.
> but array_avg won't make sense if the input argument is a date array.
> so mentioning in the doc: array_avg can accept anyarray, but anyarray
> cannot date array.
> seems ok.
>

There is existing wording for this:

"The expression can be of any JSON type, any character string type, or
bytea in UTF8 encoding."

If you add this sentence to the paragraph the link that already exists,
which simply points the reader to this sentence, becomes redundant and
should be removed.

As for table 9.16.3 - it is unwieldy already.  Lets try and make the core
syntax shorter, not longer.  We already have precedence in the subsequent
json_table section - give each major clause item a name then below the
table define the syntax and meaning for those names.  Unlike in that
section - which probably should be modified too - context_item should have
its own description line.

David J.

  7. Re: SQL/JSON query functions context_item doc entry and type requirement

    Amit Langote <amitlangote09@gmail.com> — 2024-06-20T09:46:37Z

    On Thu, Jun 20, 2024 at 1:03 AM David G. Johnston
<david.g.johnston@gmail.com> wrote:
> On Wed, Jun 19, 2024 at 8:29 AM jian he <jian.universality@gmail.com> wrote:
>>
>> On Mon, Jun 17, 2024 at 9:05 PM Chapman Flack <jcflack@acm.org> wrote:
>> >
>> > Hi,
>> >
>> > On 06/17/24 02:43, Amit Langote wrote:
>> > > <replaceable>context_item</replaceable> expression can be a value of
>> > > any type that can be cast to <type>jsonb</type>. This includes types
>> > > such as <type>char</type>,  <type>text</type>, <type>bpchar</type>,
>> > > <type>character varying</type>, and <type>bytea</type> (with
>> > > <code>ENCODING UTF8</code>), as well as any domains over these types.
>> >
>> > Reading this message in conjunction with [0] makes me think that we are
>> > really talking about a function that takes a first parameter of type jsonb,
>> > and behaves exactly that way (so any cast required is applied by the system
>> > ahead of the call). Under those conditions, this seems like an unusual
>> > sentence to add in the docs, at least until we have also documented that
>> > tan's argument can be of any type that can be cast to double precision.
>> >
>>
>> I guess it would be fine to add an unusual sentence to the docs.
>>
>> imagine a function: array_avg(anyarray) returns anyelement.
>> array_avg calculate an array's elements's avg. like
>> array('{1,2,3}'::int[]) returns 2.
>> but array_avg won't make sense if the input argument is a date array.
>> so mentioning in the doc: array_avg can accept anyarray, but anyarray
>> cannot date array.
>> seems ok.
>
>
> There is existing wording for this:
>
> "The expression can be of any JSON type, any character string type, or bytea in UTF8 encoding."
>
> If you add this sentence to the paragraph the link that already exists, which simply points the reader to this sentence, becomes redundant and should be removed.

I've just posted a patch in the other thread [1] to restrict
context_item to be of jsonb type, which users would need to ensure by
adding an explicit cast if needed.  I think that makes this
clarification unnecessary.

> As for table 9.16.3 - it is unwieldy already.  Lets try and make the core syntax shorter, not longer.  We already have precedence in the subsequent json_table section - give each major clause item a name then below the table define the syntax and meaning for those names.  Unlike in that section - which probably should be modified too - context_item should have its own description line.

I had posted a patch a little while ago at [1] to render the syntax a
bit differently with each function getting its own syntax synopsis.
Resending it here; have addressed Jian He's comments.

--
Thanks, Amit Langote
[1] https://www.postgresql.org/message-id/CA%2BHiwqF2Z6FATWQV6bG9NeKYf%3D%2B%2BfOgmdbYc9gWSNJ81jfqCuA%40mail.gmail.com

  8. Re: SQL/JSON query functions context_item doc entry and type requirement

    jian he <jian.universality@gmail.com> — 2024-06-20T16:01:11Z

    On Thu, Jun 20, 2024 at 5:46 PM Amit Langote <amitlangote09@gmail.com> wrote:
>
> On Thu, Jun 20, 2024 at 1:03 AM David G. Johnston
> <david.g.johnston@gmail.com> wrote:
> > On Wed, Jun 19, 2024 at 8:29 AM jian he <jian.universality@gmail.com> wrote:
> >>
> >> On Mon, Jun 17, 2024 at 9:05 PM Chapman Flack <jcflack@acm.org> wrote:
> >> >
> >> > Hi,
> >> >
> >> > On 06/17/24 02:43, Amit Langote wrote:
> >> > > <replaceable>context_item</replaceable> expression can be a value of
> >> > > any type that can be cast to <type>jsonb</type>. This includes types
> >> > > such as <type>char</type>,  <type>text</type>, <type>bpchar</type>,
> >> > > <type>character varying</type>, and <type>bytea</type> (with
> >> > > <code>ENCODING UTF8</code>), as well as any domains over these types.
> >> >
> >> > Reading this message in conjunction with [0] makes me think that we are
> >> > really talking about a function that takes a first parameter of type jsonb,
> >> > and behaves exactly that way (so any cast required is applied by the system
> >> > ahead of the call). Under those conditions, this seems like an unusual
> >> > sentence to add in the docs, at least until we have also documented that
> >> > tan's argument can be of any type that can be cast to double precision.
> >> >
> >>
> >> I guess it would be fine to add an unusual sentence to the docs.
> >>
> >> imagine a function: array_avg(anyarray) returns anyelement.
> >> array_avg calculate an array's elements's avg. like
> >> array('{1,2,3}'::int[]) returns 2.
> >> but array_avg won't make sense if the input argument is a date array.
> >> so mentioning in the doc: array_avg can accept anyarray, but anyarray
> >> cannot date array.
> >> seems ok.
> >
> >
> > There is existing wording for this:
> >
> > "The expression can be of any JSON type, any character string type, or bytea in UTF8 encoding."
> >
> > If you add this sentence to the paragraph the link that already exists, which simply points the reader to this sentence, becomes redundant and should be removed.
>
> I've just posted a patch in the other thread [1] to restrict
> context_item to be of jsonb type, which users would need to ensure by
> adding an explicit cast if needed.  I think that makes this
> clarification unnecessary.
>
> > As for table 9.16.3 - it is unwieldy already.  Lets try and make the core syntax shorter, not longer.  We already have precedence in the subsequent json_table section - give each major clause item a name then below the table define the syntax and meaning for those names.  Unlike in that section - which probably should be modified too - context_item should have its own description line.
>
> I had posted a patch a little while ago at [1] to render the syntax a
> bit differently with each function getting its own syntax synopsis.
> Resending it here; have addressed Jian He's comments.
>
> --

@@ -18746,6 +18752,7 @@ ERROR:  jsonpath array subscript is out of bounds
         <literal>PASSING</literal> <replaceable>value</replaceable>s.
        </para>
        <para>
+        Returns the result of applying the SQL/JSON
         If the path expression returns multiple SQL/JSON items, it might be
         necessary to wrap the result using the <literal>WITH WRAPPER</literal>
         clause to make it a valid JSON string.  If the wrapper is


+        Returns the result of applying the SQL/JSON
 is redundant?


playing around with it.
found some minor issues:

json_exists allow:  DEFAULT expression ON ERROR, which is not
mentioned in the doc.
for example:
select JSON_EXISTS(jsonb '{"a": [1,2,3]}', 'strict $.a[5]' default
true ON ERROR);
select JSON_EXISTS(jsonb '{"a": [1,2,3]}', 'strict $.a[5]' default 0 ON ERROR);
select JSON_EXISTS(jsonb '{"a": [1,2,3]}', 'strict $.a[5]' default 11 ON ERROR);



JSON_VALUE on error, on empty semantics should be the same as json_query.
like:
[ { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression }
ON EMPTY ]
[ { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression }
ON ERROR ])

examples:
select JSON_value(jsonb '[]' , '$'  empty array on error);
select JSON_value(jsonb '[]' , '$'  empty object on error);

  9. Re: SQL/JSON query functions context_item doc entry and type requirement

    David G. Johnston <david.g.johnston@gmail.com> — 2024-06-21T00:46:44Z

    On Thu, Jun 20, 2024 at 9:01 AM jian he <jian.universality@gmail.com> wrote:

> On Thu, Jun 20, 2024 at 5:46 PM Amit Langote <amitlangote09@gmail.com>
> wrote:
> >
> > On Thu, Jun 20, 2024 at 1:03 AM David G. Johnston
> > <david.g.johnston@gmail.com> wrote:
> > > On Wed, Jun 19, 2024 at 8:29 AM jian he <jian.universality@gmail.com>
> wrote:
> > >>
> > >> On Mon, Jun 17, 2024 at 9:05 PM Chapman Flack <jcflack@acm.org>
> wrote:
> > >> >
> > >> > Hi,
> > >> >
> > >> > On 06/17/24 02:43, Amit Langote wrote:
> > >> > > <replaceable>context_item</replaceable> expression can be a value
> of
> > >> > > any type that can be cast to <type>jsonb</type>. This includes
> types
> > >> > > such as <type>char</type>,  <type>text</type>,
> <type>bpchar</type>,
> > >> > > <type>character varying</type>, and <type>bytea</type> (with
> > >> > > <code>ENCODING UTF8</code>), as well as any domains over these
> types.
> > >> >
> > >> > Reading this message in conjunction with [0] makes me think that we
> are
> > >> > really talking about a function that takes a first parameter of
> type jsonb,
> > >> > and behaves exactly that way (so any cast required is applied by
> the system
> > >> > ahead of the call). Under those conditions, this seems like an
> unusual
> > >> > sentence to add in the docs, at least until we have also documented
> that
> > >> > tan's argument can be of any type that can be cast to double
> precision.
> > >> >
> > >>
> > >> I guess it would be fine to add an unusual sentence to the docs.
> > >>
> > >> imagine a function: array_avg(anyarray) returns anyelement.
> > >> array_avg calculate an array's elements's avg. like
> > >> array('{1,2,3}'::int[]) returns 2.
> > >> but array_avg won't make sense if the input argument is a date array.
> > >> so mentioning in the doc: array_avg can accept anyarray, but anyarray
> > >> cannot date array.
> > >> seems ok.
> > >
> > >
> > > There is existing wording for this:
> > >
> > > "The expression can be of any JSON type, any character string type, or
> bytea in UTF8 encoding."
> > >
> > > If you add this sentence to the paragraph the link that already
> exists, which simply points the reader to this sentence, becomes redundant
> and should be removed.
> >
> > I've just posted a patch in the other thread [1] to restrict
> > context_item to be of jsonb type, which users would need to ensure by
> > adding an explicit cast if needed.  I think that makes this
> > clarification unnecessary.
> >
> > > As for table 9.16.3 - it is unwieldy already.  Lets try and make the
> core syntax shorter, not longer.  We already have precedence in the
> subsequent json_table section - give each major clause item a name then
> below the table define the syntax and meaning for those names.  Unlike in
> that section - which probably should be modified too - context_item should
> have its own description line.
> >
> > I had posted a patch a little while ago at [1] to render the syntax a
> > bit differently with each function getting its own syntax synopsis.
> > Resending it here; have addressed Jian He's comments.
> >
> > --
>

I was thinking more like:

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index c324906b22..b9d157663a 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18692,8 +18692,10 @@ $.* ? (@ like_regex "^\\d+$")
       <entry role="func_table_entry"><para role="func_signature">
         <indexterm><primary>json_exists</primary></indexterm>
         <function>json_exists</function> (
-        <replaceable>context_item</replaceable>,
<replaceable>path_expression</replaceable> <optional>
<literal>PASSING</literal> { <replaceable>value</replaceable>
<literal>AS</literal> <replaceable>varname</replaceable> } <optional>,
...</optional></optional>
-        <optional> { <literal>TRUE</literal> | <literal>FALSE</literal>
|<literal> UNKNOWN</literal> | <literal>ERROR</literal> } <literal>ON
ERROR</literal> </optional>)
+        <replaceable>context_item</replaceable>,
+        <replaceable>path_expression</replaceable>
+        <optional>variable_definitions</optional>
+        <optional>on_error_boolean</optional>)
        </para>
        <para>
         Returns true if the SQL/JSON
<replaceable>path_expression</replaceable>
@@ -18732,12 +18734,14 @@ ERROR:  jsonpath array subscript is out of bounds
       <entry role="func_table_entry"><para role="func_signature">
         <indexterm><primary>json_query</primary></indexterm>
         <function>json_query</function> (
-        <replaceable>context_item</replaceable>,
<replaceable>path_expression</replaceable> <optional>
<literal>PASSING</literal> { <replaceable>value</replaceable>
<literal>AS</literal> <replaceable>varname</replaceable> } <optional>,
...</optional></optional>
-        <optional> <literal>RETURNING</literal>
<replaceable>data_type</replaceable> <optional> <literal>FORMAT
JSON</literal> <optional> <literal>ENCODING UTF8</literal> </optional>
</optional> </optional>
-        <optional> { <literal>WITHOUT</literal> | <literal>WITH</literal>
{ <literal>CONDITIONAL</literal> |
<optional><literal>UNCONDITIONAL</literal></optional> } } <optional>
<literal>ARRAY</literal> </optional> <literal>WRAPPER</literal> </optional>
-        <optional> { <literal>KEEP</literal> | <literal>OMIT</literal> }
<literal>QUOTES</literal> <optional> <literal>ON SCALAR STRING</literal>
</optional> </optional>
-        <optional> { <literal>ERROR</literal> | <literal>NULL</literal> |
<literal>EMPTY</literal> { <optional> <literal>ARRAY</literal> </optional>
| <literal>OBJECT</literal> } | <literal>DEFAULT</literal>
<replaceable>expression</replaceable> } <literal>ON EMPTY</literal>
</optional>
-        <optional> { <literal>ERROR</literal> | <literal>NULL</literal> |
<literal>EMPTY</literal> { <optional> <literal>ARRAY</literal> </optional>
| <literal>OBJECT</literal> } | <literal>DEFAULT</literal>
<replaceable>expression</replaceable> } <literal>ON ERROR</literal>
</optional>)
+        <replaceable>context_item</replaceable>,
+        <replaceable>path_expression</replaceable>
+        <optional>variable_definitions</optional>
+        <optional>return_clause</optional>
+        <optional>wrapping_clause</optional>
+        <optional>quoting_clause</optional>
+        <optional>on_empty_set</optional>
+        <optional>on_error_set</optional>)
       </para>
        <para>
         Returns the result of applying the SQL/JSON
@@ -18809,11 +18813,12 @@ DETAIL:  Missing "]" after array dimensions.
       <entry role="func_table_entry"><para role="func_signature">
         <indexterm><primary>json_value</primary></indexterm>
         <function>json_value</function> (
-        <replaceable>context_item</replaceable>,
<replaceable>path_expression</replaceable>
-        <optional> <literal>PASSING</literal> {
<replaceable>value</replaceable> <literal>AS</literal>
<replaceable>varname</replaceable> } <optional>, ...</optional></optional>
-        <optional> <literal>RETURNING</literal>
<replaceable>data_type</replaceable> </optional>
-        <optional> { <literal>ERROR</literal> | <literal>NULL</literal> |
<literal>DEFAULT</literal> <replaceable>expression</replaceable> }
<literal>ON EMPTY</literal> </optional>
-        <optional> { <literal>ERROR</literal> | <literal>NULL</literal> |
<literal>DEFAULT</literal> <replaceable>expression</replaceable> }
<literal>ON ERROR</literal> </optional>)
+        <replaceable>context_item</replaceable>,
+        <replaceable>path_expression</replaceable>
+        <optional>variable_definitions</optional>
+        <optional>return_type</optional>
+        <optional>on_empty_value</optional>
+        <optional>on_error_value</optional>)
        </para>
        <para>
         Returns the result of applying the SQL/JSON

Then defining each of those below the table - keeping the on_error variants
together.




> playing around with it.
> found some minor issues:
>
> json_exists allow:  DEFAULT expression ON ERROR, which is not
> mentioned in the doc.
> for example:
> select JSON_EXISTS(jsonb '{"a": [1,2,3]}', 'strict $.a[5]' default
> true ON ERROR);
> select JSON_EXISTS(jsonb '{"a": [1,2,3]}', 'strict $.a[5]' default 0 ON
> ERROR);
> select JSON_EXISTS(jsonb '{"a": [1,2,3]}', 'strict $.a[5]' default 11 ON
> ERROR);
>

Yeah, surprised it works, the documented behavior seems logical.  Being
able to return a non-boolean here seems odd.  Especially since it is cast
to boolean on output.


> JSON_VALUE on error, on empty semantics should be the same as json_query.
> like:
> [ { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression }
> ON EMPTY ]
> [ { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression }
> ON ERROR ])
>
> examples:
> select JSON_value(jsonb '[]' , '$'  empty array on error);
> select JSON_value(jsonb '[]' , '$'  empty object on error);
>

Again the documented behavior seems to make sense though and the ability to
specify empty in the value function seems like a bug.  If you really want
an empty array or object you do have access to default.  The reason
json_query provides for an empty array/object is that it is already
expecting to produce an array (object seems a little odd).

I agree our docs and code do not match which needs to be fixed, ideally in
the direction of the standard which I'm guessing our documentation is based
off of.  But let's not go off of my guess.

David J.

  10. Re: SQL/JSON query functions context_item doc entry and type requirement

    Amit Langote <amitlangote09@gmail.com> — 2024-06-21T12:18:15Z

    On Fri, Jun 21, 2024 at 9:47 AM David G. Johnston
<david.g.johnston@gmail.com> wrote:
> On Thu, Jun 20, 2024 at 9:01 AM jian he <jian.universality@gmail.com> wrote:
>>
>> On Thu, Jun 20, 2024 at 5:46 PM Amit Langote <amitlangote09@gmail.com> wrote:
>> >
>> > On Thu, Jun 20, 2024 at 1:03 AM David G. Johnston
>> > <david.g.johnston@gmail.com> wrote:
>> > > On Wed, Jun 19, 2024 at 8:29 AM jian he <jian.universality@gmail.com> wrote:
>> > >>
>> > >> On Mon, Jun 17, 2024 at 9:05 PM Chapman Flack <jcflack@acm.org> wrote:
>> > >> >
>> > >> > Hi,
>> > >> >
>> > >> > On 06/17/24 02:43, Amit Langote wrote:
>> > >> > > <replaceable>context_item</replaceable> expression can be a value of
>> > >> > > any type that can be cast to <type>jsonb</type>. This includes types
>> > >> > > such as <type>char</type>,  <type>text</type>, <type>bpchar</type>,
>> > >> > > <type>character varying</type>, and <type>bytea</type> (with
>> > >> > > <code>ENCODING UTF8</code>), as well as any domains over these types.
>> > >> >
>> > >> > Reading this message in conjunction with [0] makes me think that we are
>> > >> > really talking about a function that takes a first parameter of type jsonb,
>> > >> > and behaves exactly that way (so any cast required is applied by the system
>> > >> > ahead of the call). Under those conditions, this seems like an unusual
>> > >> > sentence to add in the docs, at least until we have also documented that
>> > >> > tan's argument can be of any type that can be cast to double precision.
>> > >> >
>> > >>
>> > >> I guess it would be fine to add an unusual sentence to the docs.
>> > >>
>> > >> imagine a function: array_avg(anyarray) returns anyelement.
>> > >> array_avg calculate an array's elements's avg. like
>> > >> array('{1,2,3}'::int[]) returns 2.
>> > >> but array_avg won't make sense if the input argument is a date array.
>> > >> so mentioning in the doc: array_avg can accept anyarray, but anyarray
>> > >> cannot date array.
>> > >> seems ok.
>> > >
>> > >
>> > > There is existing wording for this:
>> > >
>> > > "The expression can be of any JSON type, any character string type, or bytea in UTF8 encoding."
>> > >
>> > > If you add this sentence to the paragraph the link that already exists, which simply points the reader to this sentence, becomes redundant and should be removed.
>> >
>> > I've just posted a patch in the other thread [1] to restrict
>> > context_item to be of jsonb type, which users would need to ensure by
>> > adding an explicit cast if needed.  I think that makes this
>> > clarification unnecessary.
>> >
>> > > As for table 9.16.3 - it is unwieldy already.  Lets try and make the core syntax shorter, not longer.  We already have precedence in the subsequent json_table section - give each major clause item a name then below the table define the syntax and meaning for those names.  Unlike in that section - which probably should be modified too - context_item should have its own description line.
>> >
>> > I had posted a patch a little while ago at [1] to render the syntax a
>> > bit differently with each function getting its own syntax synopsis.
>> > Resending it here; have addressed Jian He's comments.
>> >
>> > --
>
>
> I was thinking more like:
>
> diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
> index c324906b22..b9d157663a 100644
> --- a/doc/src/sgml/func.sgml
> +++ b/doc/src/sgml/func.sgml
> @@ -18692,8 +18692,10 @@ $.* ? (@ like_regex "^\\d+$")
>        <entry role="func_table_entry"><para role="func_signature">
>          <indexterm><primary>json_exists</primary></indexterm>
>          <function>json_exists</function> (
> -        <replaceable>context_item</replaceable>, <replaceable>path_expression</replaceable> <optional> <literal>PASSING</literal> { <replaceable>value</replaceable> <literal>AS</literal> <replaceable>varname</replaceable> } <optional>, ...</optional></optional>
> -        <optional> { <literal>TRUE</literal> | <literal>FALSE</literal> |<literal> UNKNOWN</literal> | <literal>ERROR</literal> } <literal>ON ERROR</literal> </optional>)
> +        <replaceable>context_item</replaceable>,
> +        <replaceable>path_expression</replaceable>
> +        <optional>variable_definitions</optional>
> +        <optional>on_error_boolean</optional>)
>         </para>
>         <para>
>          Returns true if the SQL/JSON <replaceable>path_expression</replaceable>
> @@ -18732,12 +18734,14 @@ ERROR:  jsonpath array subscript is out of bounds
>        <entry role="func_table_entry"><para role="func_signature">
>          <indexterm><primary>json_query</primary></indexterm>
>          <function>json_query</function> (
> -        <replaceable>context_item</replaceable>, <replaceable>path_expression</replaceable> <optional> <literal>PASSING</literal> { <replaceable>value</replaceable> <literal>AS</literal> <replaceable>varname</replaceable> } <optional>, ...</optional></optional>
> -        <optional> <literal>RETURNING</literal> <replaceable>data_type</replaceable> <optional> <literal>FORMAT JSON</literal> <optional> <literal>ENCODING UTF8</literal> </optional> </optional> </optional>
> -        <optional> { <literal>WITHOUT</literal> | <literal>WITH</literal> { <literal>CONDITIONAL</literal> | <optional><literal>UNCONDITIONAL</literal></optional> } } <optional> <literal>ARRAY</literal> </optional> <literal>WRAPPER</literal> </optional>
> -        <optional> { <literal>KEEP</literal> | <literal>OMIT</literal> } <literal>QUOTES</literal> <optional> <literal>ON SCALAR STRING</literal> </optional> </optional>
> -        <optional> { <literal>ERROR</literal> | <literal>NULL</literal> | <literal>EMPTY</literal> { <optional> <literal>ARRAY</literal> </optional> | <literal>OBJECT</literal> } | <literal>DEFAULT</literal> <replaceable>expression</replaceable> } <literal>ON EMPTY</literal> </optional>
> -        <optional> { <literal>ERROR</literal> | <literal>NULL</literal> | <literal>EMPTY</literal> { <optional> <literal>ARRAY</literal> </optional> | <literal>OBJECT</literal> } | <literal>DEFAULT</literal> <replaceable>expression</replaceable> } <literal>ON ERROR</literal> </optional>)
> +        <replaceable>context_item</replaceable>,
> +        <replaceable>path_expression</replaceable>
> +        <optional>variable_definitions</optional>
> +        <optional>return_clause</optional>
> +        <optional>wrapping_clause</optional>
> +        <optional>quoting_clause</optional>
> +        <optional>on_empty_set</optional>
> +        <optional>on_error_set</optional>)
>        </para>
>         <para>
>          Returns the result of applying the SQL/JSON
> @@ -18809,11 +18813,12 @@ DETAIL:  Missing "]" after array dimensions.
>        <entry role="func_table_entry"><para role="func_signature">
>          <indexterm><primary>json_value</primary></indexterm>
>          <function>json_value</function> (
> -        <replaceable>context_item</replaceable>, <replaceable>path_expression</replaceable>
> -        <optional> <literal>PASSING</literal> { <replaceable>value</replaceable> <literal>AS</literal> <replaceable>varname</replaceable> } <optional>, ...</optional></optional>
> -        <optional> <literal>RETURNING</literal> <replaceable>data_type</replaceable> </optional>
> -        <optional> { <literal>ERROR</literal> | <literal>NULL</literal> | <literal>DEFAULT</literal> <replaceable>expression</replaceable> } <literal>ON EMPTY</literal> </optional>
> -        <optional> { <literal>ERROR</literal> | <literal>NULL</literal> | <literal>DEFAULT</literal> <replaceable>expression</replaceable> } <literal>ON ERROR</literal> </optional>)
> +        <replaceable>context_item</replaceable>,
> +        <replaceable>path_expression</replaceable>
> +        <optional>variable_definitions</optional>
> +        <optional>return_type</optional>
> +        <optional>on_empty_value</optional>
> +        <optional>on_error_value</optional>)
>         </para>
>         <para>
>          Returns the result of applying the SQL/JSON
>
> Then defining each of those below the table - keeping the on_error variants together.

That sounds appealing. I'll try to come up with a patch unless you or
anyone else wants to take a stab at it.

>> playing around with it.
>> found some minor issues:
>>
>> json_exists allow:  DEFAULT expression ON ERROR, which is not
>> mentioned in the doc.
>> for example:
>> select JSON_EXISTS(jsonb '{"a": [1,2,3]}', 'strict $.a[5]' default
>> true ON ERROR);
>> select JSON_EXISTS(jsonb '{"a": [1,2,3]}', 'strict $.a[5]' default 0 ON ERROR);
>> select JSON_EXISTS(jsonb '{"a": [1,2,3]}', 'strict $.a[5]' default 11 ON ERROR);
>
>
> Yeah, surprised it works, the documented behavior seems logical.  Being able to return a non-boolean here seems odd.  Especially since it is cast to boolean on output.
>
>>
>> JSON_VALUE on error, on empty semantics should be the same as json_query.
>> like:
>> [ { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression }
>> ON EMPTY ]
>> [ { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression }
>> ON ERROR ])
>>
>> examples:
>> select JSON_value(jsonb '[]' , '$'  empty array on error);
>> select JSON_value(jsonb '[]' , '$'  empty object on error);
>
>
> Again the documented behavior seems to make sense though and the ability to specify empty in the value function seems like a bug.  If you really want an empty array or object you do have access to default.  The reason json_query provides for an empty array/object is that it is already expecting to produce an array (object seems a little odd).
>
> I agree our docs and code do not match which needs to be fixed, ideally in the direction of the standard which I'm guessing our documentation is based off of.  But let's not go off of my guess.

Oops, that is indeed not great and, yes, the problem is code not
matching the documentation, the latter of which is "correct".

Basically, the grammar allows specifying any of the all possible ON
ERROR/EMPTY behavior values irrespective of the function, so parse
analysis should be catching and flagging an attempt to specify
incompatible value for a given function, which it does not.

I've attached a patch to fix that, which I'd like to push before
anything else we've been discussing.

-- 
Thanks, Amit Langote

  11. Re: SQL/JSON query functions context_item doc entry and type requirement

    Amit Langote <amitlangote09@gmail.com> — 2024-06-22T08:31:50Z

    On Fri, Jun 21, 2024 at 9:18 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Fri, Jun 21, 2024 at 9:47 AM David G. Johnston
> <david.g.johnston@gmail.com> wrote:
> > On Thu, Jun 20, 2024 at 9:01 AM jian he <jian.universality@gmail.com> wrote:
> >> playing around with it.
> >> found some minor issues:
> >>
> >> json_exists allow:  DEFAULT expression ON ERROR, which is not
> >> mentioned in the doc.
> >> for example:
> >> select JSON_EXISTS(jsonb '{"a": [1,2,3]}', 'strict $.a[5]' default
> >> true ON ERROR);
> >> select JSON_EXISTS(jsonb '{"a": [1,2,3]}', 'strict $.a[5]' default 0 ON ERROR);
> >> select JSON_EXISTS(jsonb '{"a": [1,2,3]}', 'strict $.a[5]' default 11 ON ERROR);
> >
> >
> > Yeah, surprised it works, the documented behavior seems logical.  Being able to return a non-boolean here seems odd.  Especially since it is cast to boolean on output.
> >
> >>
> >> JSON_VALUE on error, on empty semantics should be the same as json_query.
> >> like:
> >> [ { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression }
> >> ON EMPTY ]
> >> [ { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression }
> >> ON ERROR ])
> >>
> >> examples:
> >> select JSON_value(jsonb '[]' , '$'  empty array on error);
> >> select JSON_value(jsonb '[]' , '$'  empty object on error);
> >
> >
> > Again the documented behavior seems to make sense though and the ability to specify empty in the value function seems like a bug.  If you really want an empty array or object you do have access to default.  The reason json_query provides for an empty array/object is that it is already expecting to produce an array (object seems a little odd).
> >
> > I agree our docs and code do not match which needs to be fixed, ideally in the direction of the standard which I'm guessing our documentation is based off of.  But let's not go off of my guess.
>
> Oops, that is indeed not great and, yes, the problem is code not
> matching the documentation, the latter of which is "correct".
>
> Basically, the grammar allows specifying any of the all possible ON
> ERROR/EMPTY behavior values irrespective of the function, so parse
> analysis should be catching and flagging an attempt to specify
> incompatible value for a given function, which it does not.
>
> I've attached a patch to fix that, which I'd like to push before
> anything else we've been discussing.

While there are still a few hours to go before Saturday noon UTC when
beta2 freeze goes into effect, I'm thinking to just push this after
beta2 is stamped.  Adding an open item for now.

-- 
Thanks, Amit Langote

  12. Re: SQL/JSON query functions context_item doc entry and type requirement

    jian he <jian.universality@gmail.com> — 2024-06-22T09:39:34Z

    On Fri, Jun 21, 2024 at 8:18 PM Amit Langote <amitlangote09@gmail.com> wrote:
>
> >> JSON_VALUE on error, on empty semantics should be the same as json_query.
> >> like:
> >> [ { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression }
> >> ON EMPTY ]
> >> [ { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression }
> >> ON ERROR ])
> >>
> >> examples:
> >> select JSON_value(jsonb '[]' , '$'  empty array on error);
> >> select JSON_value(jsonb '[]' , '$'  empty object on error);
> >
> > Again the documented behavior seems to make sense though and the ability to specify empty in the value function seems like a bug.  If you really want an empty array or object you do have access to default.  The reason json_query provides for an empty array/object is that it is already expecting to produce an array (object seems a little odd).
> >
> > I agree our docs and code do not match which needs to be fixed, ideally in the direction of the standard which I'm guessing our documentation is based off of.  But let's not go off of my guess.
>
> Oops, that is indeed not great and, yes, the problem is code not
> matching the documentation, the latter of which is "correct".
>
> Basically, the grammar allows specifying any of the all possible ON
> ERROR/EMPTY behavior values irrespective of the function, so parse
> analysis should be catching and flagging an attempt to specify
> incompatible value for a given function, which it does not.
>
> I've attached a patch to fix that, which I'd like to push before
> anything else we've been discussing.
>

+ errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("invalid ON ERROR behavior"),
+ errdetail("Only ERROR, NULL, EMPTY [ ARRAY | OBJECT }, or DEFAULT
<value> is allowed in ON ERROR for JSON_QUERY()."),
+ parser_errposition(pstate, func->on_error->location));

`EMPTY [ ARRAY | OBJECT }` seems not correct,
maybe just EMPTY, EMPTY ARRAY, EMPTY OBJECT.
(apply to other places)


`DEFAULT <value>`
`DEFAULT <expression>` or just `DEFAULT expression` would be more correct?
(apply to other places)

I think we should make json_query, json_value on empty, on error
behave the same way.
otherwise, it will have consistency issues for scalar jsonb.
for example, we should expect the following two queries to  return the
same result?
SELECT * FROM JSON_query(jsonb '1', '$.a' returning jsonb empty on empty);
SELECT * FROM JSON_value(jsonb '1', '$.a' returning jsonb empty on empty);

Also the json_table function will call json_value or json_query,
make these two functions on error, on empty behavior the same can
reduce unintended complexity.

So based on your
patch(v1-0001-SQL-JSON-Disallow-incompatible-values-in-ON-ERROR.patch)
and the above points, I have made some changes, attached.
it will make json_value, json_query not allow {true | false | unknown
} on error, {true | false | unknown } on empty.
json_table error message deal the same way as
b4fad46b6bc8a9bf46ff689bcb1bd4edf8f267af


BTW,
i found one JSON_TABLE document deficiency
        [ { ERROR | NULL | EMPTY { ARRAY | OBJECT } | DEFAULT
expression } ON EMPTY ]
        [ { ERROR | NULL | EMPTY { ARRAY | OBJECT } | DEFAULT
expression } ON ERROR ]

it should be

        [ { ERROR | NULL | EMPTY { [ARRAY] | OBJECT } | DEFAULT
expression } ON EMPTY ]
        [ { ERROR | NULL | EMPTY { [ARRAY] | OBJECT } | DEFAULT
expression } ON ERROR ]

  13. Re: SQL/JSON query functions context_item doc entry and type requirement

    Amit Langote <amitlangote09@gmail.com> — 2024-06-27T12:01:18Z

    Hi,

On Sat, Jun 22, 2024 at 6:39 PM jian he <jian.universality@gmail.com> wrote:
> On Fri, Jun 21, 2024 at 8:18 PM Amit Langote <amitlangote09@gmail.com> wrote:
> >
> > >> JSON_VALUE on error, on empty semantics should be the same as json_query.
> > >> like:
> > >> [ { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression }
> > >> ON EMPTY ]
> > >> [ { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression }
> > >> ON ERROR ])
> > >>
> > >> examples:
> > >> select JSON_value(jsonb '[]' , '$'  empty array on error);
> > >> select JSON_value(jsonb '[]' , '$'  empty object on error);
> > >
> > > Again the documented behavior seems to make sense though and the ability to specify empty in the value function seems like a bug.  If you really want an empty array or object you do have access to default.  The reason json_query provides for an empty array/object is that it is already expecting to produce an array (object seems a little odd).
> > >
> > > I agree our docs and code do not match which needs to be fixed, ideally in the direction of the standard which I'm guessing our documentation is based off of.  But let's not go off of my guess.
> >
> > Oops, that is indeed not great and, yes, the problem is code not
> > matching the documentation, the latter of which is "correct".
> >
> > Basically, the grammar allows specifying any of the all possible ON
> > ERROR/EMPTY behavior values irrespective of the function, so parse
> > analysis should be catching and flagging an attempt to specify
> > incompatible value for a given function, which it does not.
> >
> > I've attached a patch to fix that, which I'd like to push before
> > anything else we've been discussing.
> >
>
> + errcode(ERRCODE_SYNTAX_ERROR),
> + errmsg("invalid ON ERROR behavior"),
> + errdetail("Only ERROR, NULL, EMPTY [ ARRAY | OBJECT }, or DEFAULT
> <value> is allowed in ON ERROR for JSON_QUERY()."),
> + parser_errposition(pstate, func->on_error->location));
>
> `EMPTY [ ARRAY | OBJECT }` seems not correct,
> maybe just EMPTY, EMPTY ARRAY, EMPTY OBJECT.
> (apply to other places)

Or EMPTY [ ARRAY ], EMPTY OBJECT

> `DEFAULT <value>`
> `DEFAULT <expression>` or just `DEFAULT expression` would be more correct?
> (apply to other places)

"DEFAULT expression" sounds good.

> I think we should make json_query, json_value on empty, on error
> behave the same way.
> otherwise, it will have consistency issues for scalar jsonb.
> for example, we should expect the following two queries to  return the
> same result?
> SELECT * FROM JSON_query(jsonb '1', '$.a' returning jsonb empty on empty);
> SELECT * FROM JSON_value(jsonb '1', '$.a' returning jsonb empty on empty);
>
> Also the json_table function will call json_value or json_query,
> make these two functions on error, on empty behavior the same can
> reduce unintended complexity.
>
> So based on your
> patch(v1-0001-SQL-JSON-Disallow-incompatible-values-in-ON-ERROR.patch)
> and the above points, I have made some changes, attached.
> it will make json_value, json_query not allow {true | false | unknown
> } on error, {true | false | unknown } on empty.
> json_table error message deal the same way as
> b4fad46b6bc8a9bf46ff689bcb1bd4edf8f267af

Here is an updated patch that I think takes care of these points.

> BTW,
> i found one JSON_TABLE document deficiency
>         [ { ERROR | NULL | EMPTY { ARRAY | OBJECT } | DEFAULT
> expression } ON EMPTY ]
>         [ { ERROR | NULL | EMPTY { ARRAY | OBJECT } | DEFAULT
> expression } ON ERROR ]
>
> it should be
>
>         [ { ERROR | NULL | EMPTY { [ARRAY] | OBJECT } | DEFAULT
> expression } ON EMPTY ]
>         [ { ERROR | NULL | EMPTY { [ARRAY] | OBJECT } | DEFAULT
> expression } ON ERROR ]

You're right.  Fixed.

Also, I noticed that the grammar allows ON EMPTY in JSON_TABLE EXISTS
columns which is meaningless because JSON_EXISTS() doesn't have a
corresponding ON EMPTY clause.  Fixed grammar to prevent that in the
attached 0002.

--
Thanks, Amit Langote

  14. Re: SQL/JSON query functions context_item doc entry and type requirement

    Amit Langote <amitlangote09@gmail.com> — 2024-06-28T05:18:12Z

    On Thu, Jun 27, 2024 at 9:01 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Sat, Jun 22, 2024 at 6:39 PM jian he <jian.universality@gmail.com> wrote:
> > On Fri, Jun 21, 2024 at 8:18 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > >
> > > >> JSON_VALUE on error, on empty semantics should be the same as json_query.
> > > >> like:
> > > >> [ { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression }
> > > >> ON EMPTY ]
> > > >> [ { ERROR | NULL | EMPTY { [ ARRAY ] | OBJECT } | DEFAULT expression }
> > > >> ON ERROR ])
> > > >>
> > > >> examples:
> > > >> select JSON_value(jsonb '[]' , '$'  empty array on error);
> > > >> select JSON_value(jsonb '[]' , '$'  empty object on error);
> > > >
> > > > Again the documented behavior seems to make sense though and the ability to specify empty in the value function seems like a bug.  If you really want an empty array or object you do have access to default.  The reason json_query provides for an empty array/object is that it is already expecting to produce an array (object seems a little odd).
> > > >
> > > > I agree our docs and code do not match which needs to be fixed, ideally in the direction of the standard which I'm guessing our documentation is based off of.  But let's not go off of my guess.
> > >
> > > Oops, that is indeed not great and, yes, the problem is code not
> > > matching the documentation, the latter of which is "correct".
> > >
> > > Basically, the grammar allows specifying any of the all possible ON
> > > ERROR/EMPTY behavior values irrespective of the function, so parse
> > > analysis should be catching and flagging an attempt to specify
> > > incompatible value for a given function, which it does not.
> > >
> > > I've attached a patch to fix that, which I'd like to push before
> > > anything else we've been discussing.
> > >
> >
> > + errcode(ERRCODE_SYNTAX_ERROR),
> > + errmsg("invalid ON ERROR behavior"),
> > + errdetail("Only ERROR, NULL, EMPTY [ ARRAY | OBJECT }, or DEFAULT
> > <value> is allowed in ON ERROR for JSON_QUERY()."),
> > + parser_errposition(pstate, func->on_error->location));
> >
> > `EMPTY [ ARRAY | OBJECT }` seems not correct,
> > maybe just EMPTY, EMPTY ARRAY, EMPTY OBJECT.
> > (apply to other places)
>
> Or EMPTY [ ARRAY ], EMPTY OBJECT
>
> > `DEFAULT <value>`
> > `DEFAULT <expression>` or just `DEFAULT expression` would be more correct?
> > (apply to other places)
>
> "DEFAULT expression" sounds good.
>
> > I think we should make json_query, json_value on empty, on error
> > behave the same way.
> > otherwise, it will have consistency issues for scalar jsonb.
> > for example, we should expect the following two queries to  return the
> > same result?
> > SELECT * FROM JSON_query(jsonb '1', '$.a' returning jsonb empty on empty);
> > SELECT * FROM JSON_value(jsonb '1', '$.a' returning jsonb empty on empty);
> >
> > Also the json_table function will call json_value or json_query,
> > make these two functions on error, on empty behavior the same can
> > reduce unintended complexity.
> >
> > So based on your
> > patch(v1-0001-SQL-JSON-Disallow-incompatible-values-in-ON-ERROR.patch)
> > and the above points, I have made some changes, attached.
> > it will make json_value, json_query not allow {true | false | unknown
> > } on error, {true | false | unknown } on empty.
> > json_table error message deal the same way as
> > b4fad46b6bc8a9bf46ff689bcb1bd4edf8f267af
>
> Here is an updated patch that I think takes care of these points.
>
> > BTW,
> > i found one JSON_TABLE document deficiency
> >         [ { ERROR | NULL | EMPTY { ARRAY | OBJECT } | DEFAULT
> > expression } ON EMPTY ]
> >         [ { ERROR | NULL | EMPTY { ARRAY | OBJECT } | DEFAULT
> > expression } ON ERROR ]
> >
> > it should be
> >
> >         [ { ERROR | NULL | EMPTY { [ARRAY] | OBJECT } | DEFAULT
> > expression } ON EMPTY ]
> >         [ { ERROR | NULL | EMPTY { [ARRAY] | OBJECT } | DEFAULT
> > expression } ON ERROR ]
>
> You're right.  Fixed.
>
> Also, I noticed that the grammar allows ON EMPTY in JSON_TABLE EXISTS
> columns which is meaningless because JSON_EXISTS() doesn't have a
> corresponding ON EMPTY clause.  Fixed grammar to prevent that in the
> attached 0002.

I've pushed this for now to close out the open item.

I know there's some documentation improvement work left to do [1],
which I'll try to find some time for next week.

-- 
Thanks, Amit Langote

[1] https://www.postgresql.org/message-id/CAKFQuwbYBvUZasGj_ZnfXhC2kk4AT%3DepwGkNd2%3DRMMVXkfTNMQ%40mail.gmail.com

  15. Re: SQL/JSON query functions context_item doc entry and type requirement

    David G. Johnston <david.g.johnston@gmail.com> — 2024-06-28T05:27:06Z

    On Thursday, June 20, 2024, David G. Johnston <david.g.johnston@gmail.com>
wrote:

>
>> >
>> > > As for table 9.16.3 - it is unwieldy already.  Lets try and make the
>> core syntax shorter, not longer.  We already have precedence in the
>> subsequent json_table section - give each major clause item a name then
>> below the table define the syntax and meaning for those names.  Unlike in
>> that section - which probably should be modified too - context_item should
>> have its own description line.
>> >
>> > I had posted a patch a little while ago at [1] to render the syntax a
>> > bit differently with each function getting its own syntax synopsis.
>> > Resending it here; have addressed Jian He's comments.
>> >
>> > --
>>
>
> I was thinking more like:
>
> diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
> index c324906b22..b9d157663a 100644
> --- a/doc/src/sgml/func.sgml
> +++ b/doc/src/sgml/func.sgml
> @@ -18692,8 +18692,10 @@ $.* ? (@ like_regex "^\\d+$")
>        <entry role="func_table_entry"><para role="func_signature">
>          <indexterm><primary>json_exists</primary></indexterm>
>          <function>json_exists</function> (
> -        <replaceable>context_item</replaceable>,
> <replaceable>path_expression</replaceable> <optional>
> <literal>PASSING</literal> { <replaceable>value</replaceable>
> <literal>AS</literal> <replaceable>varname</replaceable> } <optional>,
> ...</optional></optional>
> -        <optional> { <literal>TRUE</literal> | <literal>FALSE</literal>
> |<literal> UNKNOWN</literal> | <literal>ERROR</literal> } <literal>ON
> ERROR</literal> </optional>)
> +        <replaceable>context_item</replaceable>,
> +        <replaceable>path_expression</replaceable>
> +        <optional>variable_definitions</optional>
> +        <optional>on_error_boolean</optional>)
>         </para> empty semantics should be the same as json_query.
>
>>
>>
The full first draft patch for this is here:

https://www.postgresql.org/message-id/CAKFQuwZNxNHuPk44zDF7z8qZec1Aof10aA9tWvBU5CMhEKEd8A@mail.gmail.com

David J.