Mailing-List: contact dev-help@ignite.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@ignite.apache.org
MIME-Version: 1.0
In-Reply-To: <CAJhSPbp1KO6mqL3nSaKCpwCgmOeyKwT4CdYveSoQ1292TXC6zw@mail.gmail.com>
References: <CAJhSPbriyMVto_GD1wQG0WXJJ09bUof2jju0hpH2_O1bzkdU1A@mail.gmail.com>
 <D77A1A7B-9864-447F-A1E2-1BF1BF418792@gridgain.com> <CA+eZwrF2YEZzAfB5u_7cyrqPrc4BNKBELT7Appw7pxNC3K8dSA@mail.gmail.com>
 <CA+0=VoX0ea6NySsM2qkX0kdVpeuivFX_ip9V=fdmC6-3=A=Oig@mail.gmail.com>
 <CA+eZwrF38d8RgaGQaibE9FnbM=KVPTEGTRSFeMaT=yPwnPLS=g@mail.gmail.com>
 <CA+0=VoVUdp6Bnoyj-TkAp1WruLyZKVVXvG9tz3kdPD-qvBNh2Q@mail.gmail.com> <CAJhSPbp1KO6mqL3nSaKCpwCgmOeyKwT4CdYveSoQ1292TXC6zw@mail.gmail.com>
From: Sergi Vladykin <sergi.vladykin@gmail.com>
Date: Tue, 26 Jul 2016 17:56:09 +0300
Message-ID: <CA+eZwrHK4kh=e4B6_NNvMxLeFHJcAt+F9D=t2+5vWJWemJdxgQ@mail.gmail.com>
Subject: Re: IGNITE-2294 implementation details
To: dev@ignite.apache.org
Content-Type: multipart/alternative; boundary=94eb2c1243d63cbfd405388b1cd5
archived-at: Tue, 26 Jul 2016 14:56:57 -0000

--94eb2c1243d63cbfd405388b1cd5
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

I don't like any pre-parsing, especially with some libraries other than H2.
H2 itself has enough quirks to multiply it on quirks of another library.

This is exactly what I was talking about - we need some single entry point
on API for all the SQL commands and queries. Thats why I suggested
SqlUpdate to extend Query. To me its is the cleanest approach. May be we
need to change in some backward compatible way this Query hierarchy to get
rid of extra methods but the idea is still the same.

Sergi

2016-07-26 14:34 GMT+03:00 Alexander Paschenko <
alexander.a.paschenko@gmail.com>:

> Guys,
>
> I would like to advance the discussion further. There's one quite
> important question that arose based on current state of work on this
> issue. If we use some kind of interactive console, like Visor, then
> how should it know whether SQL query it is requested to execute
> returns a result set or not? In JDBC world, solution is quite simple -
> there's base interface called Statement that all commands implement,
> and it has magic isResultSet method that tells whether statement is a
> query or an update command. The API proposed now has separate Query
> and Update operations which I believe to be a right thing by the
> reasons I outlined in the beginning of this thread. However, their
> lack of common ancestor prevents possible console clients from running
> text SQL commands in a fully transparent manner - like
> IgniteCache.execute(String sql). Therefore I see two possible ways of
> solving this:
>
> - we change API so that it includes new class or interface parenting
> both Query and Update, and clients use it to communicate with cache
> - we let (or make :) ) the client determine command type independently
> and behave accordingly - for it to work it will have some kind of
> command parsing by itself just to determine its type. Visor console
> may use simple library like JSqlParser
> (https://github.com/JSQLParser/JSqlParser; dual LGPL 2.1/ASF 2.0
> licensed) to determine request type in terms of JDBC, and behave
> accordingly.
>
> Personally, I think that the second approach is better - and here's why.
>
> First, it does not seem wise to change API simply to make console (or
> any other) clients simpler. Programmatic APIs should be concise and
> short for programmatic use, console clients should be easy to use from
> console - and that's it: after all, console client exists to free a
> user from burden of doing things programmatically, so its aim is to
> adapt API to console or whatever UI.
> Second, possible complications in client implied by such approach
> certainly won't be dramatic - I don't think that additional single
> query parsing operation in client code will make it much harder to
> develop.
> Third, as I see it now, adding a new "synthetic" entity and new method
> would take more effort to adapting the client to new API.
>
> Dmitry, Sergi, I would like to hear what you think about it all. Thanks.
>
> - Alex
>
> 2016-07-21 21:17 GMT+03:00 Dmitriy Setrakyan <dsetrakyan@apache.org>:
> > OK, then using your analogy, the current behavior in Ignite is MERGE fo=
r
> > the most part.
> >
> > My preference is that Ignite SQL should work no different from
> traditional
> > databases, which means:
> >
> > - INSERT is translated into *putIfAbsent()* call in Ignite
> > - UPDATE is translated into *replace()* call in Ignite
> > - MERGE is translated into *put()* call in Ignite
> > - For SQL BATCH calls we should delegate to Ignite batch operations, e.=
g.
> > *putAll()*
> >
> > The above should hold true for atomic and transactional put/putAll call=
s,
> > as well as for the data streamer.
> >
> > Does this make sense?
> >
> > D.
> >
> > On Thu, Jul 21, 2016 at 4:06 AM, Sergi Vladykin <
> sergi.vladykin@gmail.com>
> > wrote:
> >
> >> No, this does not make sense.
> >>
> >> There is no upsert mode in databases. There are operations: INSERT,
> UPDATE,
> >> DELETE, MERGE.
> >>
> >> I want to have clear understanding of how they have to behave in SQL
> >> databases and how they will actually behave in Ignite in different
> >> scenarios. Also I want to have clear understanding of performance
> >> implications of each decision here.
> >>
> >> Anything wrong with that?
> >>
> >> Sergi
> >>
> >> On Thu, Jul 21, 2016 at 1:04 PM, Dmitriy Setrakyan <
> dsetrakyan@apache.org>
> >> wrote:
> >>
> >> > Serj, are you asking what will happen as of today? Then the answer t=
o
> all
> >> > your questions is that duplicate keys are not an issue, and Ignite
> always
> >> > operates in **upsert** mode (which is essentially a *=E2=80=9Cput(=
=E2=80=A6)=E2=80=9D
> *method).
> >> >
> >> > However, the *=E2=80=9Cinsert=E2=80=9D* that is suggested by Alex wo=
uld delegate to
> >> > *=E2=80=9CputIfAbsent(=E2=80=A6)=E2=80=9D*, which in database world =
makes more sense.
> However, in
> >> > this case, the *=E2=80=9Cupdate=E2=80=9D* syntax should delegate to =
*=E2=80=9Creplace(=E2=80=A6)=E2=80=9D*, as
> >> > update should fail in case if a key is absent.
> >> >
> >> > Considering the above, a notion of =E2=80=9C*upsert=E2=80=9D* or =E2=
=80=9C*merge=E2=80=9D *operation
> is
> >> > very much needed, as it will give a user an option to perform
> >> > =E2=80=9Cinsert-or-update=E2=80=9D in 1 call.
> >> >
> >> > Does this make sense?
> >> >
> >> > D.
> >> >
> >> > On Wed, Jul 20, 2016 at 9:39 PM, Sergi Vladykin <
> >> sergi.vladykin@gmail.com>
> >> > wrote:
> >> >
> >> > > I'd prefer to do MERGE operation last because in H2 it is not
> standard
> >> > ANSI
> >> > > SQL MERGE. Or may be not implement it at all, or may be contribute
> ANSI
> >> > > correct version to H2, then implement it on Ignite. Need to
> investigate
> >> > the
> >> > > semantics deeper before making any decisions here.
> >> > >
> >> > > Lets start with simple scenarios for INSERT and go through all the
> >> > possible
> >> > > cases and answer the questions:
> >> > > - What will happen on key conflict in TX cache?
> >> > > - What will happen on key conflict in Atomic cache?
> >> > > - What will happen with the previous two if we use DataLoader?
> >> > > - How to make these operations efficient (it will be simple enough
> to
> >> > > implement them with separate put/putIfAbsent operations but
> probably we
> >> > > will need some batching like putAllIfAbsent for efficiency)?
> >> > >
> >> > > As for API, we still will need to have a single entry point for al=
l
> SQL
> >> > > queries/commands to allow any console work with it transparently. =
It
> >> > would
> >> > > be great if we will be able to come up with something consistent
> with
> >> > this
> >> > > idea on public API.
> >> > >
> >> > > Sergi
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> > > On Wed, Jul 20, 2016 at 2:23 PM, Dmitriy Setrakyan <
> >> > > dsetrakyan@gridgain.com>
> >> > > wrote:
> >> > >
> >> > > > Like the idea of merge and insert. I need more time to think abo=
ut
> >> the
> >> > > API
> >> > > > changes.
> >> > > >
> >> > > > Sergi, what do you think?
> >> > > >
> >> > > > Dmitriy
> >> > > >
> >> > > >
> >> > > >
> >> > > > On Jul 20, 2016, at 12:36 PM, Alexander Paschenko <
> >> > > > alexander.a.paschenko@gmail.com> wrote:
> >> > > >
> >> > > > >> Thus, I suggest that we implement MERGE as a separate operati=
on
> >> > backed
> >> > > > by putIfAbsent operation, while INSERT will be implemented via
> put.
> >> > > > >
> >> > > > > Sorry, of course I meant that MERGE has to be put-based, while
> >> INSERT
> >> > > > > has to be putIfAbsent-based.
> >> > > > >
> >> > > > > 2016-07-20 12:30 GMT+03:00 Alexander Paschenko
> >> > > > > <alexander.a.paschenko@gmail.com>:
> >> > > > >> Hell Igniters,
> >> > > > >>
> >> > > > >> In this thread I would like to share and discuss some thought=
s
> on
> >> > DML
> >> > > > >> operations' implementation, so let's start and keep it here.
> >> > Everyone
> >> > > > >> is of course welcome to share their suggestions.
> >> > > > >>
> >> > > > >> For starters, I was thinking about semantics of INSERT. In
> >> > traditional
> >> > > > >> RDBMSs, INSERT works only for records whose primary keys don'=
t
> >> > > > >> conflict with those of records that are already persistent -
> you
> >> > can't
> >> > > > >> try to insert the same key more than once because you'll get =
an
> >> > error.
> >> > > > >> However, semantics of cache put is obviously different - it
> does
> >> not
> >> > > > >> have anything about duplicate keys, it just quietly updates
> values
> >> > in
> >> > > > >> case of keys' duplication. Still, cache has putIfAbsent
> operation
> >> > that
> >> > > > >> is closer to traditional notion of INSERT, and H2's SQL diale=
ct
> >> has
> >> > > > >> MERGE operation which corresponds to semantics of cache put.
> >> Thus, I
> >> > > > >> suggest that we implement MERGE as a separate operation backe=
d
> by
> >> > > > >> putIfAbsent operation, while INSERT will be implemented via
> put.
> >> > > > >>
> >> > > > >> And one more, probably more important thing: I suggest that w=
e
> >> > create
> >> > > > >> separate class Update and corresponding operation update() in
> >> > > > >> IgniteCache. The reasons are as follows:
> >> > > > >>
> >> > > > >> - Query bears some flags that are clearly redundant for Updat=
e
> >> (page
> >> > > > >> size, locality)
> >> > > > >> - query() method in IgniteCache (one that accepts Query) and
> >> query()
> >> > > > >> methods in GridQueryIndexing return iterators. So, if we
> strive to
> >> > > > >> leave interfaces unchanged, we still will introduce some desi=
gn
> >> > > > >> ugliness like query methods returning empty iterators for
> certain
> >> > > > >> queries, and/or query flags that indicate whether it's an
> update
> >> > query
> >> > > > >> or not, etc.
> >> > > > >> - If some Queries are update queries, then continuous queries
> >> can't
> >> > be
> >> > > > >> based on them - more design-wise ugly checks and stuff like
> that.
> >> > > > >> - I'm pretty sure there's more I don't know about.
> >> > > > >>
> >> > > > >> Comments and suggestions are welcome. Sergi Vladykin, Dmitry
> >> > > > >> Setrakyan, your opinions are of particular interest, please
> >> advise.
> >> > > > >>
> >> > > > >> Regards,
> >> > > > >> Alex
> >> > > >
> >> > >
> >> >
> >>
>

--94eb2c1243d63cbfd405388b1cd5--