Il dom 8 lug 2018, 00:57 Michael Mior <[email protected]> ha scritto: > I've done a scan of the code and it looks great to me. Also compiled > and ran tests on my machine just in case. I love commits that delete > more code than they add (over 2,600 deleted lines!) I did have a > couple comments that were mostly just clarifications that I added on > the relevant commit. Thanks for this Julian! I know making these kinds > of sweeping changes can be pretty dreary. > > I noticed you had a couple suggestions in your commit message for > future changes. Should there be JIRA issues logged for these? (If > there haven't been already). Also, with all the new deprecations that > have been added (which I agree with), I wonder if it's time to start > thinking about planning a 2.0 release? > > In any case, since the changes are so pervasive, it would be good to > get this merged as soon as possible to avoid conflicts with future > PRs. Thanks again Julian! > > -- > Michael Mior > [email protected] > Le sam. 7 juil. 2018 à 16:29, Julian Hyde <[email protected]> a écrit : > > > > I have two major changes ready to check in. Can someone please review > them? > > > > * https://issues.apache.org/jira/browse/CALCITE-2280 Babel SQL parser > > * https://issues.apache.org/jira/browse/CALCITE-2259 Java 8 syntax > > > > Both are staged in the same branch - > > https://github.com/julianhyde/calcite/commits/2280-babel - but it > > could be different people reviewing each. > > > > I am especially interested in high-level comments about design and > > development practices. >
The high level approach of Babel is good to me. I am sorry I have not much time to try out the branch and do some of the implementations on Babel before 1.17 release (which I am assuming it to be immediate after merging this feature branch) Most of my problems are about letting some reserved words to be used as column/schema/table identifiers (like havung a table with name 'user' or a schema with name 'default') and it seems pretty simple to implement, if I am userstanding correctly how Babel works. I would like also to add support for writing timestamps values as raw strings. This is my +1 to that branch, hoping to merge it soon so that we have time to test it on dowstream projects for a couple of weeks before the release. There are a lot of changes (thinking about java8 syntax) and bugs are likely to be added. I did a scan thru the change and apparently it looks okay but there are tons of small changes. I really like that Calcite is moving away from Guava Great work! Enrico > > > Julian > -- -- Enrico Olivelli
