Exactly. This will allow our Jdbc driver to work transparently. Sergi
2016-07-27 12:40 GMT+03:00 Alexander Paschenko < alexander.a.pasche...@gmail.com>: > Sergi, > > You wrote: > > I'd prefer to return the same information, so it will not be empty > > Do you mean return iterator with single element that denotes number of > rows? > > Dmitriy, > > You wrote: > > What is the ticket number for this. Is the new API documented there? > > Overall issue number is 2294. There's no particular issue on API > changes, but creating one seems to be a good idea, I will do it. > > - Alex > > 2016-07-27 9:20 GMT+03:00 Dmitriy Setrakyan <dsetrak...@apache.org>: > > What is the ticket number for this. Is the new API documented there? > > > > On Tue, Jul 26, 2016 at 11:36 AM, Sergi Vladykin < > sergi.vlady...@gmail.com> > > wrote: > > > >> I don't see anything ugly in empty iterator, sorry if I insulted your > taste > >> of beauty. > >> > >> If you will take a look at Jdbc, you will see that > Statement.executeUpdate > >> method returns number of updated rows, I'd prefer to return the same > >> information, so it will not be empty (beauty is restored!). > >> > >> Sergi > >> > >> > >> > >> 2016-07-26 18:24 GMT+03:00 Alexander Paschenko < > >> alexander.a.pasche...@gmail.com>: > >> > >> > I see your point. But what about my concerns from initial post? > >> > Particularly about signatures of existing methods? I personally don't > >> > like an option of query() method always returning an empty iterator > >> > for any non-select query, it seems ugly design wise. > >> > > >> > - Alex > >> > > >> > 2016-07-26 18:15 GMT+03:00 Sergi Vladykin <sergi.vlady...@gmail.com>: > >> > > BTW, the simplest way to solve this issue is to allow running SQL > >> > commands > >> > > inside of SqlFieldsQuery. > >> > > > >> > > We may add some additional convenience API for updates if we want, > but > >> > JDBC > >> > > client will always call it like this: > >> > > > >> > > cache.query(new SqlFieldsQuery("INSERT INTO MY_TABLE > >> > > VALUES(?,?)").setArgs(1,2)); > >> > > > >> > > This will resolve any ambiguity. > >> > > > >> > > Sergi > >> > > > >> > > 2016-07-26 17:56 GMT+03:00 Sergi Vladykin <sergi.vlady...@gmail.com > >: > >> > > > >> > >> 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.pasche...@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 < > dsetrak...@apache.org > >> >: > >> > >>> > OK, then using your analogy, the current behavior in Ignite is > >> MERGE > >> > for > >> > >>> > 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 > >> > >>> calls, > >> > >>> > as well as for the data streamer. > >> > >>> > > >> > >>> > Does this make sense? > >> > >>> > > >> > >>> > D. > >> > >>> > > >> > >>> > On Thu, Jul 21, 2016 at 4:06 AM, Sergi Vladykin < > >> > >>> sergi.vlady...@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 < > >> > >>> dsetrak...@apache.org> > >> > >>> >> wrote: > >> > >>> >> > >> > >>> >> > Serj, are you asking what will happen as of today? Then the > >> answer > >> > >>> to all > >> > >>> >> > your questions is that duplicate keys are not an issue, and > >> Ignite > >> > >>> always > >> > >>> >> > operates in **upsert** mode (which is essentially a *“put(…)” > >> > >>> *method). > >> > >>> >> > > >> > >>> >> > However, the *“insert”* that is suggested by Alex would > delegate > >> > to > >> > >>> >> > *“putIfAbsent(…)”*, which in database world makes more sense. > >> > >>> However, in > >> > >>> >> > this case, the *“update”* syntax should delegate to > >> > *“replace(…)”*, > >> > >>> as > >> > >>> >> > update should fail in case if a key is absent. > >> > >>> >> > > >> > >>> >> > Considering the above, a notion of “*upsert”* or “*merge” > >> > *operation > >> > >>> is > >> > >>> >> > very much needed, as it will give a user an option to perform > >> > >>> >> > “insert-or-update” in 1 call. > >> > >>> >> > > >> > >>> >> > Does this make sense? > >> > >>> >> > > >> > >>> >> > D. > >> > >>> >> > > >> > >>> >> > On Wed, Jul 20, 2016 at 9:39 PM, Sergi Vladykin < > >> > >>> >> sergi.vlady...@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 > >> > >>> all 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 < > >> > >>> >> > > dsetrak...@gridgain.com> > >> > >>> >> > > wrote: > >> > >>> >> > > > >> > >>> >> > > > Like the idea of merge and insert. I need more time to > think > >> > >>> about > >> > >>> >> the > >> > >>> >> > > API > >> > >>> >> > > > changes. > >> > >>> >> > > > > >> > >>> >> > > > Sergi, what do you think? > >> > >>> >> > > > > >> > >>> >> > > > Dmitriy > >> > >>> >> > > > > >> > >>> >> > > > > >> > >>> >> > > > > >> > >>> >> > > > On Jul 20, 2016, at 12:36 PM, Alexander Paschenko < > >> > >>> >> > > > alexander.a.pasche...@gmail.com> wrote: > >> > >>> >> > > > > >> > >>> >> > > > >> Thus, I suggest that we implement MERGE as a separate > >> > >>> operation > >> > >>> >> > 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.pasche...@gmail.com>: > >> > >>> >> > > > >> Hell Igniters, > >> > >>> >> > > > >> > >> > >>> >> > > > >> In this thread I would like to share and discuss some > >> > >>> thoughts 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 > >> > >>> dialect > >> > >>> >> has > >> > >>> >> > > > >> MERGE operation which corresponds to semantics of > cache > >> > put. > >> > >>> >> Thus, I > >> > >>> >> > > > >> suggest that we implement MERGE as a separate > operation > >> > >>> backed by > >> > >>> >> > > > >> putIfAbsent operation, while INSERT will be > implemented > >> via > >> > >>> put. > >> > >>> >> > > > >> > >> > >>> >> > > > >> And one more, probably more important thing: I suggest > >> > that we > >> > >>> >> > create > >> > >>> >> > > > >> separate class Update and corresponding operation > >> update() > >> > in > >> > >>> >> > > > >> IgniteCache. The reasons are as follows: > >> > >>> >> > > > >> > >> > >>> >> > > > >> - Query bears some flags that are clearly redundant > for > >> > Update > >> > >>> >> (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 > >> > >>> design > >> > >>> >> > > > >> 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 > >> > >>> >> > > > > >> > >>> >> > > > >> > >>> >> > > >> > >>> >> > >> > >>> > >> > >> > >> > >> > >> > > >> >