Hi, On Sat, Jun 22, 2024 at 6:39 PM jian he <[email protected]> wrote: > On Fri, Jun 21, 2024 at 8:18 PM Amit Langote <[email protected]> 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
v2-0002-SQL-JSON-Prevent-ON-EMPTY-for-EXISTS-columns-in-J.patch
Description: Binary data
v2-0001-SQL-JSON-Disallow-incompatible-values-in-ON-ERROR.patch
Description: Binary data
