-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IGNITE-21445 New implementation for local IndexQuery #11317
base: master
Are you sure you want to change the base?
Conversation
...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryAdapter.java
Outdated
Show resolved
Hide resolved
...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryAdapter.java
Outdated
Show resolved
Hide resolved
...les/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheProxyImpl.java
Outdated
Show resolved
Hide resolved
...les/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheProxyImpl.java
Outdated
Show resolved
Hide resolved
...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryAdapter.java
Outdated
Show resolved
Hide resolved
...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java
Outdated
Show resolved
Hide resolved
...les/core/src/main/java/org/apache/ignite/internal/processors/cache/query/CacheQueryType.java
Outdated
Show resolved
Hide resolved
...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java
Outdated
Show resolved
Hide resolved
...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java
Outdated
Show resolved
Hide resolved
...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java
Outdated
Show resolved
Hide resolved
...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java
Outdated
Show resolved
Hide resolved
...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java
Outdated
Show resolved
Hide resolved
…rtition checks added + refactoring
…to one sequence of methods
…l#query(Query, ClusterGroup) + separate methods for local Scan and Index Queries
…tion correction, minor changes
…ording Index Query events added
…s moved to a different patch
…dCacheQueryManager
} | ||
}, true); | ||
|
||
return new QueryCursorImpl<>(iter); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QueryCursorImpl<>
-> QueryCursorImpl
there is no any class to inject to the generic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
Integer part = qry.partition(); | ||
|
||
if (!lostParts.isEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code isn't covered with tests. Let's remove it and add test and code in a separate ticket
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lost partitions check removed.
} | ||
} | ||
|
||
if (qry.nodes().isEmpty()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is already starts on local node, why do we need this check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IndexQueryLocalTest#testClientNodeReplicatedCache test (running a local index query on a client node) will fail if we don't do this check and throw a certain exception after it.
throw new IgniteException(new ClusterGroupEmptyException()); | ||
|
||
if (cctx.deploymentEnabled()) | ||
cctx.deploy().registerClasses(qry.scanFilter()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code doesn't covered by tests. For what purpose do you need this code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code removed.
@@ -3427,4 +3503,56 @@ IgniteBiPredicate<K, V> scanFilter() { | |||
return scanFilter; | |||
} | |||
} | |||
|
|||
/** */ | |||
public static final class IndexQueryIterator<K, V> extends GridCloseableIteratorAdapter<Object> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to private
.
return new IndexQueryIterator(idxQryRes.iter()); | ||
} | ||
catch (Exception e) { | ||
throw e; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Useless catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Catch section removed.
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
The Contribution Checklist
The description explains WHAT and WHY was made instead of HOW.
The following pattern must be used:
IGNITE-XXXX Change summary
whereXXXX
- number of JIRA issue.(see the Maintainers list)
the
green visa
attached to the JIRA ticket (see TC.Bot: Check PR)Notes
If you need any help, please email [email protected] or ask anу advice on http://asf.slack.com #ignite channel.