Re: [HACKERS] SQL/JSON in PostgreSQL

From: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
To: Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>, obartunov(at)gmail(dot)com
Cc: Pavel Stehule <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-09 20:44:03
Message-ID: ee893374-8db5-07f0-896b-ac292152ea1c@2ndQuadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.
Others might have different views.
* The timestamp handling refactoring is a good thing but would
probably be better done as a separate patch.
* 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.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-01-09 20:54:10 Re: improve type conversion of SPI_processed in Python
Previous Message Peter Eisentraut 2018-01-09 20:36:01 Re: Add %r substitution for psql prompts to show recovery status