Re: Doc Rework: Section 9.16.13 SQL/JSON Query Functions - Mailing list pgsql-hackers

hi.
the following review is based on v2-0001, v2-0002.

"context_item can be a JSON document passed as a value of type json,
jsonb document, a character or an UTF8- endoded bytea string."
is wrong?
e.g. SELECT JSON_EXISTS( NULL::bytea, 'lax $.a[5]' ERROR ON ERROR)

check following query:
select oid, typtype , typname from pg_type where typcategory = 'S';

I think a more accurate description would be:
"context_item must be a JSON document passed as a value of type json,
jsonb document, a character string type(text, name, bpchar, varchar)"
do we need to mention domain over these types?
-------------------------------------
JSON_EXISTS
Returns true if the SQL/JSON path_expression possibly referencing the
variables in variable_definitions applied to the context_item yields
any items.
I am not native English speaker, so I found it hard to comprehend.
I can understand it like:
"Returns true if the SQL/JSON path_expression (possibly referencing
the variables in variable_definitions) applied to the context_item
yields any items."

maybe we can write it into two sentences, or
"Returns true if the SQL/JSON path_expression applied to the
context_item yields any items."
because you already mentioned "path_expression can also contain
variables whose values are specified using the variable_definitions
clause described below." in the top level.
-------------------------------------
The JSON_QUERY and JSON_VALUE functions are polymorphic in their
output type with the returning_clause clause dictating what that type
is.
how about
The JSON_QUERY and JSON_VALUE functions output type can be vary, using
returning_clause specify the desired data type.

-------------------------------------
your doc: JSON_VALUE "If path_expression points to a JSON null,
JSON_VALUE returns a SQL NULL."
`SELECT JSON_VALUE(jsonb 'null', '$');` here, the path_expression
points to '$' which is not json null?
so i like to change it to
"If the extracted value is a JSON null, an SQL NULL value will return."
-------------------------------------
inconsistency:
JSON_QUERY: <returnvalue></returnvalue> { <type>jsonb</type> |
<replaceable>return_data_type</replaceable> }
JSON_VALUE: <returnvalue></returnvalue> { <type>text</type> |
<varname>return_data_type</varname> }
-------------------------------------
{{For JSON_EXISTS (... on_error_boolean), alternative can be: ERROR,
UNKNOWN, TRUE, FALSE.
For JSON_QUERY (... on_error_set on_empty_set), alternative can be:
ERROR, NULL, EMPTY ARRAY, EMPTY OBJECT, or DEFAULT followed by an
expression.
For JSON_VALUE (... on_error_set on_empty_set), alternative can be:
ERROR, NULL, or DEFAULT followed by an expression.
}}
i am not sure what does there dot means here, in the synopsis section,
three dots is significant.
Also if I understand it correctly, JSON_EXISTS can only have on_error,
then I am more confused with ``JSON_EXISTS (... on_error_boolean)``



Overall, I found this approach makes the synopsis scattered, it's not
easy to see the full picture.
for example:
```
JSON_VALUE ( context_item, path_expression [variable_definitions]
[return_type] [on_empty_value] [on_error_value]) → { text |
return_data_type }
 ```
this way it is not easy to find out that RETURNING is a keyword.
Currently in master, we can quickly see RETURNING is the keyword, the
master is kind of condense, though.
but if you are insistent with your approach,  then that is fine for me.