ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sergi Vladykin <sergi.vlady...@gmail.com>
Subject Re: CREATE TABLE SQL command syntax
Date Wed, 12 Apr 2017 18:34:43 GMT
Dmitriy,

H2 does not support any "user-specific" syntax and it should not. Instead
it has a concept of Mode, which is basically a setting which allows H2 to
be compatible with other databases. For example, some keywords that make
sense for other databases are just ignored, but this makes the statement
from other BD work in H2.

It allows us to introduce "ApacheIgnite" mode, which will allow to add some
minor tweaks into Parser. These tweaks will be covered by tests and no one
will be able to just silently break our code.

Actually what I see is that we do not need any custom parsing at all, all
we need is just need a couple of minor tweaks (like AFFINITY keyword),
other SQL must work as is. Thus trying to plug in a parser looks like an
overkill and fragile idea a priori.

Sergi

2017-04-12 20:40 GMT+03:00 Dmitriy Setrakyan <dsetrakyan@apache.org>:

> Hm... I think the truth is somewhere in the middle here.
>
> The syntax proposed by Sergi makes sense to me. However, I am still
> struggling why would H2 accept our patch, if it has AFFINITY KEY keyword in
> it, which has nothing to do with H2.
>
> It does sound like certain portions of SQL do need to be plugable to
> support the user-specific syntax.
>
> Sergi, am I missing something?
>
> D.
>
> On Wed, Apr 12, 2017 at 8:51 AM, Sergi Vladykin <sergi.vladykin@gmail.com>
> wrote:
>
> > 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 <
> > alexander.a.paschenko@gmail.com>:
> >
> > > 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.vladykin@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.paschenko@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.paschenko@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 <
> > > dsetrakyan@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
<
> > > >> > ptupitsyn@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.paschenko@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
<
> > > >> > dsetrakyan@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.paschenko@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
> > > >> > > > > > > >>
> > > >> > > > > > >
> > > >> > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message