timoninmaxim commented on code in PR #12376:
URL: https://github.com/apache/ignite/pull/12376#discussion_r2387409750
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java:
##########
@@ -699,46 +697,7 @@ private FieldsResult executeFieldsQuery(CacheQuery<?> qry,
@Nullable Object[] ar
resKey = null; // Failed to cache result.
}
else {
Review Comment:
Now you can remove the `if` condition at all.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java:
##########
@@ -699,46 +697,7 @@ private FieldsResult executeFieldsQuery(CacheQuery<?> qry,
@Nullable Object[] ar
resKey = null; // Failed to cache result.
}
else {
- assert qry.type() == SPI : "Unexpected query type: " + qry.type();
-
- if (cctx.events().isRecordable(EVT_CACHE_QUERY_EXECUTED)) {
- cctx.gridEvents().record(new CacheQueryExecutedEvent<>(
- cctx.localNode(),
- "SPI query executed.",
- EVT_CACHE_QUERY_EXECUTED,
- CacheQueryType.SPI.name(),
- cctx.name(),
- null,
- null,
- null,
- null,
- args,
- securitySubjectId(cctx),
- taskName));
- }
-
- res = new FieldsResult(rcpt);
- }
-
- try {
- if (qry.type() == SPI) {
- IgniteSpiCloseableIterator<?> iter =
cctx.kernalContext().indexing().query(cacheName, F.asList(args),
- filter(qry));
-
- res.onDone(iter);
- }
- else {
- assert qry.type() == SQL_FIELDS;
-
- throw new IllegalStateException("Should never be called.");
- }
- }
- catch (Exception e) {
- res.onDone(e);
- }
- finally {
- if (resKey != null)
- qryResCache.remove(resKey, res);
+ throw new IllegalStateException("Unexpected query type: " +
qry.type());
Review Comment:
Let's add assert in beginning of the method, like:
`assert qry.type() == SQL_FIELDS : "Unexpected query type: " + qry.type();`
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java:
##########
@@ -660,7 +658,7 @@ private FieldsResult executeFieldsQuery(CacheQuery<?> qry,
@Nullable Object[] ar
T2<String, List<Object>> resKey = null;
Review Comment:
Move `resKey` declaration to its definition. Similar to the `res` variable
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheProxyImpl.java:
##########
@@ -905,7 +886,7 @@ private void validate(Query qry) {
if (!QueryUtils.isEnabled(ctx.config()) && !(qry instanceof ScanQuery)
&&
!(qry instanceof ContinuousQuery) && !(qry instanceof
ContinuousQueryWithTransformer) &&
- !(qry instanceof SpiQuery) && !(qry instanceof SqlQuery) && !(qry
instanceof SqlFieldsQuery) &&
Review Comment:
We should keep the check for `SqlQuery`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
