From: | Oleg Bartunov <obartunov(at)gmail(dot)com> |
---|---|
To: | Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> |
Cc: | Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>, Pavel Stěhule <pavel(dot)stehule(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Piotr Stefaniak <email(at)piotr-stefaniak(dot)me>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>, Teodor Sigaev <teodor(at)postgrespro(dot)ru>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, andrew Dunstan <andrew(at)dunslane(dot)net> |
Subject: | Re: [HACKERS] SQL/JSON in PostgreSQL |
Date: | 2018-01-10 06:37:30 |
Message-ID: | CAF4Au4wVxdDfyyayxU_qdZpR3eeOqBnBahA77Ar7zvAwKGEJ=A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 9 Jan 2018 23:44, "Andrew Dunstan" <andrew(dot)dunstan(at)2ndquadrant(dot)com>
wrote:
On 01/02/2018 05:23 PM, Andrew Dunstan wrote:
>
> On 01/02/2018 05:04 PM, Nikita Glukhov wrote:
>> I have removed all extra features from the patch set, they can be
>> found in our
>> github repository:
>> https://github.com/postgrespro/sqljson/tree/sqljson_ext.
>>
>> Now there are 10 patches which have the following dependencies:
>>
>> 1:
>> 2: 1
>> 3: 2
>> 4: 2
>> 5:
>> 6:
>> 7: 2, 5, 6
>> 8: 7, 4
>> 9: 7
>> 10: 8, 9
>>
>
> OK. We need to get this into the commitfest. Preferably not all in one
> piece. I would do:
>
> 1, 5, and 6 independently
> 2, 3 and 4 together
> 7 and 8 together
> 9 and 10 together
>
> Those seem like digestible pieces.
>
> Also, there is a spurious BOM at the start of
> src/test/regress/sql/sqljson.sql in patch 7. Please fix that.
>
OK, an extended version of patch 1 has been committed. I'm working on
patches 2, 3, and 4 (the jsonpath patches) as time permits (and right
now time is very tight). Here are some very preliminary comments:
* The documentation needs improvement. A para with contents of "TODO"
is not acceptable.
* I'm not generally a fan of using flex/bison for small languages like
this. Something similar to what we're using for json itself (a
simple lexer and a recursive descent parser) would be more suitable.
flex/bison is right tool for jsonpath, which is complex thing.
Others might have different views.
* The timestamp handling refactoring is a good thing but would
probably be better done as a separate patch.
Agree
* the code is pretty sparsely commented. Quite apart from other
considerations that makes it harder to review.
I also note that the later patches have no documentation whatsoever.
That needs serious work, and if you want to get these patches in then
please supply some documentation ASAP. If you need help with English we
can work on that, but just throwing patches of this size and complexity
over the wall into the commitfest without any documentation is not the
way to proceed.
Andrew, we are back from holidays and I will start writing on
documentation. I have difficulty with design of documentation, since it's
unclear to me how detailed it should be. I'm inclining to follow xml style
of documentation, which is quite formal and could be more easy to write.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Oliver Ford | 2018-01-10 06:56:41 | Re: Add RANGE with values and exclusions clauses to the Window Functions |
Previous Message | Thomas Munro | 2018-01-10 06:12:03 | Re: pgsql: Improve scripting language in pgbench |