Done. Sergi
2017-05-23 13:48 GMT+03:00 Andrey Mashenkov <andrey.mashen...@gmail.com>: > IGNITE-5252 [1] is ready for review. > Sergi, would you please take a look at it? > > [1] https://issues.apache.org/jira/browse/IGNITE-5252 > > On Sat, May 20, 2017 at 7:07 PM, Andrey Mashenkov < > andrey.mashen...@gmail.com> wrote: > > > Dmitry, > > > > Here is a link ot ticket [1] > > > > [1] https://issues.apache.org/jira/browse/IGNITE-5252 > > > > On Sat, May 20, 2017 at 1:00 AM, Dmitriy Setrakyan < > dsetrak...@apache.org> > > wrote: > > > >> I cannot find a ticket for it. Has it been filed? > >> > >> On Fri, May 19, 2017 at 12:38 AM, Vladimir Ozerov <voze...@gridgain.com > > > >> wrote: > >> > >> > Ah, got it. Then I am ok with the change as well. > >> > > >> > On Fri, May 19, 2017 at 9:24 AM, Sergi Vladykin < > >> sergi.vlady...@gmail.com> > >> > wrote: > >> > > >> > > Nope, the proposal was to have a FieldsQueryCursor interface with > >> > > getFieldName(int column) method, may be + some other methods we will > >> add > >> > > later. This does not require any complex code modifications or > >> exposing > >> > > internal APIs. > >> > > > >> > > I'm not against new SQL API, it is a good idea, but it should not > >> prevent > >> > > us from making easy fixes in existing API when we need it. > >> > > > >> > > Sergi > >> > > > >> > > 2017-05-18 23:20 GMT+03:00 Vladimir Ozerov <voze...@gridgain.com>: > >> > > > >> > > > Proposal is about returning GridQueryFieldMetadata from > QueryCursor, > >> > > which > >> > > > is internal interface. This interface is counterintuitive and is > not > >> > > ready > >> > > > to be exposed to users. For example, it has method "typeName" > which > >> > > > actually returns table name. And has method "fieldTypeName" which > >> > returns > >> > > > something like "java.lang.Object". Add "type name" concept from > our > >> > > > BinaryConfiguration/QueryEntity, which have different semantics, > >> and > >> > you > >> > > > end up with totally confused users on what "type name" means in > >> Ignite. > >> > > > > >> > > > Let's do not expose strange things to users, and accurately create > >> new > >> > > > clean SQL API instead. There is no strong demand for this feature. > >> > > > > >> > > > On Thu, May 18, 2017 at 7:39 PM, Sergi Vladykin < > >> > > sergi.vlady...@gmail.com> > >> > > > wrote: > >> > > > > >> > > > > It should not require any internals movement, it must be an easy > >> fix. > >> > > > > > >> > > > > Sergi > >> > > > > > >> > > > > 2017-05-18 15:36 GMT+03:00 Vladimir Ozerov < > voze...@gridgain.com > >> >: > >> > > > > > >> > > > > > With all the changes to internals we made, new API can be > >> created > >> > > very > >> > > > > > quickly somewhere around AI 2.2 or AI 2.3. Currently the whole > >> API > >> > is > >> > > > > > located in the wrong place, as it is bounded to cache. So the > >> more > >> > we > >> > > > add > >> > > > > > now, the more we will deprecate in several months. Remember, > >> that > >> > > this > >> > > > > > feature will require not only new interface, but moving > existing > >> > > > > *internal* > >> > > > > > metadata classes to public space. These classes were never > >> designed > >> > > to > >> > > > be > >> > > > > > exposed to users in the first place. > >> > > > > > > >> > > > > > This is why I am strongly against this change at the moment. > No > >> > need > >> > > to > >> > > > > > make already outdated and complex API even more complex > without > >> > > strong > >> > > > > > demand from users. > >> > > > > > > >> > > > > > On Thu, May 18, 2017 at 3:29 PM, Pavel Tupitsyn < > >> > > ptupit...@apache.org> > >> > > > > > wrote: > >> > > > > > > >> > > > > > > I agree that this change makes sense. > >> > > > > > > With complex queries it may be non-trivial to get the right > >> > column > >> > > by > >> > > > > > index > >> > > > > > > from results. > >> > > > > > > With metadata user no longer needs to care about result > column > >> > > order, > >> > > > > and > >> > > > > > > refactorings are easier. > >> > > > > > > > >> > > > > > > Pavel > >> > > > > > > > >> > > > > > > On Thu, May 18, 2017 at 2:36 PM, Sergi Vladykin < > >> > > > > > sergi.vlady...@gmail.com> > >> > > > > > > wrote: > >> > > > > > > > >> > > > > > > > I believe we will not see this new SQL API soon. It is not > >> even > >> > > in > >> > > > > > design > >> > > > > > > > stage. > >> > > > > > > > > >> > > > > > > > The change proposed by Andrey is very simple and our users > >> will > >> > > > > benefit > >> > > > > > > > from it right away. > >> > > > > > > > > >> > > > > > > > I see no reasons to disallow this change. > >> > > > > > > > > >> > > > > > > > Sergi > >> > > > > > > > > >> > > > > > > > 2017-05-18 12:35 GMT+03:00 Vladimir Ozerov < > >> > voze...@gridgain.com > >> > > >: > >> > > > > > > > > >> > > > > > > > > Result set metadata is exposed to JDBC and ODBC drivers > >> > because > >> > > > it > >> > > > > is > >> > > > > > > > > required by JDBC specification and lot's external > >> > applications > >> > > > use > >> > > > > > it. > >> > > > > > > I > >> > > > > > > > do > >> > > > > > > > > not see big demand for this feature in native SQL, where > >> user > >> > > > > > normally > >> > > > > > > > > knows the model. Another point is that with changes > >> > introduced > >> > > in > >> > > > > > > recent > >> > > > > > > > > versions (DML, DDL, shared schemas), we need brand new > >> native > >> > > SQL > >> > > > > > API, > >> > > > > > > as > >> > > > > > > > > current IgniteCache.query() cannot conveniently reflect > >> > current > >> > > > and > >> > > > > > > > planned > >> > > > > > > > > Ignite capabilities. > >> > > > > > > > > > >> > > > > > > > > For this reason I do not think we should do proposed > >> change. > >> > > > > Instead, > >> > > > > > > we > >> > > > > > > > > should add metadata retrieval to new SQL API. > >> > > > > > > > > > >> > > > > > > > > Vladimir. > >> > > > > > > > > > >> > > > > > > > > On Thu, May 18, 2017 at 12:19 PM, Andrey Mashenkov < > >> > > > > > > > > andrey.mashen...@gmail.com> wrote: > >> > > > > > > > > > >> > > > > > > > > > Hi Igniters, > >> > > > > > > > > > > >> > > > > > > > > > When user run Sql query via JDBC, he can get fields > >> > metadata > >> > > > > (field > >> > > > > > > > > names, > >> > > > > > > > > > its types and etc.) from ResultSet. > >> > > > > > > > > > With IgniteCache.query method he gets some QueryCursor > >> > > > > > > implementation, > >> > > > > > > > > but > >> > > > > > > > > > QueryCursor interface doesn't have any methods for > this. > >> > > > > > > > > > > >> > > > > > > > > > For now, the only way to get metadata is try to cast > >> result > >> > > to > >> > > > > > > internal > >> > > > > > > > > > QueryCursorImpl class. > >> > > > > > > > > > > >> > > > > > > > > > I think it should break nothing if we overload > >> > > > > > > > > > IgniteCache.query(SqlFieldsQuery q) return type to a > >> new > >> > > > > > > > > FieldsQueryCursor > >> > > > > > > > > > interface. > >> > > > > > > > > > FieldsQueryCursor will be inherits from QueryCursor > and > >> > > provide > >> > > > > > > > > additional > >> > > > > > > > > > methods, > >> > > > > > > > > > > >> > > > > > > > > > Thoughts? > >> > > > > > > > > > > >> > > > > > > > > > -- > >> > > > > > > > > > Best regards, > >> > > > > > > > > > Andrey V. Mashenkov > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > > > > > > > > -- > > Best regards, > > Andrey V. Mashenkov > > > > > > -- > Best regards, > Andrey V. Mashenkov >
