út 12. 11. 2019 v 1:13 odesílatel Nikita Glukhov <n.glu...@postgrespro.ru>
napsal:
> 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%.
>
super, I'lll check main patch only now - to time when it will be merged to
core
>
> 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.
>
Maybe it is locale depending issue. My LANG is LANG=cs_CZ.UTF-8
>
> 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.
>
ok
Pavel
>
> [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
>
