2018-01-09 21:44 GMT+01:00 Andrew Dunstan <andrew.duns...@2ndquadrant.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 > >
