IGNITE-2294 implementation details

[Sergi]

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 <[hidden email]>:

> 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 <[hidden email]>:
>
>> Why do we need to count query arguments? Can anyone clarify?
>>
>> Sergi
>>
>> 2016-08-04 5:07 GMT+03:00 Alexey Kuznetsov <[hidden email]>:
>>
>>> 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 .
>>>
>>
>>
>

[al.psc]

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 <[hidden email]>:

> 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 <[hidden email]>:
>
>> 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 <[hidden email]>:
>>
>>> Why do we need to count query arguments? Can anyone clarify?
>>>
>>> Sergi
>>>
>>> 2016-08-04 5:07 GMT+03:00 Alexey Kuznetsov <[hidden email]>:
>>>
>>>> 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 .
>>>>
>>>
>>>
>>

[Sergi]

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 <
[hidden email]>:

> 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 <[hidden email]>:
> > 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 <[hidden email]>:
> >
> >> 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 <[hidden email]>:
> >>
> >>> Why do we need to count query arguments? Can anyone clarify?
> >>>
> >>> Sergi
> >>>
> >>> 2016-08-04 5:07 GMT+03:00 Alexey Kuznetsov <[hidden email]>:
> >>>
> >>>> 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 .
> >>>>
> >>>
> >>>
> >>
>

[al.psc]

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 <[hidden email]>:

> 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 <
> [hidden email]>:
>
>> 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 <[hidden email]>:
>> > 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 <[hidden email]>:
>> >
>> >> 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 <[hidden email]>:
>> >>
>> >>> Why do we need to count query arguments? Can anyone clarify?
>> >>>
>> >>> Sergi
>> >>>
>> >>> 2016-08-04 5:07 GMT+03:00 Alexey Kuznetsov <[hidden email]>:
>> >>>
>> >>>> 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 .
>> >>>>
>> >>>
>> >>>
>> >>
>>

[al.psc]

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
<[hidden email]>:

> 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 <[hidden email]>:
>> 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 <
>> [hidden email]>:
>>
>>> 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 <[hidden email]>:
>>> > 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 <[hidden email]>:
>>> >
>>> >> 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 <[hidden email]>:
>>> >>
>>> >>> Why do we need to count query arguments? Can anyone clarify?
>>> >>>
>>> >>> Sergi
>>> >>>
>>> >>> 2016-08-04 5:07 GMT+03:00 Alexey Kuznetsov <[hidden email]>:
>>> >>>
>>> >>>> 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 .
>>> >>>>
>>> >>>
>>> >>>
>>> >>
>>>

[dsetrakyan]

On Fri, Aug 5, 2016 at 8:24 AM, Alexander Paschenko <
[hidden email]> wrote:

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

Responded in the ticket:
https://issues.apache.org/jira/browse/IGNITE-2294

>
> - Alex
>
> 2016-08-04 21:16 GMT+03:00 Alexander Paschenko
> <[hidden email]>:
> > 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 <[hidden email]>:
> >> 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 <
> >> [hidden email]>:
> >>
> >>> 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 <[hidden email]>:
> >>> > 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 <[hidden email]>:
> >>> >
> >>> >> 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 <[hidden email]
> >:
> >>> >>
> >>> >>> Why do we need to count query arguments? Can anyone clarify?
> >>> >>>
> >>> >>> Sergi
> >>> >>>
> >>> >>> 2016-08-04 5:07 GMT+03:00 Alexey Kuznetsov <
> [hidden email]>:
> >>> >>>
> >>> >>>> 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 .
> >>> >>>>
> >>> >>>
> >>> >>>
> >>> >>
> >>>
>