Re: [HACKERS] SQL/JSON in PostgreSQL

2018-01-09 21:44 GMT+01:00 Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>:

>
>
> 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.
>

There should be introduction to JSONPath expressions

* 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.
>

I am think so flex/bison in this case is correct - JSONPath expression is
more complex language than JSON self

Regards

Pavel

> * 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
>
>