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 > > >>> >> > > > > > >>> >> > > > > >>> >> > > > >>> >> > > >>> > > >> > > >> > > >