Re: IGNITE-2294 implementation details

[Alexander Paschenko]

Dmitriy,

I've updated the issue with current state of work and proposal for
data streamer use, please have a look/advise.

- Alex

2016-08-04 21:16 GMT+03:00 Alexander Paschenko
:
> Sergi,
>
> OK, all fixed, there's no trace of update operations in public API now, 
> thanks.
> Will update doc and issue shortly.
>
> - Alex
>
> 2016-08-04 12:32 GMT+03:00 Sergi Vladykin :
>> Ok, it is not a problem for us if we will not fail fast on wrong number
>> of arguments, but will fail on the first query execution. Lets just drop
>> this argument counting.
>>
>> We should not show SqlFieldsQuery.isQuery on public API if it is useless
>> for the end users. If this stuff is needed for Jdbc we have to find a way
>> to make it private.
>>
>> Sergi
>>
>> 2016-08-04 11:42 GMT+03:00 Alexander Paschenko <
>> alexander.a.pasche...@gmail.com>:
>>
>>> Sergi,
>>>
>>> > Why do we need to count query arguments? Can anyone clarify?
>>> Say, to make parameter index checks early like all major vendors do.
>>> That's why they are counted now.
>>>
>>> > Also about new public APIs. I don't see why we need
>>> SqlFieldsQuery.isQuery,
>>> > looks like it has to be optional, so it will only confuse users.
>>> It _is_ optional. And why I believe we need this flag is said in its
>>> javadoc as well as google doc I've provided link to. Again, I think
>>> that it's better to check user input early. In this case it is
>>> correspondence of expected result, be it result set or update counter,
>>> to the type of SQL query given. I honestly don't like an idea of
>>> sending a request for execution to cluster and then throwing an
>>> exception when we see that (already computed) result does not match
>>> what we expected. So checking query type _before_ it is executed
>>> against _optional_ flag (set by JDBC driver) could help.
>>>
>>> > QueryCursor.isResultSet makes sense only for SqlFieldsQuery only in Jdbc.
>>> Thanks, will fix it.
>>>
>>> - Alex
>>>
>>> 2016-08-04 9:43 GMT+03:00 Sergi Vladykin :
>>> > About new public APIs 2.
>>> >
>>> > QueryCursor.isResultSet makes sense only for SqlFieldsQuery only in Jdbc.
>>> > Thus it must be private on QueryCursorEx like fieldsMeta() for example.
>>> >
>>> > All in all, there should be no changes on public API at this point.
>>> >
>>> > Sergi
>>> >
>>> > 2016-08-04 9:05 GMT+03:00 Sergi Vladykin :
>>> >
>>> >> Also about new public APIs. I don't see why we need
>>> >> SqlFieldsQuery.isQuery, looks like it has to be optional, so it will
>>> only
>>> >> confuse users.
>>> >>
>>> >> Sergi
>>> >>
>>> >> 2016-08-04 9:00 GMT+03:00 Sergi Vladykin :
>>> >>
>>> >>> Why do we need to count query arguments? Can anyone clarify?
>>> >>>
>>> >>> Sergi
>>> >>>
>>> >>> 2016-08-04 5:07 GMT+03:00 Alexey Kuznetsov :
>>> >>>
>>>  About arguments number. I see following options here:
>>>  1. Somehow use H2 engine to parse SQL string and check results.
>>>  2. Use some other parsing library instead  of H2 but this will bring
>>> one
>>>  more dependency to Ignite.
>>>  3. Write some simple parser by ourselves .
>>> 
>>> >>>
>>> >>>
>>> >>
>>>

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

Sergi,

OK, all fixed, there's no trace of update operations in public API now, thanks.
Will update doc and issue shortly.

- Alex

2016-08-04 12:32 GMT+03:00 Sergi Vladykin :
> Ok, it is not a problem for us if we will not fail fast on wrong number
> of arguments, but will fail on the first query execution. Lets just drop
> this argument counting.
>
> We should not show SqlFieldsQuery.isQuery on public API if it is useless
> for the end users. If this stuff is needed for Jdbc we have to find a way
> to make it private.
>
> Sergi
>
> 2016-08-04 11:42 GMT+03:00 Alexander Paschenko <
> alexander.a.pasche...@gmail.com>:
>
>> Sergi,
>>
>> > Why do we need to count query arguments? Can anyone clarify?
>> Say, to make parameter index checks early like all major vendors do.
>> That's why they are counted now.
>>
>> > Also about new public APIs. I don't see why we need
>> SqlFieldsQuery.isQuery,
>> > looks like it has to be optional, so it will only confuse users.
>> It _is_ optional. And why I believe we need this flag is said in its
>> javadoc as well as google doc I've provided link to. Again, I think
>> that it's better to check user input early. In this case it is
>> correspondence of expected result, be it result set or update counter,
>> to the type of SQL query given. I honestly don't like an idea of
>> sending a request for execution to cluster and then throwing an
>> exception when we see that (already computed) result does not match
>> what we expected. So checking query type _before_ it is executed
>> against _optional_ flag (set by JDBC driver) could help.
>>
>> > QueryCursor.isResultSet makes sense only for SqlFieldsQuery only in Jdbc.
>> Thanks, will fix it.
>>
>> - Alex
>>
>> 2016-08-04 9:43 GMT+03:00 Sergi Vladykin :
>> > About new public APIs 2.
>> >
>> > QueryCursor.isResultSet makes sense only for SqlFieldsQuery only in Jdbc.
>> > Thus it must be private on QueryCursorEx like fieldsMeta() for example.
>> >
>> > All in all, there should be no changes on public API at this point.
>> >
>> > Sergi
>> >
>> > 2016-08-04 9:05 GMT+03:00 Sergi Vladykin :
>> >
>> >> Also about new public APIs. I don't see why we need
>> >> SqlFieldsQuery.isQuery, looks like it has to be optional, so it will
>> only
>> >> confuse users.
>> >>
>> >> Sergi
>> >>
>> >> 2016-08-04 9:00 GMT+03:00 Sergi Vladykin :
>> >>
>> >>> Why do we need to count query arguments? Can anyone clarify?
>> >>>
>> >>> Sergi
>> >>>
>> >>> 2016-08-04 5:07 GMT+03:00 Alexey Kuznetsov :
>> >>>
>>  About arguments number. I see following options here:
>>  1. Somehow use H2 engine to parse SQL string and check results.
>>  2. Use some other parsing library instead  of H2 but this will bring
>> one
>>  more dependency to Ignite.
>>  3. Write some simple parser by ourselves .
>> 
>> >>>
>> >>>
>> >>
>>

Re: IGNITE-2294 implementation details

[Sergi Vladykin]

Ok, it is not a problem for us if we will not fail fast on wrong number
of arguments, but will fail on the first query execution. Lets just drop
this argument counting.

We should not show SqlFieldsQuery.isQuery on public API if it is useless
for the end users. If this stuff is needed for Jdbc we have to find a way
to make it private.

Sergi

2016-08-04 11:42 GMT+03:00 Alexander Paschenko <
alexander.a.pasche...@gmail.com>:

> Sergi,
>
> > Why do we need to count query arguments? Can anyone clarify?
> Say, to make parameter index checks early like all major vendors do.
> That's why they are counted now.
>
> > Also about new public APIs. I don't see why we need
> SqlFieldsQuery.isQuery,
> > looks like it has to be optional, so it will only confuse users.
> It _is_ optional. And why I believe we need this flag is said in its
> javadoc as well as google doc I've provided link to. Again, I think
> that it's better to check user input early. In this case it is
> correspondence of expected result, be it result set or update counter,
> to the type of SQL query given. I honestly don't like an idea of
> sending a request for execution to cluster and then throwing an
> exception when we see that (already computed) result does not match
> what we expected. So checking query type _before_ it is executed
> against _optional_ flag (set by JDBC driver) could help.
>
> > QueryCursor.isResultSet makes sense only for SqlFieldsQuery only in Jdbc.
> Thanks, will fix it.
>
> - Alex
>
> 2016-08-04 9:43 GMT+03:00 Sergi Vladykin :
> > About new public APIs 2.
> >
> > QueryCursor.isResultSet makes sense only for SqlFieldsQuery only in Jdbc.
> > Thus it must be private on QueryCursorEx like fieldsMeta() for example.
> >
> > All in all, there should be no changes on public API at this point.
> >
> > Sergi
> >
> > 2016-08-04 9:05 GMT+03:00 Sergi Vladykin :
> >
> >> Also about new public APIs. I don't see why we need
> >> SqlFieldsQuery.isQuery, looks like it has to be optional, so it will
> only
> >> confuse users.
> >>
> >> Sergi
> >>
> >> 2016-08-04 9:00 GMT+03:00 Sergi Vladykin :
> >>
> >>> Why do we need to count query arguments? Can anyone clarify?
> >>>
> >>> Sergi
> >>>
> >>> 2016-08-04 5:07 GMT+03:00 Alexey Kuznetsov :
> >>>
>  About arguments number. I see following options here:
>  1. Somehow use H2 engine to parse SQL string and check results.
>  2. Use some other parsing library instead  of H2 but this will bring
> one
>  more dependency to Ignite.
>  3. Write some simple parser by ourselves .
> 
> >>>
> >>>
> >>
>

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

Sergi,

> Why do we need to count query arguments? Can anyone clarify?
Say, to make parameter index checks early like all major vendors do.
That's why they are counted now.

> Also about new public APIs. I don't see why we need SqlFieldsQuery.isQuery,
> looks like it has to be optional, so it will only confuse users.
It _is_ optional. And why I believe we need this flag is said in its
javadoc as well as google doc I've provided link to. Again, I think
that it's better to check user input early. In this case it is
correspondence of expected result, be it result set or update counter,
to the type of SQL query given. I honestly don't like an idea of
sending a request for execution to cluster and then throwing an
exception when we see that (already computed) result does not match
what we expected. So checking query type _before_ it is executed
against _optional_ flag (set by JDBC driver) could help.

> QueryCursor.isResultSet makes sense only for SqlFieldsQuery only in Jdbc.
Thanks, will fix it.

- Alex

2016-08-04 9:43 GMT+03:00 Sergi Vladykin :
> About new public APIs 2.
>
> QueryCursor.isResultSet makes sense only for SqlFieldsQuery only in Jdbc.
> Thus it must be private on QueryCursorEx like fieldsMeta() for example.
>
> All in all, there should be no changes on public API at this point.
>
> Sergi
>
> 2016-08-04 9:05 GMT+03:00 Sergi Vladykin :
>
>> Also about new public APIs. I don't see why we need
>> SqlFieldsQuery.isQuery, looks like it has to be optional, so it will only
>> confuse users.
>>
>> Sergi
>>
>> 2016-08-04 9:00 GMT+03:00 Sergi Vladykin :
>>
>>> Why do we need to count query arguments? Can anyone clarify?
>>>
>>> Sergi
>>>
>>> 2016-08-04 5:07 GMT+03:00 Alexey Kuznetsov :
>>>
 About arguments number. I see following options here:
 1. Somehow use H2 engine to parse SQL string and check results.
 2. Use some other parsing library instead  of H2 but this will bring one
 more dependency to Ignite.
 3. Write some simple parser by ourselves .

>>>
>>>
>>

Re: IGNITE-2294 implementation details

[Sergi Vladykin]

About new public APIs 2.

QueryCursor.isResultSet makes sense only for SqlFieldsQuery only in Jdbc.
Thus it must be private on QueryCursorEx like fieldsMeta() for example.

All in all, there should be no changes on public API at this point.

Sergi

2016-08-04 9:05 GMT+03:00 Sergi Vladykin :

> Also about new public APIs. I don't see why we need
> SqlFieldsQuery.isQuery, looks like it has to be optional, so it will only
> confuse users.
>
> Sergi
>
> 2016-08-04 9:00 GMT+03:00 Sergi Vladykin :
>
>> Why do we need to count query arguments? Can anyone clarify?
>>
>> Sergi
>>
>> 2016-08-04 5:07 GMT+03:00 Alexey Kuznetsov :
>>
>>> About arguments number. I see following options here:
>>> 1. Somehow use H2 engine to parse SQL string and check results.
>>> 2. Use some other parsing library instead  of H2 but this will bring one
>>> more dependency to Ignite.
>>> 3. Write some simple parser by ourselves .
>>>
>>
>>
>

Re: IGNITE-2294 implementation details

[Sergi Vladykin]

Also about new public APIs. I don't see why we need SqlFieldsQuery.isQuery,
looks like it has to be optional, so it will only confuse users.

Sergi

2016-08-04 9:00 GMT+03:00 Sergi Vladykin :

> Why do we need to count query arguments? Can anyone clarify?
>
> Sergi
>
> 2016-08-04 5:07 GMT+03:00 Alexey Kuznetsov :
>
>> About arguments number. I see following options here:
>> 1. Somehow use H2 engine to parse SQL string and check results.
>> 2. Use some other parsing library instead  of H2 but this will bring one
>> more dependency to Ignite.
>> 3. Write some simple parser by ourselves .
>>
>
>

Re: IGNITE-2294 implementation details

[Sergi Vladykin]

Why do we need to count query arguments? Can anyone clarify?

Sergi

2016-08-04 5:07 GMT+03:00 Alexey Kuznetsov :

> About arguments number. I see following options here:
> 1. Somehow use H2 engine to parse SQL string and check results.
> 2. Use some other parsing library instead  of H2 but this will bring one
> more dependency to Ignite.
> 3. Write some simple parser by ourselves .
>

Re: IGNITE-2294 implementation details

[Alexey Kuznetsov]

About arguments number. I see following options here:
1. Somehow use H2 engine to parse SQL string and check results.
2. Use some other parsing library instead  of H2 but this will bring one
more dependency to Ignite.
3. Write some simple parser by ourselves .

Re: IGNITE-2294 implementation details

[Valentin Kulichenko]

Alex,

See my comments below.

-Val

On Wed, Aug 3, 2016 at 5:47 AM, Alexander Paschenko <
alexander.a.pasche...@gmail.com> wrote:

> Guys,
>
> I have few questions about current state of the art regarding JDBC driver.
>
> First: in JdbcPreparedStatement, we determine number of arguments
> simply by counting question mark symbols. What if query string
> contains a string literal with question mark in it? It seems like a
> bug to me.
>

Agree that it looks like a bug, but I'm not sure how we can fix it. Do you
have any ideas?


>
> Second: in JdbcStatement.executeQuery we null 'rs' field, but don't
> set it with the new result set. It gets set only by 'execute' method,
> by result of 'executeQuery' call. However, in javadoc of Statement
> interface there's nothing regarding its 'getResultSet' returning last
> result only on 'execute' call. In other words, it seems to me that
> calling 'getResultSet' after 'executeQuery' will always return null,
> which I doubt is right. Moreover, we null 'rs' field prior to doing
> anything, and I question its correctness (or would like to understand
> reasons behind such design) as well. H2's JdbcStatement overwrites
> previous result only when new one is available - it directly sets new
> result without making field null prior to that. We probably should
> store last result until the new one is actually available, what do you
> think?
>

I don't think anyone will ever use getResultSet() method, because it's
mainly used for multiple results which we do not support. So I would just
do this in the same way as other major drivers do (unless this violates the
spec, of course).


>
> - Alex
>
> 2016-08-03 3:15 GMT+03:00 Dmitriy Setrakyan :
> > On Tue, Aug 2, 2016 at 2:21 PM, Alexander Paschenko <
> > alexander.a.pasche...@gmail.com> wrote:
> >
> >> Dmitriy,
> >>
> >> Sorry, link access fixed, please check now.
> >> Will sum up current status on issue page, meanwhile links to both docs
> are
> >> there.
> >>
> >
> > Thanks! I have comments, but I will wait till your list the proposed
> > changes in the Jira, so I can comment there.
> >
> >
> >>
> >> — Alex
> >> 2 авг. 2016 г. 10:51 PM пользователь "Dmitriy Setrakyan" <
> >> dsetrak...@apache.org> написал:
> >>
> >> > Alex,
> >> >
> >> > Can you please also make me happy and put all your design into the
> ticket
> >> > instead of sending it around in emails?
> >> >
> >> > On top of that, the link you provided is protected. I cannot access
> it.
> >> >
> >> > D.
> >> >
> >> > On Tue, Aug 2, 2016 at 8:24 AM, Alexander Paschenko <
> >> > alexander.a.pasche...@gmail.com> wrote:
> >> >
> >> > > I have pushed "zero" version of JDBC updates support, currently
> >> > > without batching (working on it).
> >> > > Sergi, also to make you happy here's another doc with changes to
> >> > > public API: http://goo.gl/FvGKUs
> >> > >
> >> > > - Alex
> >> > >
> >> > > 2016-08-01 20:06 GMT+03:00 Sergi Vladykin  >:
> >> > > > Ok, I think you don't really understand what public API really
> is, so
> >> > let
> >> > > > me clarify. What you have described are all internal classes,
> public
> >> > API
> >> > > is
> >> > > > what end user will see and work with, like Ignite, IgniteCache,
> >> > > > QueryCursor, etc... All the internal changes do not require any
> >> special
> >> > > > discussion, until they are really complex or big or important, so
> you
> >> > > think
> >> > > > it makes sense to notify everyone about them.
> >> > > >
> >> > > > Here we should not have any public API changes for now and I don't
> >> see
> >> > > any
> >> > > > in your doc, so it looks fine to me.
> >> > > >
> >> > > > The only possible issue I see is origKeyClass and origValueClass.
> >> These
> >> > > > classes can be unavailable on nodes and most of the time we will
> have
> >> > to
> >> > > > work with binary format. Please make sure that this case is
> correctly
> >> > > > handled.
> >> > > >
> >> > > > Sergi
> >> > > >
> >> > > > 2016-08-01 18:14 GMT+03:00 Alexander Paschenko <
> >> > > > alexander.a.pasche...@gmail.com>:
> >> > > >
> >> > > >> Guys,
> >> > > >>
> >> > > >> Here's documented version of current API changes - it's quite
> modest
> >> > > >> https://goo.gl/Y6Cv1b
> >> > > >>
> >> > > >> - Alex
> >> > > >>
> >> > > >> 2016-07-28 20:34 GMT+03:00 Alexander Paschenko
> >> > > >> :
> >> > > >> > Sergi,
> >> > > >> >
> >> > > >> > OK, I've done it as you said, thanks.
> >> > > >> > Now working on binary marshaller support.
> >> > > >> >
> >> > > >> > - Alex
> >> > > >> >
> >> > > >> > 2016-07-28 9:08 GMT+03:00 Sergi Vladykin <
> >> sergi.vlady...@gmail.com
> >> > >:
> >> > > >> >> I had a quick look at the PR.
> >> > > >> >>
> >> > > >> >> I don't like this @QueryCacheKey and setKeyProp method on
> public
> >> > API.
> >> > > >> They
> >> > > >> >> solve nothing but add complexity and make key to be stored
> twice
> >> in
> >> > > >> cache,
> >> > > >> >> which is wrong. 

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

Folks,

OK, I have one thing to clarify about Statement.getResultSet: its
javadoc says that it "should be called only once per result", so
setting field to null every one and then is starting to make sense.

However, this "should" word does not tell us what we have to do if the
method gets called more than once. And after having had a look at
drivers of MySQL and Postgres I can tell that they don't null their
results explicitly and simply return the result they have. Frankly, as
a user, I'd be surprised by getting first a ResultSet and then null
after two consecutive calls of getResultSet. Still, this seems to be
one thing to decide by ourselves, so I'm asking your opinion about
this.

Another thing regarding JDBC driver behavior that I would like to
discuss is batch statements support, particularly behavior in case of
error - implementation of Statement.executeBatch().

JDBC javadocs don't specify what exactly we should do when an error
occurs during execution of some individual batch element - it says
that whether we fail fast or continue execution depends on us, and
this behavior just should be consistent with behavior of DBMS in
general. Here's how other engines handle this.

H2 attempts to run all elements of a batch, then, in case of one (or
more!) errors throws a BatchUpdateException that contains chain of all
exceptions as well as update counters for ALL batch elements - say, if
we have 3 elements batch and failed on first 2 and succeeded with the
3rd, then this exception will bear array of [-1, -1,
update_counter_for_3rd_stmt].

MySQL behaves depending on connection param set in connection string -
it can either fail fast or process all batch elements and then throw a
detailed exception, as H2 does.

Postgres, as I see it from the code, always fails fast.

Which behavior do you think it would be better to implement in Ignite?

- Alex

2016-08-03 15:47 GMT+03:00 Alexander Paschenko
:
> Guys,
>
> I have few questions about current state of the art regarding JDBC driver.
>
> First: in JdbcPreparedStatement, we determine number of arguments
> simply by counting question mark symbols. What if query string
> contains a string literal with question mark in it? It seems like a
> bug to me.
>
> Second: in JdbcStatement.executeQuery we null 'rs' field, but don't
> set it with the new result set. It gets set only by 'execute' method,
> by result of 'executeQuery' call. However, in javadoc of Statement
> interface there's nothing regarding its 'getResultSet' returning last
> result only on 'execute' call. In other words, it seems to me that
> calling 'getResultSet' after 'executeQuery' will always return null,
> which I doubt is right. Moreover, we null 'rs' field prior to doing
> anything, and I question its correctness (or would like to understand
> reasons behind such design) as well. H2's JdbcStatement overwrites
> previous result only when new one is available - it directly sets new
> result without making field null prior to that. We probably should
> store last result until the new one is actually available, what do you
> think?
>
> - Alex
>
> 2016-08-03 3:15 GMT+03:00 Dmitriy Setrakyan :
>> On Tue, Aug 2, 2016 at 2:21 PM, Alexander Paschenko <
>> alexander.a.pasche...@gmail.com> wrote:
>>
>>> Dmitriy,
>>>
>>> Sorry, link access fixed, please check now.
>>> Will sum up current status on issue page, meanwhile links to both docs are
>>> there.
>>>
>>
>> Thanks! I have comments, but I will wait till your list the proposed
>> changes in the Jira, so I can comment there.
>>
>>
>>>
>>> — Alex
>>> 2 авг. 2016 г. 10:51 PM пользователь "Dmitriy Setrakyan" <
>>> dsetrak...@apache.org> написал:
>>>
>>> > Alex,
>>> >
>>> > Can you please also make me happy and put all your design into the ticket
>>> > instead of sending it around in emails?
>>> >
>>> > On top of that, the link you provided is protected. I cannot access it.
>>> >
>>> > D.
>>> >
>>> > On Tue, Aug 2, 2016 at 8:24 AM, Alexander Paschenko <
>>> > alexander.a.pasche...@gmail.com> wrote:
>>> >
>>> > > I have pushed "zero" version of JDBC updates support, currently
>>> > > without batching (working on it).
>>> > > Sergi, also to make you happy here's another doc with changes to
>>> > > public API: http://goo.gl/FvGKUs
>>> > >
>>> > > - Alex
>>> > >
>>> > > 2016-08-01 20:06 GMT+03:00 Sergi Vladykin :
>>> > > > Ok, I think you don't really understand what public API really is, so
>>> > let
>>> > > > me clarify. What you have described are all internal classes, public
>>> > API
>>> > > is
>>> > > > what end user will see and work with, like Ignite, IgniteCache,
>>> > > > QueryCursor, etc... All the internal changes do not require any
>>> special
>>> > > > discussion, until they are really complex or big or important, so you
>>> > > think
>>> > > > it makes sense to notify everyone about them.
>>> > > >
>>> > > > Here we should not have any public API changes for now and I 

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

Guys,

I have few questions about current state of the art regarding JDBC driver.

First: in JdbcPreparedStatement, we determine number of arguments
simply by counting question mark symbols. What if query string
contains a string literal with question mark in it? It seems like a
bug to me.

Second: in JdbcStatement.executeQuery we null 'rs' field, but don't
set it with the new result set. It gets set only by 'execute' method,
by result of 'executeQuery' call. However, in javadoc of Statement
interface there's nothing regarding its 'getResultSet' returning last
result only on 'execute' call. In other words, it seems to me that
calling 'getResultSet' after 'executeQuery' will always return null,
which I doubt is right. Moreover, we null 'rs' field prior to doing
anything, and I question its correctness (or would like to understand
reasons behind such design) as well. H2's JdbcStatement overwrites
previous result only when new one is available - it directly sets new
result without making field null prior to that. We probably should
store last result until the new one is actually available, what do you
think?

- Alex

2016-08-03 3:15 GMT+03:00 Dmitriy Setrakyan :
> On Tue, Aug 2, 2016 at 2:21 PM, Alexander Paschenko <
> alexander.a.pasche...@gmail.com> wrote:
>
>> Dmitriy,
>>
>> Sorry, link access fixed, please check now.
>> Will sum up current status on issue page, meanwhile links to both docs are
>> there.
>>
>
> Thanks! I have comments, but I will wait till your list the proposed
> changes in the Jira, so I can comment there.
>
>
>>
>> — Alex
>> 2 авг. 2016 г. 10:51 PM пользователь "Dmitriy Setrakyan" <
>> dsetrak...@apache.org> написал:
>>
>> > Alex,
>> >
>> > Can you please also make me happy and put all your design into the ticket
>> > instead of sending it around in emails?
>> >
>> > On top of that, the link you provided is protected. I cannot access it.
>> >
>> > D.
>> >
>> > On Tue, Aug 2, 2016 at 8:24 AM, Alexander Paschenko <
>> > alexander.a.pasche...@gmail.com> wrote:
>> >
>> > > I have pushed "zero" version of JDBC updates support, currently
>> > > without batching (working on it).
>> > > Sergi, also to make you happy here's another doc with changes to
>> > > public API: http://goo.gl/FvGKUs
>> > >
>> > > - Alex
>> > >
>> > > 2016-08-01 20:06 GMT+03:00 Sergi Vladykin :
>> > > > Ok, I think you don't really understand what public API really is, so
>> > let
>> > > > me clarify. What you have described are all internal classes, public
>> > API
>> > > is
>> > > > what end user will see and work with, like Ignite, IgniteCache,
>> > > > QueryCursor, etc... All the internal changes do not require any
>> special
>> > > > discussion, until they are really complex or big or important, so you
>> > > think
>> > > > it makes sense to notify everyone about them.
>> > > >
>> > > > Here we should not have any public API changes for now and I don't
>> see
>> > > any
>> > > > in your doc, so it looks fine to me.
>> > > >
>> > > > The only possible issue I see is origKeyClass and origValueClass.
>> These
>> > > > classes can be unavailable on nodes and most of the time we will have
>> > to
>> > > > work with binary format. Please make sure that this case is correctly
>> > > > handled.
>> > > >
>> > > > Sergi
>> > > >
>> > > > 2016-08-01 18:14 GMT+03:00 Alexander Paschenko <
>> > > > alexander.a.pasche...@gmail.com>:
>> > > >
>> > > >> Guys,
>> > > >>
>> > > >> Here's documented version of current API changes - it's quite modest
>> > > >> https://goo.gl/Y6Cv1b
>> > > >>
>> > > >> - Alex
>> > > >>
>> > > >> 2016-07-28 20:34 GMT+03:00 Alexander Paschenko
>> > > >> :
>> > > >> > Sergi,
>> > > >> >
>> > > >> > OK, I've done it as you said, thanks.
>> > > >> > Now working on binary marshaller support.
>> > > >> >
>> > > >> > - Alex
>> > > >> >
>> > > >> > 2016-07-28 9:08 GMT+03:00 Sergi Vladykin <
>> sergi.vlady...@gmail.com
>> > >:
>> > > >> >> I had a quick look at the PR.
>> > > >> >>
>> > > >> >> I don't like this @QueryCacheKey and setKeyProp method on public
>> > API.
>> > > >> They
>> > > >> >> solve nothing but add complexity and make key to be stored twice
>> in
>> > > >> cache,
>> > > >> >> which is wrong. Please remove this.
>> > > >> >>
>> > > >> >> If you want to do some public API changes you have to discuss
>> them
>> > > >> publicly
>> > > >> >> before implementing them, ok?
>> > > >> >>
>> > > >> >> I did not look deeper yet, lets fix the obvious issue first.
>> > > >> >>
>> > > >> >> Sergi
>> > > >> >>
>> > > >> >> 2016-07-27 21:44 GMT+03:00 Alexander Paschenko <
>> > > >> >> alexander.a.pasche...@gmail.com>:
>> > > >> >>
>> > > >> >>> Sergi,
>> > > >> >>>
>> > > >> >>> I've made changes to the API according to your valuable
>> > > >> >>> recommendations, thank you very much for giving them. Please
>> refer
>> > > to
>> > > >> >>> PR to see current state of the work.
>> > > >> >>> Will surely look into ODBC, .NET and 

Re: IGNITE-2294 implementation details

[Dmitriy Setrakyan]

On Tue, Aug 2, 2016 at 2:21 PM, Alexander Paschenko <
alexander.a.pasche...@gmail.com> wrote:

> Dmitriy,
>
> Sorry, link access fixed, please check now.
> Will sum up current status on issue page, meanwhile links to both docs are
> there.
>

Thanks! I have comments, but I will wait till your list the proposed
changes in the Jira, so I can comment there.


>
> — Alex
> 2 авг. 2016 г. 10:51 PM пользователь "Dmitriy Setrakyan" <
> dsetrak...@apache.org> написал:
>
> > Alex,
> >
> > Can you please also make me happy and put all your design into the ticket
> > instead of sending it around in emails?
> >
> > On top of that, the link you provided is protected. I cannot access it.
> >
> > D.
> >
> > On Tue, Aug 2, 2016 at 8:24 AM, Alexander Paschenko <
> > alexander.a.pasche...@gmail.com> wrote:
> >
> > > I have pushed "zero" version of JDBC updates support, currently
> > > without batching (working on it).
> > > Sergi, also to make you happy here's another doc with changes to
> > > public API: http://goo.gl/FvGKUs
> > >
> > > - Alex
> > >
> > > 2016-08-01 20:06 GMT+03:00 Sergi Vladykin :
> > > > Ok, I think you don't really understand what public API really is, so
> > let
> > > > me clarify. What you have described are all internal classes, public
> > API
> > > is
> > > > what end user will see and work with, like Ignite, IgniteCache,
> > > > QueryCursor, etc... All the internal changes do not require any
> special
> > > > discussion, until they are really complex or big or important, so you
> > > think
> > > > it makes sense to notify everyone about them.
> > > >
> > > > Here we should not have any public API changes for now and I don't
> see
> > > any
> > > > in your doc, so it looks fine to me.
> > > >
> > > > The only possible issue I see is origKeyClass and origValueClass.
> These
> > > > classes can be unavailable on nodes and most of the time we will have
> > to
> > > > work with binary format. Please make sure that this case is correctly
> > > > handled.
> > > >
> > > > Sergi
> > > >
> > > > 2016-08-01 18:14 GMT+03:00 Alexander Paschenko <
> > > > alexander.a.pasche...@gmail.com>:
> > > >
> > > >> Guys,
> > > >>
> > > >> Here's documented version of current API changes - it's quite modest
> > > >> https://goo.gl/Y6Cv1b
> > > >>
> > > >> - Alex
> > > >>
> > > >> 2016-07-28 20:34 GMT+03:00 Alexander Paschenko
> > > >> :
> > > >> > Sergi,
> > > >> >
> > > >> > OK, I've done it as you said, thanks.
> > > >> > Now working on binary marshaller support.
> > > >> >
> > > >> > - Alex
> > > >> >
> > > >> > 2016-07-28 9:08 GMT+03:00 Sergi Vladykin <
> sergi.vlady...@gmail.com
> > >:
> > > >> >> I had a quick look at the PR.
> > > >> >>
> > > >> >> I don't like this @QueryCacheKey and setKeyProp method on public
> > API.
> > > >> They
> > > >> >> solve nothing but add complexity and make key to be stored twice
> in
> > > >> cache,
> > > >> >> which is wrong. Please remove this.
> > > >> >>
> > > >> >> If you want to do some public API changes you have to discuss
> them
> > > >> publicly
> > > >> >> before implementing them, ok?
> > > >> >>
> > > >> >> I did not look deeper yet, lets fix the obvious issue first.
> > > >> >>
> > > >> >> Sergi
> > > >> >>
> > > >> >> 2016-07-27 21:44 GMT+03:00 Alexander Paschenko <
> > > >> >> alexander.a.pasche...@gmail.com>:
> > > >> >>
> > > >> >>> Sergi,
> > > >> >>>
> > > >> >>> I've made changes to the API according to your valuable
> > > >> >>> recommendations, thank you very much for giving them. Please
> refer
> > > to
> > > >> >>> PR to see current state of the work.
> > > >> >>> Will surely look into ODBC, .NET and Visor. Though they will
> most
> > > >> >>> likely have to support a new feature rather than considerably
> > change
> > > >> >>> existing logic.
> > > >> >>>
> > > >> >>> - Alex
> > > >> >>>
> > > >> >>> 2016-07-27 14:23 GMT+03:00 Sergi Vladykin <
> > sergi.vlady...@gmail.com
> > > >:
> > > >> >>> > Please don't forget about ODBC, .NET and Visor. They all have
> to
> > > >> work in
> > > >> >>> > the same way.
> > > >> >>> >
> > > >> >>> > Sergi
> > > >> >>> >
> > > >> >>> > 2016-07-27 14:15 GMT+03:00 Alexander Paschenko <
> > > >> >>> > alexander.a.pasche...@gmail.com>:
> > > >> >>> >
> > > >> >>> >> OK, I've found that bold cast to QueryCursor in
> > > IgniteCacheProxy
> > > >> >>> >> and had a look at how SqlFieldsQuery is used in JDBC driver.
> > > Thanks.
> > > >> >>> >>
> > > >> >>> >> - Alex
> > > >> >>> >>
> > > >> >>> >> 2016-07-27 13:02 GMT+03:00 Sergi Vladykin <
> > > sergi.vlady...@gmail.com
> > > >> >:
> > > >> >>> >> > 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
> > > >> >>> 

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

Dmitriy,

Sorry, link access fixed, please check now.
Will sum up current status on issue page, meanwhile links to both docs are
there.

— Alex
2 авг. 2016 г. 10:51 PM пользователь "Dmitriy Setrakyan" <
dsetrak...@apache.org> написал:

> Alex,
>
> Can you please also make me happy and put all your design into the ticket
> instead of sending it around in emails?
>
> On top of that, the link you provided is protected. I cannot access it.
>
> D.
>
> On Tue, Aug 2, 2016 at 8:24 AM, Alexander Paschenko <
> alexander.a.pasche...@gmail.com> wrote:
>
> > I have pushed "zero" version of JDBC updates support, currently
> > without batching (working on it).
> > Sergi, also to make you happy here's another doc with changes to
> > public API: http://goo.gl/FvGKUs
> >
> > - Alex
> >
> > 2016-08-01 20:06 GMT+03:00 Sergi Vladykin :
> > > Ok, I think you don't really understand what public API really is, so
> let
> > > me clarify. What you have described are all internal classes, public
> API
> > is
> > > what end user will see and work with, like Ignite, IgniteCache,
> > > QueryCursor, etc... All the internal changes do not require any special
> > > discussion, until they are really complex or big or important, so you
> > think
> > > it makes sense to notify everyone about them.
> > >
> > > Here we should not have any public API changes for now and I don't see
> > any
> > > in your doc, so it looks fine to me.
> > >
> > > The only possible issue I see is origKeyClass and origValueClass. These
> > > classes can be unavailable on nodes and most of the time we will have
> to
> > > work with binary format. Please make sure that this case is correctly
> > > handled.
> > >
> > > Sergi
> > >
> > > 2016-08-01 18:14 GMT+03:00 Alexander Paschenko <
> > > alexander.a.pasche...@gmail.com>:
> > >
> > >> Guys,
> > >>
> > >> Here's documented version of current API changes - it's quite modest
> > >> https://goo.gl/Y6Cv1b
> > >>
> > >> - Alex
> > >>
> > >> 2016-07-28 20:34 GMT+03:00 Alexander Paschenko
> > >> :
> > >> > Sergi,
> > >> >
> > >> > OK, I've done it as you said, thanks.
> > >> > Now working on binary marshaller support.
> > >> >
> > >> > - Alex
> > >> >
> > >> > 2016-07-28 9:08 GMT+03:00 Sergi Vladykin  >:
> > >> >> I had a quick look at the PR.
> > >> >>
> > >> >> I don't like this @QueryCacheKey and setKeyProp method on public
> API.
> > >> They
> > >> >> solve nothing but add complexity and make key to be stored twice in
> > >> cache,
> > >> >> which is wrong. Please remove this.
> > >> >>
> > >> >> If you want to do some public API changes you have to discuss them
> > >> publicly
> > >> >> before implementing them, ok?
> > >> >>
> > >> >> I did not look deeper yet, lets fix the obvious issue first.
> > >> >>
> > >> >> Sergi
> > >> >>
> > >> >> 2016-07-27 21:44 GMT+03:00 Alexander Paschenko <
> > >> >> alexander.a.pasche...@gmail.com>:
> > >> >>
> > >> >>> Sergi,
> > >> >>>
> > >> >>> I've made changes to the API according to your valuable
> > >> >>> recommendations, thank you very much for giving them. Please refer
> > to
> > >> >>> PR to see current state of the work.
> > >> >>> Will surely look into ODBC, .NET and Visor. Though they will most
> > >> >>> likely have to support a new feature rather than considerably
> change
> > >> >>> existing logic.
> > >> >>>
> > >> >>> - Alex
> > >> >>>
> > >> >>> 2016-07-27 14:23 GMT+03:00 Sergi Vladykin <
> sergi.vlady...@gmail.com
> > >:
> > >> >>> > Please don't forget about ODBC, .NET and Visor. They all have to
> > >> work in
> > >> >>> > the same way.
> > >> >>> >
> > >> >>> > Sergi
> > >> >>> >
> > >> >>> > 2016-07-27 14:15 GMT+03:00 Alexander Paschenko <
> > >> >>> > alexander.a.pasche...@gmail.com>:
> > >> >>> >
> > >> >>> >> OK, I've found that bold cast to QueryCursor in
> > IgniteCacheProxy
> > >> >>> >> and had a look at how SqlFieldsQuery is used in JDBC driver.
> > Thanks.
> > >> >>> >>
> > >> >>> >> - Alex
> > >> >>> >>
> > >> >>> >> 2016-07-27 13:02 GMT+03:00 Sergi Vladykin <
> > sergi.vlady...@gmail.com
> > >> >:
> > >> >>> >> > 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.
> > >> 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
> > >> >>> >> >>
> > >> 

Re: IGNITE-2294 implementation details

[Dmitriy Setrakyan]

Alex,

Can you please also make me happy and put all your design into the ticket
instead of sending it around in emails?

On top of that, the link you provided is protected. I cannot access it.

D.

On Tue, Aug 2, 2016 at 8:24 AM, Alexander Paschenko <
alexander.a.pasche...@gmail.com> wrote:

> I have pushed "zero" version of JDBC updates support, currently
> without batching (working on it).
> Sergi, also to make you happy here's another doc with changes to
> public API: http://goo.gl/FvGKUs
>
> - Alex
>
> 2016-08-01 20:06 GMT+03:00 Sergi Vladykin :
> > Ok, I think you don't really understand what public API really is, so let
> > me clarify. What you have described are all internal classes, public API
> is
> > what end user will see and work with, like Ignite, IgniteCache,
> > QueryCursor, etc... All the internal changes do not require any special
> > discussion, until they are really complex or big or important, so you
> think
> > it makes sense to notify everyone about them.
> >
> > Here we should not have any public API changes for now and I don't see
> any
> > in your doc, so it looks fine to me.
> >
> > The only possible issue I see is origKeyClass and origValueClass. These
> > classes can be unavailable on nodes and most of the time we will have to
> > work with binary format. Please make sure that this case is correctly
> > handled.
> >
> > Sergi
> >
> > 2016-08-01 18:14 GMT+03:00 Alexander Paschenko <
> > alexander.a.pasche...@gmail.com>:
> >
> >> Guys,
> >>
> >> Here's documented version of current API changes - it's quite modest
> >> https://goo.gl/Y6Cv1b
> >>
> >> - Alex
> >>
> >> 2016-07-28 20:34 GMT+03:00 Alexander Paschenko
> >> :
> >> > Sergi,
> >> >
> >> > OK, I've done it as you said, thanks.
> >> > Now working on binary marshaller support.
> >> >
> >> > - Alex
> >> >
> >> > 2016-07-28 9:08 GMT+03:00 Sergi Vladykin :
> >> >> I had a quick look at the PR.
> >> >>
> >> >> I don't like this @QueryCacheKey and setKeyProp method on public API.
> >> They
> >> >> solve nothing but add complexity and make key to be stored twice in
> >> cache,
> >> >> which is wrong. Please remove this.
> >> >>
> >> >> If you want to do some public API changes you have to discuss them
> >> publicly
> >> >> before implementing them, ok?
> >> >>
> >> >> I did not look deeper yet, lets fix the obvious issue first.
> >> >>
> >> >> Sergi
> >> >>
> >> >> 2016-07-27 21:44 GMT+03:00 Alexander Paschenko <
> >> >> alexander.a.pasche...@gmail.com>:
> >> >>
> >> >>> Sergi,
> >> >>>
> >> >>> I've made changes to the API according to your valuable
> >> >>> recommendations, thank you very much for giving them. Please refer
> to
> >> >>> PR to see current state of the work.
> >> >>> Will surely look into ODBC, .NET and Visor. Though they will most
> >> >>> likely have to support a new feature rather than considerably change
> >> >>> existing logic.
> >> >>>
> >> >>> - Alex
> >> >>>
> >> >>> 2016-07-27 14:23 GMT+03:00 Sergi Vladykin  >:
> >> >>> > Please don't forget about ODBC, .NET and Visor. They all have to
> >> work in
> >> >>> > the same way.
> >> >>> >
> >> >>> > Sergi
> >> >>> >
> >> >>> > 2016-07-27 14:15 GMT+03:00 Alexander Paschenko <
> >> >>> > alexander.a.pasche...@gmail.com>:
> >> >>> >
> >> >>> >> OK, I've found that bold cast to QueryCursor in
> IgniteCacheProxy
> >> >>> >> and had a look at how SqlFieldsQuery is used in JDBC driver.
> Thanks.
> >> >>> >>
> >> >>> >> - Alex
> >> >>> >>
> >> >>> >> 2016-07-27 13:02 GMT+03:00 Sergi Vladykin <
> sergi.vlady...@gmail.com
> >> >:
> >> >>> >> > 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.
> >> 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
> 

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

I have pushed "zero" version of JDBC updates support, currently
without batching (working on it).
Sergi, also to make you happy here's another doc with changes to
public API: http://goo.gl/FvGKUs

- Alex

2016-08-01 20:06 GMT+03:00 Sergi Vladykin :
> Ok, I think you don't really understand what public API really is, so let
> me clarify. What you have described are all internal classes, public API is
> what end user will see and work with, like Ignite, IgniteCache,
> QueryCursor, etc... All the internal changes do not require any special
> discussion, until they are really complex or big or important, so you think
> it makes sense to notify everyone about them.
>
> Here we should not have any public API changes for now and I don't see any
> in your doc, so it looks fine to me.
>
> The only possible issue I see is origKeyClass and origValueClass. These
> classes can be unavailable on nodes and most of the time we will have to
> work with binary format. Please make sure that this case is correctly
> handled.
>
> Sergi
>
> 2016-08-01 18:14 GMT+03:00 Alexander Paschenko <
> alexander.a.pasche...@gmail.com>:
>
>> Guys,
>>
>> Here's documented version of current API changes - it's quite modest
>> https://goo.gl/Y6Cv1b
>>
>> - Alex
>>
>> 2016-07-28 20:34 GMT+03:00 Alexander Paschenko
>> :
>> > Sergi,
>> >
>> > OK, I've done it as you said, thanks.
>> > Now working on binary marshaller support.
>> >
>> > - Alex
>> >
>> > 2016-07-28 9:08 GMT+03:00 Sergi Vladykin :
>> >> I had a quick look at the PR.
>> >>
>> >> I don't like this @QueryCacheKey and setKeyProp method on public API.
>> They
>> >> solve nothing but add complexity and make key to be stored twice in
>> cache,
>> >> which is wrong. Please remove this.
>> >>
>> >> If you want to do some public API changes you have to discuss them
>> publicly
>> >> before implementing them, ok?
>> >>
>> >> I did not look deeper yet, lets fix the obvious issue first.
>> >>
>> >> Sergi
>> >>
>> >> 2016-07-27 21:44 GMT+03:00 Alexander Paschenko <
>> >> alexander.a.pasche...@gmail.com>:
>> >>
>> >>> Sergi,
>> >>>
>> >>> I've made changes to the API according to your valuable
>> >>> recommendations, thank you very much for giving them. Please refer to
>> >>> PR to see current state of the work.
>> >>> Will surely look into ODBC, .NET and Visor. Though they will most
>> >>> likely have to support a new feature rather than considerably change
>> >>> existing logic.
>> >>>
>> >>> - Alex
>> >>>
>> >>> 2016-07-27 14:23 GMT+03:00 Sergi Vladykin :
>> >>> > Please don't forget about ODBC, .NET and Visor. They all have to
>> work in
>> >>> > the same way.
>> >>> >
>> >>> > Sergi
>> >>> >
>> >>> > 2016-07-27 14:15 GMT+03:00 Alexander Paschenko <
>> >>> > alexander.a.pasche...@gmail.com>:
>> >>> >
>> >>> >> OK, I've found that bold cast to QueryCursor in IgniteCacheProxy
>> >>> >> and had a look at how SqlFieldsQuery is used in JDBC driver. Thanks.
>> >>> >>
>> >>> >> - Alex
>> >>> >>
>> >>> >> 2016-07-27 13:02 GMT+03:00 Sergi Vladykin > >:
>> >>> >> > 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.
>> 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
>> >>> >> >> >>

Re: IGNITE-2294 implementation details

[Sergi Vladykin]

Ok, I think you don't really understand what public API really is, so let
me clarify. What you have described are all internal classes, public API is
what end user will see and work with, like Ignite, IgniteCache,
QueryCursor, etc... All the internal changes do not require any special
discussion, until they are really complex or big or important, so you think
it makes sense to notify everyone about them.

Here we should not have any public API changes for now and I don't see any
in your doc, so it looks fine to me.

The only possible issue I see is origKeyClass and origValueClass. These
classes can be unavailable on nodes and most of the time we will have to
work with binary format. Please make sure that this case is correctly
handled.

Sergi

2016-08-01 18:14 GMT+03:00 Alexander Paschenko <
alexander.a.pasche...@gmail.com>:

> Guys,
>
> Here's documented version of current API changes - it's quite modest
> https://goo.gl/Y6Cv1b
>
> - Alex
>
> 2016-07-28 20:34 GMT+03:00 Alexander Paschenko
> :
> > Sergi,
> >
> > OK, I've done it as you said, thanks.
> > Now working on binary marshaller support.
> >
> > - Alex
> >
> > 2016-07-28 9:08 GMT+03:00 Sergi Vladykin :
> >> I had a quick look at the PR.
> >>
> >> I don't like this @QueryCacheKey and setKeyProp method on public API.
> They
> >> solve nothing but add complexity and make key to be stored twice in
> cache,
> >> which is wrong. Please remove this.
> >>
> >> If you want to do some public API changes you have to discuss them
> publicly
> >> before implementing them, ok?
> >>
> >> I did not look deeper yet, lets fix the obvious issue first.
> >>
> >> Sergi
> >>
> >> 2016-07-27 21:44 GMT+03:00 Alexander Paschenko <
> >> alexander.a.pasche...@gmail.com>:
> >>
> >>> Sergi,
> >>>
> >>> I've made changes to the API according to your valuable
> >>> recommendations, thank you very much for giving them. Please refer to
> >>> PR to see current state of the work.
> >>> Will surely look into ODBC, .NET and Visor. Though they will most
> >>> likely have to support a new feature rather than considerably change
> >>> existing logic.
> >>>
> >>> - Alex
> >>>
> >>> 2016-07-27 14:23 GMT+03:00 Sergi Vladykin :
> >>> > Please don't forget about ODBC, .NET and Visor. They all have to
> work in
> >>> > the same way.
> >>> >
> >>> > Sergi
> >>> >
> >>> > 2016-07-27 14:15 GMT+03:00 Alexander Paschenko <
> >>> > alexander.a.pasche...@gmail.com>:
> >>> >
> >>> >> OK, I've found that bold cast to QueryCursor in IgniteCacheProxy
> >>> >> and had a look at how SqlFieldsQuery is used in JDBC driver. Thanks.
> >>> >>
> >>> >> - Alex
> >>> >>
> >>> >> 2016-07-27 13:02 GMT+03:00 Sergi Vladykin  >:
> >>> >> > 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.
> 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
> 

Re: IGNITE-2294 implementation details

[Dmitriy Setrakyan]

Alexey, can you attach this document to the ticket, or better yet, just
copy it there?

On Mon, Aug 1, 2016 at 8:14 AM, Alexander Paschenko <
alexander.a.pasche...@gmail.com> wrote:

> Guys,
>
> Here's documented version of current API changes - it's quite modest
> https://goo.gl/Y6Cv1b
>
> - Alex
>
> 2016-07-28 20:34 GMT+03:00 Alexander Paschenko
> :
> > Sergi,
> >
> > OK, I've done it as you said, thanks.
> > Now working on binary marshaller support.
> >
> > - Alex
> >
> > 2016-07-28 9:08 GMT+03:00 Sergi Vladykin :
> >> I had a quick look at the PR.
> >>
> >> I don't like this @QueryCacheKey and setKeyProp method on public API.
> They
> >> solve nothing but add complexity and make key to be stored twice in
> cache,
> >> which is wrong. Please remove this.
> >>
> >> If you want to do some public API changes you have to discuss them
> publicly
> >> before implementing them, ok?
> >>
> >> I did not look deeper yet, lets fix the obvious issue first.
> >>
> >> Sergi
> >>
> >> 2016-07-27 21:44 GMT+03:00 Alexander Paschenko <
> >> alexander.a.pasche...@gmail.com>:
> >>
> >>> Sergi,
> >>>
> >>> I've made changes to the API according to your valuable
> >>> recommendations, thank you very much for giving them. Please refer to
> >>> PR to see current state of the work.
> >>> Will surely look into ODBC, .NET and Visor. Though they will most
> >>> likely have to support a new feature rather than considerably change
> >>> existing logic.
> >>>
> >>> - Alex
> >>>
> >>> 2016-07-27 14:23 GMT+03:00 Sergi Vladykin :
> >>> > Please don't forget about ODBC, .NET and Visor. They all have to
> work in
> >>> > the same way.
> >>> >
> >>> > Sergi
> >>> >
> >>> > 2016-07-27 14:15 GMT+03:00 Alexander Paschenko <
> >>> > alexander.a.pasche...@gmail.com>:
> >>> >
> >>> >> OK, I've found that bold cast to QueryCursor in IgniteCacheProxy
> >>> >> and had a look at how SqlFieldsQuery is used in JDBC driver. Thanks.
> >>> >>
> >>> >> - Alex
> >>> >>
> >>> >> 2016-07-27 13:02 GMT+03:00 Sergi Vladykin  >:
> >>> >> > 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.
> 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 

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

Guys,

Here's documented version of current API changes - it's quite modest
https://goo.gl/Y6Cv1b

- Alex

2016-07-28 20:34 GMT+03:00 Alexander Paschenko
:
> Sergi,
>
> OK, I've done it as you said, thanks.
> Now working on binary marshaller support.
>
> - Alex
>
> 2016-07-28 9:08 GMT+03:00 Sergi Vladykin :
>> I had a quick look at the PR.
>>
>> I don't like this @QueryCacheKey and setKeyProp method on public API. They
>> solve nothing but add complexity and make key to be stored twice in cache,
>> which is wrong. Please remove this.
>>
>> If you want to do some public API changes you have to discuss them publicly
>> before implementing them, ok?
>>
>> I did not look deeper yet, lets fix the obvious issue first.
>>
>> Sergi
>>
>> 2016-07-27 21:44 GMT+03:00 Alexander Paschenko <
>> alexander.a.pasche...@gmail.com>:
>>
>>> Sergi,
>>>
>>> I've made changes to the API according to your valuable
>>> recommendations, thank you very much for giving them. Please refer to
>>> PR to see current state of the work.
>>> Will surely look into ODBC, .NET and Visor. Though they will most
>>> likely have to support a new feature rather than considerably change
>>> existing logic.
>>>
>>> - Alex
>>>
>>> 2016-07-27 14:23 GMT+03:00 Sergi Vladykin :
>>> > Please don't forget about ODBC, .NET and Visor. They all have to work in
>>> > the same way.
>>> >
>>> > Sergi
>>> >
>>> > 2016-07-27 14:15 GMT+03:00 Alexander Paschenko <
>>> > alexander.a.pasche...@gmail.com>:
>>> >
>>> >> OK, I've found that bold cast to QueryCursor in IgniteCacheProxy
>>> >> and had a look at how SqlFieldsQuery is used in JDBC driver. Thanks.
>>> >>
>>> >> - Alex
>>> >>
>>> >> 2016-07-27 13:02 GMT+03:00 Sergi Vladykin :
>>> >> > 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. 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 >> >:
>>> >> >> > 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>:
>>> >> >> >> 

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

Sergi,

OK, I've done it as you said, thanks.
Now working on binary marshaller support.

- Alex

2016-07-28 9:08 GMT+03:00 Sergi Vladykin :
> I had a quick look at the PR.
>
> I don't like this @QueryCacheKey and setKeyProp method on public API. They
> solve nothing but add complexity and make key to be stored twice in cache,
> which is wrong. Please remove this.
>
> If you want to do some public API changes you have to discuss them publicly
> before implementing them, ok?
>
> I did not look deeper yet, lets fix the obvious issue first.
>
> Sergi
>
> 2016-07-27 21:44 GMT+03:00 Alexander Paschenko <
> alexander.a.pasche...@gmail.com>:
>
>> Sergi,
>>
>> I've made changes to the API according to your valuable
>> recommendations, thank you very much for giving them. Please refer to
>> PR to see current state of the work.
>> Will surely look into ODBC, .NET and Visor. Though they will most
>> likely have to support a new feature rather than considerably change
>> existing logic.
>>
>> - Alex
>>
>> 2016-07-27 14:23 GMT+03:00 Sergi Vladykin :
>> > Please don't forget about ODBC, .NET and Visor. They all have to work in
>> > the same way.
>> >
>> > Sergi
>> >
>> > 2016-07-27 14:15 GMT+03:00 Alexander Paschenko <
>> > alexander.a.pasche...@gmail.com>:
>> >
>> >> OK, I've found that bold cast to QueryCursor in IgniteCacheProxy
>> >> and had a look at how SqlFieldsQuery is used in JDBC driver. Thanks.
>> >>
>> >> - Alex
>> >>
>> >> 2016-07-27 13:02 GMT+03:00 Sergi Vladykin :
>> >> > 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. 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 > >:
>> >> >> > 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 

Re: IGNITE-2294 implementation details

[Sergi Vladykin]

I had a quick look at the PR.

I don't like this @QueryCacheKey and setKeyProp method on public API. They
solve nothing but add complexity and make key to be stored twice in cache,
which is wrong. Please remove this.

If you want to do some public API changes you have to discuss them publicly
before implementing them, ok?

I did not look deeper yet, lets fix the obvious issue first.

Sergi

2016-07-27 21:44 GMT+03:00 Alexander Paschenko <
alexander.a.pasche...@gmail.com>:

> Sergi,
>
> I've made changes to the API according to your valuable
> recommendations, thank you very much for giving them. Please refer to
> PR to see current state of the work.
> Will surely look into ODBC, .NET and Visor. Though they will most
> likely have to support a new feature rather than considerably change
> existing logic.
>
> - Alex
>
> 2016-07-27 14:23 GMT+03:00 Sergi Vladykin :
> > Please don't forget about ODBC, .NET and Visor. They all have to work in
> > the same way.
> >
> > Sergi
> >
> > 2016-07-27 14:15 GMT+03:00 Alexander Paschenko <
> > alexander.a.pasche...@gmail.com>:
> >
> >> OK, I've found that bold cast to QueryCursor in IgniteCacheProxy
> >> and had a look at how SqlFieldsQuery is used in JDBC driver. Thanks.
> >>
> >> - Alex
> >>
> >> 2016-07-27 13:02 GMT+03:00 Sergi Vladykin :
> >> > 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. 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  >:
> >> >> > 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
> >> >> >> >> > >
> >> >> >> >> > > 

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

Sergi,

I've made changes to the API according to your valuable
recommendations, thank you very much for giving them. Please refer to
PR to see current state of the work.
Will surely look into ODBC, .NET and Visor. Though they will most
likely have to support a new feature rather than considerably change
existing logic.

- Alex

2016-07-27 14:23 GMT+03:00 Sergi Vladykin :
> Please don't forget about ODBC, .NET and Visor. They all have to work in
> the same way.
>
> Sergi
>
> 2016-07-27 14:15 GMT+03:00 Alexander Paschenko <
> alexander.a.pasche...@gmail.com>:
>
>> OK, I've found that bold cast to QueryCursor in IgniteCacheProxy
>> and had a look at how SqlFieldsQuery is used in JDBC driver. Thanks.
>>
>> - Alex
>>
>> 2016-07-27 13:02 GMT+03:00 Sergi Vladykin :
>> > 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. 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 :
>> >> > 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 :
>> >> >> > 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
>> >> >> 

Re: IGNITE-2294 implementation details

[Sergi Vladykin]

Please don't forget about ODBC, .NET and Visor. They all have to work in
the same way.

Sergi

2016-07-27 14:15 GMT+03:00 Alexander Paschenko <
alexander.a.pasche...@gmail.com>:

> OK, I've found that bold cast to QueryCursor in IgniteCacheProxy
> and had a look at how SqlFieldsQuery is used in JDBC driver. Thanks.
>
> - Alex
>
> 2016-07-27 13:02 GMT+03:00 Sergi Vladykin :
> > 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. 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 :
> >> > 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 :
> >> >> > 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 

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

OK, I've found that bold cast to QueryCursor in IgniteCacheProxy
and had a look at how SqlFieldsQuery is used in JDBC driver. Thanks.

- Alex

2016-07-27 13:02 GMT+03:00 Sergi Vladykin :
> 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. 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 :
>> > 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 :
>> >> > 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 

Re: IGNITE-2294 implementation details

[Sergi Vladykin]

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

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

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

Re: IGNITE-2294 implementation details

[Sergi Vladykin]

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

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

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 :
> What is the ticket number for this. Is the new API documented there?
>
> On Tue, Jul 26, 2016 at 11:36 AM, Sergi Vladykin 
> 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 :
>> > > 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 :
>> > >
>> > >> 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 

Re: IGNITE-2294 implementation details

[Dmitriy Setrakyan]

What is the ticket number for this. Is the new API documented there?

On Tue, Jul 26, 2016 at 11:36 AM, Sergi Vladykin 
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 :
> > > 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 :
> > >
> > >> 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  >:
> > >>> > OK, then using your analogy, the current behavior in Ignite is
> MERGE
> > for
> > >>> > the most part.
> 

Re: IGNITE-2294 implementation details

[Sergi Vladykin]

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

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

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

Re: IGNITE-2294 implementation details

[Sergi Vladykin]

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

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

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

Re: IGNITE-2294 implementation details

[Dmitriy Setrakyan]

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

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

I agree. It looks like we first of all need some algorithm of key
generation for new objects, and it does not necessarily have to be
involved with DDL. The first suggestion that comes to my mind on that
matter is, obviously, marking some method of the class persisted with
magical annotation - so that the value will be able to supply its own
key when needed.

Alex

2016-07-21 17:11 GMT+03:00 Sergi Vladykin :
> I think these problems we need to solve regardless of DDL.
>
> Sergi
>
> On 21 июля 2016 г., 16:40, Alexey Goncharuk 
> wrote:
>
>> Suppose I have a PersonKey {int id;} class and Person {@SqlQueryField int
>> id; @SqlQueryField String name; @SqlQueryField int age;} class.
>>
>> How would an insert statement look like?
>> INSERT INTO Person (_key, _val) values (...)
>> INSERT INTO Person (_key, _val, id, name, age) values (...) -> Obviously
>> will not be usable from any console unless we are able to parse "new
>> PersonKey(1)" statements.
>>
>> INSERT INTO Person (id, name, age) values (...) -> How do we know how to
>> construct the PersonKey? Most likely I am missing something, but this is
>> not clear for me so far.
>>

Re: IGNITE-2294 implementation details

[Alexey Goncharuk]

Suppose I have a PersonKey {int id;} class and Person {@SqlQueryField int
id; @SqlQueryField String name; @SqlQueryField int age;} class.

How would an insert statement look like?
INSERT INTO Person (_key, _val) values (...)
INSERT INTO Person (_key, _val, id, name, age) values (...) -> Obviously
will not be usable from any console unless we are able to parse "new
PersonKey(1)" statements.

INSERT INTO Person (id, name, age) values (...) -> How do we know how to
construct the PersonKey? Most likely I am missing something, but this is
not clear for me so far.

Re: IGNITE-2294 implementation details

[Sergi Vladykin]

I don't see why we need to start with DDL. We have table definitions with
key and value definitions in them, so what is the problem?

Sergi

On Thu, Jul 21, 2016 at 2:14 PM, Alexey Goncharuk <
alexey.goncha...@gmail.com> wrote:

> For me the main question here is how we define Key and Value for a cache
> when using SQL interface. Unless we are going to re-architect the whole
> Ignite, it will still be a key-value storage in the first place, so
> whenever we do an INSERT, we must generate Key and Value. Given that values
> in INSERT are supposed to be fields of either Key or Value, this confuses
> me a lot.
>
> Maybe we should come up with DDL first?
>
> 2016-07-21 14:06 GMT+03:00 Sergi Vladykin :
>
> > 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
> > > > > > :
> > > > > >> 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 

Re: IGNITE-2294 implementation details

[Alexey Goncharuk]

For me the main question here is how we define Key and Value for a cache
when using SQL interface. Unless we are going to re-architect the whole
Ignite, it will still be a key-value storage in the first place, so
whenever we do an INSERT, we must generate Key and Value. Given that values
in INSERT are supposed to be fields of either Key or Value, this confuses
me a lot.

Maybe we should come up with DDL first?

2016-07-21 14:06 GMT+03:00 Sergi Vladykin :

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

Re: IGNITE-2294 implementation details

[Sergi Vladykin]

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

Re: IGNITE-2294 implementation details

[Sergi Vladykin]

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

Re: IGNITE-2294 implementation details

[Dmitriy Setrakyan]

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

Re: IGNITE-2294 implementation details

[Alexander Paschenko]

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

IGNITE-2294 implementation details

[Alexander Paschenko]

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