Message-ID: <4b3cb6d0-d22b-2ae3-792c-9042f3dda2d9@gmail.com>
Date: Mon, 3 Apr 2023 23:00:00 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: SQL/JSON revisited
Content-Language: en-US
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
References: <20230403171647.5xonog3myjdgnad7@alvherre.pgsql>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: <20230403171647.5xonog3myjdgnad7@alvherre.pgsql>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/4b3cb6d0-d22b-2ae3-792c-9042f3dda2d9%40gmail.com>
Precedence: bulk

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