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
