I've seen the commit coming through and I do like the approach of keeping the execution logic separate, thanks Julian!
Best, Stamatis On Mon, Jun 1, 2020 at 9:02 PM Julian Hyde <jh...@apache.org> wrote: > Just to finish this thread... I've now merged 3946. > > I did what Stamatis suggested, and moved the DDL classes into core. > However, I kept the execution logic (e.g. creating the table when > someone sends a SqlCreateTable command) in server. To do this, I > created a new interface DdlExecutor, and moved the logic from all of > the DDL sub-classes of SqlNode into it. It was a lot more work than I > expected. > > Julian > > On Fri, May 29, 2020 at 12:26 PM Julian Hyde <jh...@apache.org> wrote: > > > > Thanks for the feedback, everyone. Based on the feedback I will move > > the DDL classes to Core. I reserve the right to reverse direction if > > those DDL classes start changing in weird ways due to dialect > > requirements in Babel. > > > > If you have further comments, let's discuss in > > https://issues.apache.org/jira/browse/CALCITE-3946. > > > > I aim to get 3946 merged in the next day or two. > > > > Julian > > > > On Sun, May 17, 2020 at 2:20 PM Stamatis Zampetakis <zabe...@gmail.com> > wrote: > > > > > > Hello, > > > > > > I prefer a bit more the alternative of putting all DDL classes into > "core". > > > > > > When people talk about an SQL parser usually they mean the complete > beast > > > (DQL, DDL, DML). > > > Personally, I wouldn't be surprised to find DDL in "core", especially > since > > > DML is already in. > > > > > > The dependency from "babel" to "server" sounds a bit weird at first but > > > given that the only classes > > > that we have there at the moment are those for handling DDL statements > it > > > doesn't bother me a lot. > > > > > > If people prefer to go with the "copy-paste" approach I am also fine > with > > > that but it is my least favorite. > > > > > > Best, > > > Stamatis > > > > > > > > > On Fri, May 15, 2020 at 10:34 PM Enrico Olivelli <eolive...@gmail.com> > > > wrote: > > > > > > > Il Ven 15 Mag 2020, 22:26 Julian Hyde <jh...@apache.org> ha scritto: > > > > > > > > > Currently, there is no DDL in Calcite's "core" module (or its SQL > > > > > parser) and the SQL parser in the "server" module adds DDL > extensions > > > > > for Calcite's object types. > > > > > > > > > > There is a PR [1][2] to add support for "CREATE TABLE" to Babel, > and > > > > > it makes "babel" extend the "server" module. In particular, it uses > > > > > classes SqlDdlNodes and SqlCreateTable from "server". > > > > > > > > > > I am uncomfortable with that approach, because "server" does other > > > > > things besides parse DDL (it creates a stateful server and handles > > > > > RPC). Also, the needs of other DBMSs' DDL might make Calcite's DDL > > > > > classes more complicated. So, it all seems to be unnecessary > coupling. > > > > > > > > > > The alternative is to copy-paste the DDL classes (currently > > > > > SqlDdlNodes, SqlCreateTable and SqlColumnDeclaration) from "babel" > > > > > into "server". > > > > > > > > > > Another alternative would be to move all DDL classes into "core". > > > > > > > > > > None of the alternatives are great, but I prefer the copy-paste > > > > > approach. What do y'all think? > > > > > > > > > > > > > It would be interesting for HerdDB community to have DDL in core > Calcite > > > > > > > > > > > > Enrico > > > > > > > > > > > > > Julian > > > > > > > > > > [1] https://github.com/apache/calcite/pull/1938 > > > > > [2] https://issues.apache.org/jira/browse/CALCITE-3946 > > > > > > > > > >
