Hi Alvaro, On Tue, Apr 4, 2023 at 2:16 AM Alvaro Herrera <alvhe...@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
v15-0002-delta.patch
Description: Binary data