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
