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