ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrey Mashenkov <andrey.mashen...@gmail.com>
Subject Re: SqlFields query result does not expose fields metadata.
Date Tue, 23 May 2017 10:48:05 GMT
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.mashenkov@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 <dsetrakyan@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 <vozerov@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.vladykin@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 <vozerov@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.vladykin@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 <vozerov@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 <
>> > > ptupitsyn@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.vladykin@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 <
>> > vozerov@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.mashenkov@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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message