Re: SQL/JSON revisited
Alexander Law <exclusion@gmail.com>
From: Alexander Lakhin <exclusion@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Erik Rijkers <er@xs4all.nl>, Andrew Dunstan <andrew@dunslane.net>,
Amit Langote <amitlangote09@gmail.com>,
Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
Andres Freund <andres@anarazel.de>, vignesh C <vignesh21@gmail.com>,
PostgreSQL-development <pgsql-hackers@postgresql.org>,
e.indrupskaya@postgrespro.ru
Date: 2023-04-03T20:00:00Z
Lists: pgsql-hackers
Hi Alvaro, 03.04.2023 20:16, Alvaro Herrera wrote: > 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. > > - 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. Please take a look at the following minor issues in v15-0002-SQL-JSON-query-functions.patch: 1) s/addreess/address/ 2) ECPGColLabelCommon gone with 83f1c7b74, but is still mentioned in ecpg.trailer. 3) s/ExecEvalJsonCoercion/ExecEvalJsonExprCoercion/ ? (there is no ExecEvalJsonCoercion() function) 4) json_table mentioned in func.sgml: <xref linkend="functions-sqljson-querying"/> details the SQL/JSON functions that can be used to query JSON data, except for <function>json_table</function>. but if JSON_TABLE not going to be committed in v16, maybe remove that reference to it. There is also a reference to JSON_TABLE in src/backend/parser/README: parse_jsontable.c handle JSON_TABLE (It was added with 9853bf6ab and survived the revert of SQL JSON last year somehow.) Best regards, Alexander
Commits
-
Add SQL/JSON query functions
- 6185c9737cf4 17.0 landed
-
Add soft error handling to some expression nodes
- aaaf9449ec6b 17.0 landed
-
Adjust populate_record_field() to handle errors softly
- 1edb3b491bee 17.0 landed
-
Refactor code used by jsonpath executor to fetch variables
- faa2b953ba3b 17.0 landed
-
Code review for recent SQL/JSON commits
- 71bfd1543f8b 16.0 landed
-
Fix inconsistencies and style issues in new SQL/JSON code
- 60966f56c3e4 16.0 landed
-
Don't install postmaster symlink anymore
- 37e267335068 16.0 cited
-
Revert SQL/JSON features
- 2f2b18bd3f55 16.0 cited
-
Doc: standardize markup a bit more.
- 47046763c3ed 13.0 cited