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 Thu, 13 Apr 2017 06:22:21 GMT
I do not think we need it.

In standard SQL we already have KEY and COLUMN, also we already have CREATE
TABLE syntax. Adding AFFINITY to them is not a big deal.

The thing CONFIGURATION looks like a completely new entity for SQL and I
prefer to avoid sticking it into H2, also I would avoid having it in Ignite.

If we need to create cache configuration templates in SQL, then lets use
functions:

CALL NEW_CACHE_CONFIGURATION(...);

They will be completely independent from H2. The only problem is that (as
we already discussed time ago) that for better usability we may need to
contribute named parameters for H2 functions. But this is the right thing
to do here.

Sergi

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

> Got it. Can we also add CONFIGURATION keyword?
>
> On Wed, Apr 12, 2017 at 11:34 AM, Sergi Vladykin <sergi.vladykin@gmail.com
> >
> wrote:
>
> > 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