If it is that little, then all this copy/paste shit-coding makes no sense. We have to add a respective mode to H2, add respective tests to H2, so that other contributors of H2 will not occasionally break our stuff. Thats it.
I will be the first H2 committer who will reject you patch, don't waste your time. Sergi 2017-04-12 16:33 GMT+03:00 Alexander Paschenko < [email protected]>: > Sergi, > > First, it would be as little as overriding the part responsible for > CREATE TABLE - there's no need to touch anything else as luckily H2 > parser is internally structured well enough. > > Second, although it is not all-around perfect, I am most confident > that this is far better than dragging into H2 bunch of stuff that they > don't really need just because we need it there or can smug it there. > > I think I'll just spend some time in the weekend and come up with a > prototype as otherwise this talk seems to be just a chit-chat. > > - Alex > > 2017-04-12 14:38 GMT+03:00 Sergi Vladykin <[email protected]>: > > So basically in inherited class you are going co copy/paste base class > > methods and tweak them? I don't like this approach. > > > > Sergi > > > > 2017-04-12 14:07 GMT+03:00 Alexander Paschenko < > > [email protected]>: > > > >> Sergi, > >> > >> As I've written in my previous post, it would be just inheriting Parser > on > >> Ignite side and plugging its instance in SINGLE place. Just making H2's > >> Parser internal methods protected instead of private would let us do the > >> trick. > >> > >> — Alex > >> > >> среда, 12 апреля 2017 г. пользователь Sergi Vladykin написал: > >> > >> > I don't see how you make H2 Parser extendable, you will have to add > >> plugin > >> > call to every *potentially* extendable place in it. In general this > does > >> > not work. As H2 guy I would also reject patch like this. > >> > > >> > Sergi > >> > > >> > 2017-04-12 13:10 GMT+03:00 Alexander Paschenko < > >> > [email protected] <javascript:;>>: > >> > > >> > > Sergi, > >> > > > >> > > Please have a closer look at what I've written in my first post. I > >> don't > >> > > see why we have to cling to H2 and its parsing modes all the time — > >> after > >> > > all, we're just talking string processing now, aren't we? (Yes, > complex > >> > and > >> > > non trivial, but still.) > >> > > > >> > > What's wrong with idea of patching H2 to allow custom parsing? (With > >> the > >> > > parsing itself living in Ignite code, obviously, not in H2.). > >> > > > >> > > What I propose is just to make H2's Parser class extendable and > make H2 > >> > > aware of its descendants via config params. And that's all with > respect > >> > to > >> > > H2, nothing more. > >> > > > >> > > After that, on Ignite side we do all we want with our parser based > on > >> > > theirs. It resembles story with custom types — first we make H2 > >> > extendable > >> > > in the way we need, then we introduce exact features we need on > Ignite > >> > > side. > >> > > > >> > > — Alex > >> > > > >> > > среда, 12 апреля 2017 г. пользователь Sergi Vladykin написал: > >> > > > >> > > > It definitely makes sense to add a separate mode for Ignite in H2. > >> > Though > >> > > > it is wrong to think that it will allow us to add any crazy > syntax we > >> > > want > >> > > > (and it is actually a wrong idea imo), only the minor variations > of > >> the > >> > > > existing syntax. But this must be enough. > >> > > > > >> > > > I believe we should end up with something like > >> > > > > >> > > > CREATE TABLE person > >> > > > ( > >> > > > id INT PRIMARY KEY, > >> > > > orgId INT AFFINITY KEY, > >> > > > name VARCHAR > >> > > > ) > >> > > > WITH "cfg:my_config_template.xml" > >> > > > > >> > > > Sergi > >> > > > > >> > > > > >> > > > 2017-04-12 7:54 GMT+03:00 Dmitriy Setrakyan < > [email protected] > >> > <javascript:;> > >> > > > <javascript:;>>: > >> > > > > >> > > > > Agree, the updated syntax looks better. One change though: KEY > -> > >> > > PRIMARY > >> > > > > KEY. > >> > > > > > >> > > > > Sergi, what do you think? > >> > > > > > >> > > > > D. > >> > > > > > >> > > > > On Tue, Apr 11, 2017 at 9:50 PM, Pavel Tupitsyn < > >> > [email protected] <javascript:;> > >> > > > <javascript:;>> > >> > > > > wrote: > >> > > > > > >> > > > > > I think "WITH" syntax is ugly and cumbersome. > >> > > > > > > >> > > > > > We should go with this one: > >> > > > > > CREATE TABLE Person (id int AFFINITY KEY, uid uuid KEY, > firstName > >> > > > > > varchar, lastName varchar) > >> > > > > > > >> > > > > > All databases (i.e. [1], [2]) work this way, I see no reason > to > >> > > invent > >> > > > > > something different and confuse the users. > >> > > > > > > >> > > > > > [1] > >> > > > > > https://docs.microsoft.com/en-us/sql/t-sql/statements/create > >> > > > > > -table-transact-sql#syntax-1 > >> > > > > > [2] https://www.postgresql.org/docs/9.1/static/sql- > >> > createtable.html > >> > > > > > > >> > > > > > On Wed, Apr 12, 2017 at 6:12 AM, Alexander Paschenko < > >> > > > > > [email protected] <javascript:;> > <javascript:;>> > >> > wrote: > >> > > > > > > >> > > > > > > Dmitry, > >> > > > > > > > >> > > > > > > For H2 it would be something like this - please note all > those > >> > > > quotes, > >> > > > > > > commas and equality signs that would be mandatory: > >> > > > > > > > >> > > > > > > CREATE TABLE Person (id int, uid uuid, firstName varchar, > >> > lastName > >> > > > > > > varchar) WITH "keyFields=id,uuid","affinityKey=id" > >> > > > > > > > >> > > > > > > With suggested approach, it would be something like > >> > > > > > > > >> > > > > > > CREATE TABLE Person (id int AFFINITY KEY, uid uuid KEY, > >> firstName > >> > > > > > > varchar, lastName varchar) > >> > > > > > > > >> > > > > > > While this may not look like a drastic improvement in this > >> > > particular > >> > > > > > > case, we someday most likely will want either an all-custom > >> > CREATE > >> > > > > > > CACHE command, or a whole bunch of new options for CREATE > >> TABLE, > >> > if > >> > > > we > >> > > > > > > decide not to go with CREATE CACHE - I personally think that > >> > stuff > >> > > > > > > like > >> > > > > > > > >> > > > > > > CREATE TABLE ... WITH > >> > > > > > > "keyFields=id,uuid","affinityKey=id","cacheType= > >> > > > > partitioned","atomicity= > >> > > > > > > atomic","partitions=3" > >> > > > > > > > >> > > > > > > which will arise if we continue to try to stuff everything > into > >> > > WITH > >> > > > > > > will just bring more ugliness with time, and that's not to > >> > mention > >> > > > > > > that new CREATE CACHE syntax will be impossible or > relatively > >> > hard > >> > > to > >> > > > > > > introduce as we will have to approve it with H2 folks, and > >> that's > >> > > how > >> > > > > > > it will be with any new param or command that we want. > >> > > > > > > > >> > > > > > > Allowing to plug custom parser into H2 (as we do now with > table > >> > > > > > > engine) will let us introduce any syntax we want and focus > on > >> > > > > > > usability and not on compromises and workarounds (which WITH > >> > > keyword > >> > > > > > > currently is). > >> > > > > > > > >> > > > > > > - Alex > >> > > > > > > > >> > > > > > > 2017-04-12 5:11 GMT+03:00 Dmitriy Setrakyan < > >> > [email protected] <javascript:;> > >> > > > <javascript:;>>: > >> > > > > > > > Alexeander, > >> > > > > > > > > >> > > > > > > > Can you please provide an example of what the CREATE TABLE > >> > > command > >> > > > > > would > >> > > > > > > > look like if we use WITH syntax from H2 vs. what you are > >> > > proposing? > >> > > > > > > > > >> > > > > > > > D. > >> > > > > > > > > >> > > > > > > > On Tue, Apr 11, 2017 at 6:35 PM, Alexander Paschenko < > >> > > > > > > > [email protected] <javascript:;> > >> > <javascript:;>> wrote: > >> > > > > > > > > >> > > > > > > >> Hello Igniters, > >> > > > > > > >> > >> > > > > > > >> Yup, it's THAT time once again as we haven't ultimately > >> > settled > >> > > on > >> > > > > > > >> anything with the subj. as of yet, but I believe that now > >> with > >> > > DDL > >> > > > > on > >> > > > > > > >> its way this talk can't be avoided anymore (sorry guys). > >> > > > > > > >> > >> > > > > > > >> The last time we talked about Ignite specific stuff we > need > >> to > >> > > > have > >> > > > > in > >> > > > > > > >> CREATE TABLE (key fields list, affinity key, am I missing > >> > > > > anything?), > >> > > > > > > >> the simplest approach suggested by Sergi was that we > simply > >> > use > >> > > > WITH > >> > > > > > > >> part of H2's CREATE TABLE to pass stuff we need. > >> > > > > > > >> > >> > > > > > > >> This could work, but needless to say that such commands > >> would > >> > > look > >> > > > > > plain > >> > > > > > > >> ugly. > >> > > > > > > >> > >> > > > > > > >> I think we should go with custom syntax after all, BUT > not > >> in > >> > a > >> > > > way > >> > > > > > > >> suggested before by Sergi (propose Apache Ignite mode to > >> H2). > >> > > > > > > >> > >> > > > > > > >> Instead, I suggest that we propose to H2 patch that would > >> > allow > >> > > > > > > >> plugging in *custom SQL parser* directly based on theirs > >> > (quite > >> > > > > > > >> elegant one) – I've had a look at their code, and this > >> should > >> > > not > >> > > > be > >> > > > > > > >> hard. > >> > > > > > > >> > >> > > > > > > >> Work on such a patch making syntax parsing overridable > would > >> > > take > >> > > > a > >> > > > > > > >> couple days which is not much time AND would give us the > >> > > > opportunity > >> > > > > > > >> to introduce to Ignite virtually any syntax we wish - > both > >> now > >> > > and > >> > > > > in > >> > > > > > > >> the future. Without worrying about compatibility with H2 > >> ever > >> > > > again, > >> > > > > > > >> that is. > >> > > > > > > >> > >> > > > > > > >> Thoughts? After we agree on this principally and after H2 > >> > patch > >> > > > for > >> > > > > > > >> custom parsing is ready, we can roll our sleeves and > focus > >> on > >> > > > syntax > >> > > > > > > >> itself. > >> > > > > > > >> > >> > > > > > > >> - Alex > >> > > > > > > >> > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> >
