From: Nikita Glukhov <n.gluhov@postgrespro.ru>
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Oleg Bartunov <obartunov@gmail.com>
Cc: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Subject: Re: SQL/JSON: JSON_TABLE
Date: Tue, 12 Nov 2019 03:09:42 +0300
Message-ID: <bc39560b-6681-9314-44e5-815c16f99560@postgrespro.ru> (raw)
In-Reply-To: <CAFj8pRDBiX=tqHJEWKevfatwSPkb4C6bYHvxTYhxrG8mTvJ2+A@mail.gmail.com>
References: <132f26c4-dfc6-f8fd-4764-2cbf455a3aec@postgrespro.ru>
<fdface69-deda-6e58-cb22-3e57e001923e@postgrespro.ru>
<CA+q6zcXo7WeBnDoY5W8-ujB16UPs3xKH8LGA=q1_R8=Atb_8_Q@mail.gmail.com>
<5ded9f9d-cf93-5bd6-a4c3-ad8937245ced@postgrespro.ru>
<6f5255ce-ae02-d213-49aa-7b86004ab080@postgrespro.ru>
<c2f32c9f-9a69-202b-a8aa-d93c769a579e@postgrespro.ru>
<CA+TgmobvF-T6f8Z1LWcR1XzjHG4u8i7TxrAiN8QLzuR=OxLW8Q@mail.gmail.com>
<deafbced-b992-04f5-64ac-5feeb3942f5c@postgrespro.ru>
<7f23e8bd-d9fa-1c88-12ad-ded12ea5f221@postgrespro.ru>
<CAFj8pRCc=7w3Ce1R4o4x8TkSLHrU1Ru+vXUg=qAvCnEUywEAeA@mail.gmail.com>
<c9dbd1fb-6398-d5bb-19cf-71e71e2f750b@postgrespro.ru>
<CAFj8pRCxyZgTSJWoxaKiDUc-BKYgKmzsAMXF2QeKv-dpB6pyrw@mail.gmail.com>
<94ab028a-07aa-38e5-b184-3d640f642bfb@postgrespro.ru>
<5d304e31-9db2-c6d0-c282-b2e4000b0430@postgrespro.ru>
<CAFj8pRCx2MTmhSnUHsCzZCLz0SfA-JJ_VGv6g3EHAtmPLyppKw@mail.gmail.com>
<CAFj8pRDBiX=tqHJEWKevfatwSPkb4C6bYHvxTYhxrG8mTvJ2+A@mail.gmail.com>
Attached 40th version of the patches.
On 19.10.2019 18:31, Pavel Stehule wrote:
> This patch is still pretty big - it is about 6000 lines (without any
> documentation). I checked the standard - and this patch try to implement
>
> JSON_TABLE as part of T821
> Plan clause T824
> Plan default clause T838.
>
> Unfortunately for last two features there are few documentation other
> than standard, and probably other databases doesn't implement these
> features (I didn't find it in Oracle, MySQL, MSSQL and DB2) . Can be
> this patch divided by these features? I hope so separate review and
> commit can increase a chance to merge this code (or the most
> significant part) in this release.
>
> It is pretty hard to do any deeper review without documentation and
> without other information sources.
>
> What do you think?
I think it is a good idea. So I have split JSON_TABLE patch into three
patches, each SQL feature. This really helped to reduce the size of the
main patch by about 40%.
On 30.09.2019 19:09, Pavel Stehule wrote:
> Regress tests fails on my comp - intel 64bit Linux, gcc 9.2.1
>
Unfortunately, this is still not reproducible on my computer with 64bit
Linux and gcc 9.2.1.
> Comments:
>
> * +<->/* Only XMLTABLE and JSON_TABLE are supported currently */
>
> this comment has not sense more. Can be removed. Probably long time
> there will not be new format like XML or JSON
Fixed.
> * there are new 600 lines to parse_clause.c, maybe this code can be
> placed in new file parse_jsontable.c ? parse_clause.c is pretty long
> already (json_table has very complex syntax)
Ok, the code was moved to parse_jsontable.c.
> *
> +<->if (list_length(ci->passing.values) > 0)
> +<->{
> +<-><-->ListCell *exprlc;
> +<-><-->ListCell *namelc;
> +
>
> It's uncommon usage of list_length function. More common is just "if
> (ci->passing.values) {}". Is there any reason for list_length?
>
Fixed.
> * I tested some examples that I found on net. It works very well.
> Minor issues are white chars for json type. Probably json_table should
> to trim returned values, because after cutting from document, original
> white chars lost sense. It is not a problem jsonb type, that reduce
> white chars on input.
>
> I did only simple tests and I didn't find any other issues than white
> chars problems for json type. I'll continue in some deeper tests.
> Please, prepare documentation. Without documentation there is not
> clean what features are supported. I have to do blind testing.
>
I have added some documentation to the patches which has simply been
copied from [1], but It still needs some work.
[1]
https://www.postgresql.org/message-id/732208d3-56c3-25a4-8f08-3be1d54ad51b%40postgrespro.ru
--
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachments:
0001-SQL-JSON-functions-v40.patch.gz (application/gzip, 120.3K) download
0002-JSON_TABLE-v40.patch.gz (application/gzip, 25.4K) download
0003-JSON_TABLE-PLAN-DEFAULT-clause-v40.patch.gz (application/gzip, 7.1K) download
0004-JSON_TABLE-PLAN-clause-v40.patch.gz (application/gzip, 13.0K) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: n.gluhov@postgrespro.ru, pavel.stehule@gmail.com, robertmhaas@gmail.com, 9erthalion6@gmail.com, obartunov@gmail.com, andrew.dunstan@2ndquadrant.com
Subject: Re: SQL/JSON: JSON_TABLE
In-Reply-To: <bc39560b-6681-9314-44e5-815c16f99560@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox