On Fri, Jun 21, 2024 at 9:18 PM Amit Langote <amitlangot...@gmail.com> wrote:
> On Fri, Jun 21, 2024 at 9:47 AM David G. Johnston
> <david.g.johns...@gmail.com> wrote:
> > On Thu, Jun 20, 2024 at 9:01 AM jian he <jian.universal...@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


Reply via email to