Mailing-List: contact dev-help@ignite.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@ignite.incubator.apache.org
Received-SPF: pass (nike.apache.org: domain of vozerov@gridgain.com designates
 209.85.216.173 as permitted sender)
MIME-Version: 1.0
Date: Sat, 21 Mar 2015 00:38:51 +0300
Message-ID: 
 <CAGt6o8kgw2+w++Ws_pSi38XLK2eZCNwV2XN63NP3nDoWsqM18Q@mail.gmail.com>
Subject: Queries API considerations.
From: Vladimir Ozerov <vozerov@gridgain.com>
To: dev@ignite.incubator.apache.org
Content-Type: multipart/alternative; boundary=001a1132f7ea78966a0511bf24a6

--001a1132f7ea78966a0511bf24a6
Content-Type: text/plain; charset=UTF-8

Guys,

The more I look into our queries API, the more questions appears.

1) User can either iterate over result set, or get all returned values. In
the latter case query is automatically closed after "getAll()" finished.
But this is not the case for iterator. At least this is not mentioned is
JavaDocs. For this reason instead of doing:

for (CacheEntry entry : cache.query(...)) {
}

user will have to do the following:

try (Query qry = cache.query(...)) {
    for (CacheEntry entry : qry) {
    }
}

Looks like we have to close query whenever end of results stream is
reached, irrespective of whether this was getAll(), or iterator end.
Thoughts?

2) Initial query inside contnuous query appears to be fundamentally wrong
concept. Continuous query is about events. Regular query is about iteration
over existing enries. Why do we mix them? They have nothing in common.
If user want to run "initial query" ... just let him do that by hand after
continuous query is started.

Furthermore, IgniteCache.query() accepts SQL and TEXT queries.
ContinuousQuery.initialQuery accepts SQL, TEXT and FIELDS queries, WTF? How
are we going to explain user this semantics?

Furthermore, from running regular queries user knows that when
QueryCursor.getAll() is called, query is closed. But will continuous query
be closed when the user call getAll() on initial query result? No, it
won't. Another inconsistency.

Furthermore, when a QueryCursor returned from IgniteCache.continuousQuery()
method is closed, it closes both continuous and initial queries. But what
if I want to release initial query resources, but leave continuous query
running? It is impossible at the moment - either keep open both, or close
both.

I propose to remove initial query together with a cursor returned from
IgniteCache.continuousQuery(). It has no value. Instead, continuousQuery()
method should return a kind of "handle" which can be closed (e.g. extends
AutoCloseable), nothing more.

3) ContinuousQuery extends Query. Query have only one property - page size.
Continuous query does not use this. How come we expose not used property to
the user? ContinuousQuery should not extend Query as it has nothing common
with it except of a word "query" in name.

Also, as we distinguish entries queries (SQL, TEXT) and fields queries,
probably it does not make sense to make them share the same Query class.

What if we define the following hierarchy:
- Query, SqlQuery extends Query, TextQuery extends Query;
- SqlFieldsQuery
- ContinuousQuery

It will map perfectly to existing methods:
- query(Query)
- fieldsQuery(SqlFIeldsQuery)
- continuousQuery(ContinuousQuery).

4) Finally, instead of having paired methods "queryXXX" and
"localQueryXXX", we can add "local" property to query classes and remove
"localQueryXXX" methods from API.

Thoughts?

--001a1132f7ea78966a0511bf24a6--