Re: [DISCUSS] DDL parsing, and relationship between "server" and "babel" modules
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 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 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 > 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 > > > wrote: > > > > > > > Il Ven 15 Mag 2020, 22:26 Julian Hyde 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 > > > > > > > > > >
Re: [DISCUSS] DDL parsing, and relationship between "server" and "babel" modules
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 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 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 > > wrote: > > > > > Il Ven 15 Mag 2020, 22:26 Julian Hyde 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 > > > > > > >
Re: [DISCUSS] DDL parsing, and relationship between "server" and "babel" modules
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 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 > wrote: > > > Il Ven 15 Mag 2020, 22:26 Julian Hyde 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 > > > > >
Re: [DISCUSS] DDL parsing, and relationship between "server" and "babel" modules
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 wrote: > Il Ven 15 Mag 2020, 22:26 Julian Hyde 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 > > >
Re: [DISCUSS] DDL parsing, and relationship between "server" and "babel" modules
Il Ven 15 Mag 2020, 22:26 Julian Hyde 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 >
[DISCUSS] DDL parsing, and relationship between "server" and "babel" modules
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? Julian [1] https://github.com/apache/calcite/pull/1938 [2] https://issues.apache.org/jira/browse/CALCITE-3946
