Where did you see R in SqlFieldsQuery? Sergi
2016-07-27 12:59 GMT+03:00 Alexander Paschenko < alexander.a.pasche...@gmail.com>: > Sergi, > > But current signature of query() method returns not just some > iterator, but rather iterator of R which is type param of Query - > i.e., we won't be able to return an int inside a QueryCursor<R>. At > least without API change (signature of query() method will have to be > changed to drop genericness, or in some other weird way). Is this what > we really want? Or am I missing something in your point? > > - Alex > > 2016-07-27 12:51 GMT+03:00 Sergi Vladykin <sergi.vlady...@gmail.com>: > > 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 > >> >> > >>> >> > > > > >> >> > >>> >> > > > >> >> > >>> >> > > >> >> > >>> >> > >> >> > >>> > >> >> > >> > >> >> > >> > >> >> > > >> >> > >> >