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 <sergi.vlady...@gmail.com>:
> 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 <
> alexander.a.pasche...@gmail.com>:
>
>> 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 <
>> > alexander.a.pasche...@gmail.com <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 <dsetrak...@apache.org
>> > <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 <
>> > ptupit...@apache.org <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 <
>> > > > > > alexander.a.pasche...@gmail.com <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 <
>> > dsetrak...@apache.org <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 <
>> > > > > > > > alexander.a.pasche...@gmail.com <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
>> > > > > > > >>
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>

Reply via email to