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

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
seemslike a bug.  If you really want an empty array or object you do have access to default.  The reason json_query
providesfor 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
guessingour 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 ]