Re: SQL/JSON revisited

Hi Alvaro,

On Tue, Apr 4, 2023 at 2:16 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> On 2023-Mar-29, Alvaro Herrera wrote:
> > In the meantime, here's the next two patches of the series: IS JSON and
> > the "query" functions.  I think this is as much as I can get done for
> > this release, so the last two pieces of functionality would have to wait
> > for 17.  I still need to clean these up some more.  These are not
> > thoroughly tested either; 0001 compiles and passes regression tests, but
> > I didn't verify 0003 other than there being no Git conflicts and bison
> > doesn't complain.
> >
> > Also, notable here is that I realized that I need to backtrack on my
> > change of the WITHOUT_LA: the original patch had it for TIME (in WITHOUT
> > TIME ZONE), and I changed to be for UNIQUE.  But now that I've done
> > "JSON query functions" I realize that it needed to be the other way for
> > the WITHOUT ARRAY WRAPPER clause too.  So 0002 reverts that choice.
>
> So I pushed 0001 on Friday, and here are 0002 (which I intend to push
> shortly, since it shouldn't be controversial) and the "JSON query
> functions" patch as 0003.  After looking at it some more, I think there
> are some things that need to be addressed by one of the authors:
>
> - the gram.y solution to the "ON ERROR/ON EMPTY" clauses is quite ugly.
>   I think we could make that stuff use something similar to
>   ConstraintAttributeSpec with an accompanying post-processing function.
>   That would reduce the number of ad-hoc hacks, which seem excessive.

Do you mean the solution involving the JsonBehavior node?

> - the changes in formatting.h have no explanation whatsoever.  At the
>   very least, the new function should have a comment in the .c file.
>   (And why is it at end of file?  I bet there's a better location)
>
> - some nasty hacks are being used in the ECPG grammar with no tests at
>   all.  It's easy to add a few lines to the .pgc file I added in prior
>   commits.
>
> - Some functions in jsonfuncs.c have changed from throwing hard errors
>   into soft ones.  I think this deserves more commentary.
>
> - func.sgml: The new functions are documented in a separate table for no
>   reason that I can see.  Needs to be merged into one of the existing
>   tables.  I didn't actually review the docs.

I made the jsonfuncs.c changes to use soft error handling when needed,
so I took a stab at that; attached a delta patch, which also fixes the
problematic comments mentioned by Alexander in his comments 1 and 3.

I'll need to spend some time to address other points.

-- 
Thanks, Amit Langote
EDB: http://www.enterprisedb.com