Re: SQL/JSON: functions

Attached 42th version of the patches.

On 18.01.2020 21:21, Pavel Stehule wrote:
> On 18. 1. 2020 v 18:46 Nikita Glukhov <n.gluhov@postgrespro.ru 
> <mailto:n.gluhov@postgrespro.ru>> wrote:
>
>     Attached 41th version of the patches.
>
>     Changes since previous version:
>       * Enabled DEFAULT clause for ON ERROR/ON EMPTY behaviors in JSON_QUERY()
>       * Added RETURNING clause to JSON_EXISTS() ("side effect" of implementation
>         EXISTS PATH columns in JSON_TABLE)
>       * ARRAY in EMPTY ARRAY ON ERROR clause is optional now for better Oracle
>         compatibility
>
>
>     On 17.01.2020 9:54, Pavel Stehule wrote:
>
>>     I tested cumulative patch - sent in json_table patch.
>>
>>     I almost satisfied by quality of this patch. There is very good
>>     conformance with standard and with Oracle. Unfortunately MySQL in
>>     this part of JSON support is not compatible.
>>
>>     I found one issue, when I tested some examples from Oracle.
>>
>>     SELECT JSON_VALUE('{a:100}', '$.a' RETURNING int) AS value;
>>
>>     then the result was null.
>>
>>     But it is wrong, because it should to raise a exception, because
>>     this json is broken on Postgres (Postgres requires quoted
>>     attribute names)
>>
>>     json_query has same problem
>>
>>     postgres=# SELECT JSON_QUERY('{a:100, b:200, c:300}', '$') AS value;
>>     ┌───────┐
>>     │ value │
>>     ╞═══════╡
>>     │ ∅     │
>>     └───────┘
>>     (1 row)
>>
>>     It should to check if input is correct json
>
>     By the standard, it is implementation-defined whether JSON parsing errors
>     should be caught by ON ERROR clause.
>
>     SQL/JSON query functions use "JSON API common syntax" which is a combination
>     of JSON context item and JSON path.  It passes context item to JSON path
>     engine with ALREADY PARSED flag set to False.  ALREADY PARSED flag can enable
>     special parsing rules.
>
>     Corresponding quotes from the standard:
>
>     10.14 <JSON API common syntax>
>        <JSON API common syntax> (
>           Parameter: "JSON API COMMON SYNTAX"
>        ) Returns: "STATUS" and "SQL/JSON SEQUENCE"
>
>     General Rules:
>     ...
>     3) General Rules of Subclause 9.39, "SQL/JSON path language: syntax and
>         semantics", are applied with P as PATH SPECIFICATION, C as CONTEXT ITEM,
>         False as ALREADY PARSED, and PC as PASSING CLAUSE; let ST be the STATUS
>         and let SEQ be the SQL/JSON SEQUENCE returned from the application of
>         those General Rules.
>
>     9.39 SQL/JSON path language: syntax and semantics
>
>     "SQL/JSON path language: syntax and semantics" [General Rules] (
>        Parameter: "PATH SPECIFICATION",
>        Parameter: "CONTEXT ITEM",
>        Parameter: "ALREADY PARSED",
>        Parameter: "PASSING CLAUSE"
>     ) Returns: "STATUS" and "SQL/JSON SEQUENCE"
>
>     General Rules:
>     ...
>
>     4) If ALREADY PARSED is False, then it is implementation-defined whether the
>         following rules are applied:
>        a) The General Rules of Subclause 9.36, "Parsing JSON text", are applied with
>           JT as JSON TEXT, an implementation-defined <JSON key uniqueness constraint>
>           as UNIQUENESS CONSTRAINT, and FO as FORMAT OPTION; let ST be the STATUS and
>           let CISJI be the SQL/JSON ITEM returned from the application of those
>           General Rules.
>        b) If ST is not successful completion, then ST is returned as the STATUS of
>           this application of these General Rules, and no further General Rules of
>           this Subclause are applied.
>
>
>     I decided to apply this rules, so the parsing errors are caught now by ON ERROR
>     (NULL ON ERROR is by default).
>
>     postgres=# SELECT JSON_VALUE('error', '$' ERROR ON ERROR);
>     ERROR:  invalid input syntax for type json
>     DETAIL:  Token "error" is invalid.
>     CONTEXT:  JSON data, line 1: error
>
>     I'm not sure if it would be better to add an implicit cast to json type that
>     will be executed before so that parsing errors can no longer be caught.
>     But implicit casting can simplify a bit execution of SQL/JSON query functions.
>
>
>     I have checked error handling in JSON parsing in Oracle 18c/19c, and it behaves
>     like our current implementation.  But Oracle seems to do JSON parsing on demand:
>
>     Oracle19c> SELECT JSON_VALUE('{a:1 error, b:2}', '$.a' ERROR ON ERROR) FROM dual;
>     1
>
>     Oracle19c> SELECT JSON_VALUE('{a:1 error, b:2}', '$.b' ERROR ON ERROR) FROM dual;
>     ORA-40441: JSON syntax error
>
>     Oracle19c> SELECT JSON_VALUE('{a:1 error, b:2}', '$.b') FROM dual;
>     NULL
>
>
> Everywhere I don't like default masking error. I think so can be very 
> confusing to get NULL
> (by default) instead a error of broken format.
>
> I vote for check of input is correct JSON, and if it, then start 
> processing. Else to raise a error.
>
> More - our JSON Parser is different than Oracle's JSON parser. And if 
> somebody will run Oracle's JSONs,
> then he get some result on Oracle. But on Postgres, same JSON can be 
> invalid, and he get NULL.
>
> The raising some errors looks like only one safe variant.

I have removed handling of parsing errors in SQL/JSON functions and JSON_TABLE.
Now, FORMAT JSON expressions (implicit or explicit) are simply transformed into
ordinary casts to json type, and these casts are executed before execution of
SQL/JSON functions.  Previously, separate expression was created for such casts,
and it was executed in the separate subtransaction in ExecEvalJsonExpr().  So,
this change also simplifies the code a bit.

-- 
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company