[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r337607084
##
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryFutureAdapter.java
##
@@ -327,9 +331,15 @@ protected void onNodeLeft(UUID evtNodeId) {
protected void enqueue(Collection col) {
assert Thread.holdsLock(this);
-queue.add((Collection)col);
-
-cnt.addAndGet(col.size());
+if(limitCnt <= 0 || limitCnt >= col.size()) {
+queue.add((Collection)col);
+cnt.addAndGet(col.size());
+limitCnt -= col.size();
+} else {
+int toAdd = limitCnt;
Review comment:
I've made a PR with the only one additional field
https://github.com/apache/ignite/pull/6997/
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r337537966
##
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryFutureAdapter.java
##
@@ -327,9 +331,15 @@ protected void onNodeLeft(UUID evtNodeId) {
protected void enqueue(Collection col) {
assert Thread.holdsLock(this);
-queue.add((Collection)col);
-
-cnt.addAndGet(col.size());
+if(limitCnt <= 0 || limitCnt >= col.size()) {
+queue.add((Collection)col);
+cnt.addAndGet(col.size());
+limitCnt -= col.size();
+} else {
+int toAdd = limitCnt;
Review comment:
This is true for local query, but isn't for distributed
.
enqueue() method is called on every page received from remote node.
Assume we receive 2 pages from 2 nodes and try to enqueue them concurrently.
Both pages will be trimmed and added to queue, but the only first is
expected to be added.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r337509475
##
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryFutureAdapter.java
##
@@ -327,9 +331,15 @@ protected void onNodeLeft(UUID evtNodeId) {
protected void enqueue(Collection col) {
assert Thread.holdsLock(this);
-queue.add((Collection)col);
-
-cnt.addAndGet(col.size());
+if(limitCnt <= 0 || limitCnt >= col.size()) {
+queue.add((Collection)col);
+cnt.addAndGet(col.size());
+limitCnt -= col.size();
+} else {
+int toAdd = limitCnt;
Review comment:
limitCnt should be decreased in 'else' case as well
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r332395577
##
File path:
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheFullTextQuerySelfTest.java
##
@@ -59,6 +59,22 @@
/** Cache name */
private static final String PERSON_CACHE = "Person";
+/** Limitation to query response size */
+private static final int QUERY_LIMIT = 5;
+
+/**
+ * Container for expected values and all available entries
+ */
+private static final class TestPair{
Review comment:
Missed space before '{'
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r332394278
##
File path:
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheFullTextQuerySelfTest.java
##
@@ -212,86 +255,84 @@ private static void clearCache(IgniteEx ignite) {
*
* @throws IgniteCheckedException if failed.
*/
-private static void validateQueryResults(IgniteEx ignite, Query qry,
Set exp,
+private static void validateQueryResults(IgniteEx ignite, TextQuery qry,
Set exp,
boolean keepBinary) throws IgniteCheckedException {
IgniteCache cache = ignite.cache(PERSON_CACHE);
if (keepBinary) {
IgniteCache cache0 = cache.withKeepBinary();
try (QueryCursor> cursor =
cache0.query(qry)) {
-Set exp0 = new HashSet<>(exp);
-
-List> all = new ArrayList<>();
-
-for (Cache.Entry entry :
cursor.getAll()) {
-all.add(entry);
-
-assertEquals(entry.getKey().toString(),
entry.getValue().field("name"));
-assertEquals(entry.getKey(),
entry.getValue().field("age"));
+TestPair testPair = processExpectedWithBinary(exp, cursor);
-exp0.remove(entry.getKey());
-}
-
-checkForMissedKeys(ignite, exp0, all);
+assertResult(ignite, qry, testPair);
}
try (QueryCursor> cursor =
cache0.query(qry)) {
-Set exp0 = new HashSet<>(exp);
-
-List> all = new ArrayList<>();
-
-for (Cache.Entry entry :
cursor.getAll()) {
-all.add(entry);
-assertEquals(entry.getKey().toString(),
entry.getValue().field("name"));
+TestPair testPair = processExpectedWithBinary(exp, cursor);
-assertEquals(entry.getKey(),
entry.getValue().field("age"));
-
-exp0.remove(entry.getKey());
-}
-
-checkForMissedKeys(ignite, exp0, all);
+assertResult(ignite, qry, testPair);
}
}
else {
try (QueryCursor> cursor =
cache.query(qry)) {
-Set exp0 = new HashSet<>(exp);
-List> all = new ArrayList<>();
+TestPair testPair = processExpected(exp, cursor);
-for (Cache.Entry entry : cursor.getAll()) {
-all.add(entry);
+assertResult(ignite, qry, testPair);
-assertEquals(entry.getKey().toString(),
entry.getValue().name);
+}
-assertEquals(entry.getKey(),
Integer.valueOf(entry.getValue().age));
+try (QueryCursor> cursor =
cache.query(qry)) {
-exp0.remove(entry.getKey());
-}
+TestPair testPair = processExpected(exp, cursor);
-checkForMissedKeys(ignite, exp0, all);
+assertResult(ignite, qry, testPair);
}
+}
+}
-try (QueryCursor> cursor =
cache.query(qry)) {
-Set exp0 = new HashSet<>(exp);
+private static void assertResult(IgniteEx ignite, TextQuery qry,
+TestPair testPair) throws IgniteCheckedException {
+if (qry.getLimit() > 0){
+assertTrue(testPair.all.size() <= QUERY_LIMIT);
Review comment:
Should be:
```
if (qry.getLimit() > 0)
assertTrue(testPair.all.size() <= QUERY_LIMIT);
else
checkForMissedKeys(ignite, testPair.expected, testPair.all);
```
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r331551796
##
File path:
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheFullTextQuerySelfTest.java
##
@@ -59,6 +59,19 @@
/** Cache name */
private static final String PERSON_CACHE = "Person";
+/** Limitation to query response size */
+private static final int QUERY_LIMIT = 5;
+
+private static final class TestPair{
+
+public final Set expected;
+public final List> all = new ArrayList<>();
+
+public TestPair(Set exp) {
+this.expected = new HashSet<>(exp);
+}
+}
Review comment:
This class also should have minimal javadoc.
https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r331550740
##
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/platform/cache/PlatformCache.java
##
@@ -1422,6 +1422,11 @@ private Query readTextQuery(BinaryRawReader reader) {
String typ = reader.readString();
final int pageSize = reader.readInt();
+//TODO uncomment when limit parameter is added to Platforms
Review comment:
Let's either create a ticket for this or remove commented code.
https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-TODOs
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery URL: https://github.com/apache/ignite/pull/6917#discussion_r331066651 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryRequest.java ## @@ -257,6 +260,7 @@ public GridCacheQueryRequest( GridCacheQueryType type, boolean fields, String clause, +int limit, Review comment: Missed javadoc. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r331072758
##
File path:
modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridLuceneIndex.java
##
@@ -241,7 +241,7 @@ public void remove(CacheObject key) throws
IgniteCheckedException {
* @throws IgniteCheckedException If failed.
*/
public GridCloseableIterator> query(String qry,
-IndexingQueryFilter filters) throws IgniteCheckedException {
+IndexingQueryFilter filters, int limit) throws IgniteCheckedException {
Review comment:
Missed javadoc.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r331074808
##
File path:
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheFullTextQuerySelfTest.java
##
@@ -212,86 +255,84 @@ private static void clearCache(IgniteEx ignite) {
*
* @throws IgniteCheckedException if failed.
*/
-private static void validateQueryResults(IgniteEx ignite, Query qry,
Set exp,
+private static void validateQueryResults(IgniteEx ignite, TextQuery qry,
Set exp,
boolean keepBinary) throws IgniteCheckedException {
IgniteCache cache = ignite.cache(PERSON_CACHE);
if (keepBinary) {
IgniteCache cache0 = cache.withKeepBinary();
try (QueryCursor> cursor =
cache0.query(qry)) {
-Set exp0 = new HashSet<>(exp);
-
-List> all = new ArrayList<>();
-
-for (Cache.Entry entry :
cursor.getAll()) {
-all.add(entry);
-
-assertEquals(entry.getKey().toString(),
entry.getValue().field("name"));
-assertEquals(entry.getKey(),
entry.getValue().field("age"));
+TestPair testPair = processExpectedWithBinary(exp, cursor);
-exp0.remove(entry.getKey());
-}
-
-checkForMissedKeys(ignite, exp0, all);
+assertResult(ignite, qry, testPair);
}
try (QueryCursor> cursor =
cache0.query(qry)) {
-Set exp0 = new HashSet<>(exp);
-
-List> all = new ArrayList<>();
-
-for (Cache.Entry entry :
cursor.getAll()) {
-all.add(entry);
-assertEquals(entry.getKey().toString(),
entry.getValue().field("name"));
+TestPair testPair = processExpectedWithBinary(exp, cursor);
-assertEquals(entry.getKey(),
entry.getValue().field("age"));
-
-exp0.remove(entry.getKey());
-}
-
-checkForMissedKeys(ignite, exp0, all);
+assertResult(ignite, qry, testPair);
}
}
else {
try (QueryCursor> cursor =
cache.query(qry)) {
-Set exp0 = new HashSet<>(exp);
-List> all = new ArrayList<>();
+TestPair testPair = processExpected(exp, cursor);
-for (Cache.Entry entry : cursor.getAll()) {
-all.add(entry);
+assertResult(ignite, qry, testPair);
-assertEquals(entry.getKey().toString(),
entry.getValue().name);
+}
-assertEquals(entry.getKey(),
Integer.valueOf(entry.getValue().age));
+try (QueryCursor> cursor =
cache.query(qry)) {
-exp0.remove(entry.getKey());
-}
+TestPair testPair = processExpected(exp, cursor);
-checkForMissedKeys(ignite, exp0, all);
+assertResult(ignite, qry, testPair);
}
+}
+}
-try (QueryCursor> cursor =
cache.query(qry)) {
-Set exp0 = new HashSet<>(exp);
+private static void assertResult(IgniteEx ignite, TextQuery qry,
+TestPair testPair) throws IgniteCheckedException {
+if (qry.getLimit() > 0){
+assertTrue(testPair.all.size() <= QUERY_LIMIT);
Review comment:
Braces for one-liners are redundant.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r331074256
##
File path:
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheFullTextQuerySelfTest.java
##
@@ -212,86 +255,84 @@ private static void clearCache(IgniteEx ignite) {
*
* @throws IgniteCheckedException if failed.
*/
-private static void validateQueryResults(IgniteEx ignite, Query qry,
Set exp,
+private static void validateQueryResults(IgniteEx ignite, TextQuery qry,
Set exp,
boolean keepBinary) throws IgniteCheckedException {
IgniteCache cache = ignite.cache(PERSON_CACHE);
if (keepBinary) {
IgniteCache cache0 = cache.withKeepBinary();
try (QueryCursor> cursor =
cache0.query(qry)) {
-Set exp0 = new HashSet<>(exp);
-
-List> all = new ArrayList<>();
-
-for (Cache.Entry entry :
cursor.getAll()) {
-all.add(entry);
-
-assertEquals(entry.getKey().toString(),
entry.getValue().field("name"));
-assertEquals(entry.getKey(),
entry.getValue().field("age"));
+TestPair testPair = processExpectedWithBinary(exp, cursor);
-exp0.remove(entry.getKey());
-}
-
-checkForMissedKeys(ignite, exp0, all);
+assertResult(ignite, qry, testPair);
}
try (QueryCursor> cursor =
cache0.query(qry)) {
-Set exp0 = new HashSet<>(exp);
-
-List> all = new ArrayList<>();
-
-for (Cache.Entry entry :
cursor.getAll()) {
-all.add(entry);
-assertEquals(entry.getKey().toString(),
entry.getValue().field("name"));
+TestPair testPair = processExpectedWithBinary(exp, cursor);
-assertEquals(entry.getKey(),
entry.getValue().field("age"));
-
-exp0.remove(entry.getKey());
-}
-
-checkForMissedKeys(ignite, exp0, all);
+assertResult(ignite, qry, testPair);
}
}
else {
try (QueryCursor> cursor =
cache.query(qry)) {
-Set exp0 = new HashSet<>(exp);
-List> all = new ArrayList<>();
+TestPair testPair = processExpected(exp, cursor);
-for (Cache.Entry entry : cursor.getAll()) {
-all.add(entry);
+assertResult(ignite, qry, testPair);
-assertEquals(entry.getKey().toString(),
entry.getValue().name);
+}
-assertEquals(entry.getKey(),
Integer.valueOf(entry.getValue().age));
+try (QueryCursor> cursor =
cache.query(qry)) {
-exp0.remove(entry.getKey());
-}
+TestPair testPair = processExpected(exp, cursor);
-checkForMissedKeys(ignite, exp0, all);
+assertResult(ignite, qry, testPair);
}
+}
+}
-try (QueryCursor> cursor =
cache.query(qry)) {
-Set exp0 = new HashSet<>(exp);
+private static void assertResult(IgniteEx ignite, TextQuery qry,
+TestPair testPair) throws IgniteCheckedException {
+if (qry.getLimit() > 0){
+assertTrue(testPair.all.size() <= QUERY_LIMIT);
+} else {
+checkForMissedKeys(ignite, testPair.expected, testPair.all);
+}
+}
-List> all = new ArrayList<>();
+@NotNull private static GridCacheFullTextQuerySelfTest.TestPair
processExpectedWithBinary(Set exp,
+QueryCursor> cursor) {
+TestPair testPair = new TestPair(exp);
-for (Cache.Entry entry : cursor.getAll()) {
-all.add(entry);
+for (Cache.Entry entry : cursor.getAll()) {
+testPair.all.add(entry);
-assertEquals(entry.getKey().toString(),
entry.getValue().name);
+assertEquals(entry.getKey().toString(),
entry.getValue().field("name"));
-assertEquals(entry.getKey().intValue(),
entry.getValue().age);
+assertEquals(entry.getKey(), entry.getValue().field("age"));
-exp0.remove(entry.getKey());
-}
+testPair.expected.remove(entry.getKey());
+}
+return testPair;
+}
-checkForMissedKeys(ignite, exp0, all);
-}
+@NotNull private static GridCacheFullTextQuerySelfTest.TestPair
processExpected(Set exp,
Review comment:
Missed javadoc.
Method modificators order.
This is an automated message from the Apache Git Service.
To respond to the
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r331064991
##
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java
##
@@ -1951,7 +1951,9 @@ public String cacheName() {
*/
@GridInternal
private static class MetadataJob implements
IgniteCallable> {
-/** */
+/**
+ *
Review comment:
Let's keep one-lined empty javadoc.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery URL: https://github.com/apache/ignite/pull/6917#discussion_r330026453 ## File path: modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheFullTextQuerySelfTest.java ## @@ -58,6 +58,17 @@ /** Cache name */ private static final String PERSON_CACHE = "Person"; +private static final int QUERY_LIMIT = 5; Review comment: Missed javadocs 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r331074178
##
File path:
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheFullTextQuerySelfTest.java
##
@@ -212,86 +255,84 @@ private static void clearCache(IgniteEx ignite) {
*
* @throws IgniteCheckedException if failed.
*/
-private static void validateQueryResults(IgniteEx ignite, Query qry,
Set exp,
+private static void validateQueryResults(IgniteEx ignite, TextQuery qry,
Set exp,
boolean keepBinary) throws IgniteCheckedException {
IgniteCache cache = ignite.cache(PERSON_CACHE);
if (keepBinary) {
IgniteCache cache0 = cache.withKeepBinary();
try (QueryCursor> cursor =
cache0.query(qry)) {
-Set exp0 = new HashSet<>(exp);
-
-List> all = new ArrayList<>();
-
-for (Cache.Entry entry :
cursor.getAll()) {
-all.add(entry);
-
-assertEquals(entry.getKey().toString(),
entry.getValue().field("name"));
-assertEquals(entry.getKey(),
entry.getValue().field("age"));
+TestPair testPair = processExpectedWithBinary(exp, cursor);
-exp0.remove(entry.getKey());
-}
-
-checkForMissedKeys(ignite, exp0, all);
+assertResult(ignite, qry, testPair);
}
try (QueryCursor> cursor =
cache0.query(qry)) {
-Set exp0 = new HashSet<>(exp);
-
-List> all = new ArrayList<>();
-
-for (Cache.Entry entry :
cursor.getAll()) {
-all.add(entry);
-assertEquals(entry.getKey().toString(),
entry.getValue().field("name"));
+TestPair testPair = processExpectedWithBinary(exp, cursor);
-assertEquals(entry.getKey(),
entry.getValue().field("age"));
-
-exp0.remove(entry.getKey());
-}
-
-checkForMissedKeys(ignite, exp0, all);
+assertResult(ignite, qry, testPair);
}
}
else {
try (QueryCursor> cursor =
cache.query(qry)) {
-Set exp0 = new HashSet<>(exp);
-List> all = new ArrayList<>();
+TestPair testPair = processExpected(exp, cursor);
-for (Cache.Entry entry : cursor.getAll()) {
-all.add(entry);
+assertResult(ignite, qry, testPair);
-assertEquals(entry.getKey().toString(),
entry.getValue().name);
+}
-assertEquals(entry.getKey(),
Integer.valueOf(entry.getValue().age));
+try (QueryCursor> cursor =
cache.query(qry)) {
-exp0.remove(entry.getKey());
-}
+TestPair testPair = processExpected(exp, cursor);
-checkForMissedKeys(ignite, exp0, all);
+assertResult(ignite, qry, testPair);
}
+}
+}
-try (QueryCursor> cursor =
cache.query(qry)) {
-Set exp0 = new HashSet<>(exp);
+private static void assertResult(IgniteEx ignite, TextQuery qry,
+TestPair testPair) throws IgniteCheckedException {
+if (qry.getLimit() > 0){
+assertTrue(testPair.all.size() <= QUERY_LIMIT);
+} else {
+checkForMissedKeys(ignite, testPair.expected, testPair.all);
+}
+}
-List> all = new ArrayList<>();
+@NotNull private static GridCacheFullTextQuerySelfTest.TestPair
processExpectedWithBinary(Set exp,
Review comment:
Missed javadoc.
Method modificators order.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r331065213
##
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java
##
@@ -2751,19 +2754,19 @@ public int size() {
* @return Created query.
*/
public CacheQuery> createFullTextQuery(String clsName,
-String search, boolean keepBinary) {
+String search, int limit, boolean keepBinary) {
Review comment:
Missed javadoc for 'limit' param
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r331072273
##
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java
##
@@ -2712,7 +2712,7 @@ public void remove(GridCacheContext cctx, CacheDataRow
row)
* @throws IgniteCheckedException If failed.
*/
public GridCloseableIterator> queryText(final
String cacheName, final String clause,
-final String resType, final IndexingQueryFilter filters) throws
IgniteCheckedException {
+final String resType, final IndexingQueryFilter filters, int limit)
throws IgniteCheckedException {
Review comment:
Missed javadoc.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r331064296
##
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryFutureAdapter.java
##
@@ -327,9 +334,14 @@ protected void onNodeLeft(UUID evtNodeId) {
protected void enqueue(Collection col) {
assert Thread.holdsLock(this);
-queue.add((Collection)col);
-
-cnt.addAndGet(col.size());
+if(limit <= 0 || total.addAndGet(col.size()) <= limit) {
Review comment:
Is it possible to rewrite this with no adding 'total' field, but using
CAS-loop or cnt.getAndUpdate() ?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r331065732
##
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheProxyImpl.java
##
@@ -520,13 +520,13 @@ public CountDownLatch getInitLatch(){
}
/**
- * @param filter Filter.
+ * @param query Filter.
Review comment:
Javadoc is incorrect.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery URL: https://github.com/apache/ignite/pull/6917#discussion_r331072228 ## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryIndexing.java ## @@ -138,7 +138,7 @@ public long streamUpdateQuery(String schemaName, String qry, @Nullable Object[] * @throws IgniteCheckedException If failed. */ public GridCloseableIterator> queryLocalText(String schemaName, String cacheName, -String qry, String typeName, IndexingQueryFilter filter) throws IgniteCheckedException; +String qry, String typeName, IndexingQueryFilter filter, int limit) throws IgniteCheckedException; Review comment: Missed javadoc. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r331070480
##
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/platform/cache/PlatformCache.java
##
@@ -1420,9 +1420,10 @@ private Query readTextQuery(BinaryRawReader reader) {
boolean loc = reader.readBoolean();
String txt = reader.readString();
String typ = reader.readString();
+final int limit = reader.readInt();
final int pageSize = reader.readInt();
-return new TextQuery(typ, txt).setPageSize(pageSize).setLocal(loc);
+return new TextQuery(typ, txt,
limit).setPageSize(pageSize).setLocal(loc);
Review comment:
Platforms do not support 'limit' param yet.
So, attempt to read it can lead an error.
Let's set a default value here and create a separate ticket for the issue.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r331061172
##
File path:
modules/core/src/main/java/org/apache/ignite/cache/query/TextQuery.java
##
@@ -79,6 +82,19 @@ public TextQuery(String type, String txt) {
setText(txt);
}
+/**
+ * Constructs query for the given search string.
+ *
+ * @param type Type.
+ * @param txt Search string.
+ * @param limit Limit.
Review comment:
This is a public API class.
Let's make javadoc more verbose for 'limit' param at least.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] AMashenkov commented on a change in pull request #6917: IGNITE-12189 Implementation correct limit for TextQuery
AMashenkov commented on a change in pull request #6917: IGNITE-12189
Implementation correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6917#discussion_r331073922
##
File path:
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheFullTextQuerySelfTest.java
##
@@ -212,86 +255,84 @@ private static void clearCache(IgniteEx ignite) {
*
* @throws IgniteCheckedException if failed.
*/
-private static void validateQueryResults(IgniteEx ignite, Query qry,
Set exp,
+private static void validateQueryResults(IgniteEx ignite, TextQuery qry,
Set exp,
boolean keepBinary) throws IgniteCheckedException {
IgniteCache cache = ignite.cache(PERSON_CACHE);
if (keepBinary) {
IgniteCache cache0 = cache.withKeepBinary();
try (QueryCursor> cursor =
cache0.query(qry)) {
-Set exp0 = new HashSet<>(exp);
-
-List> all = new ArrayList<>();
-
-for (Cache.Entry entry :
cursor.getAll()) {
-all.add(entry);
-
-assertEquals(entry.getKey().toString(),
entry.getValue().field("name"));
-assertEquals(entry.getKey(),
entry.getValue().field("age"));
+TestPair testPair = processExpectedWithBinary(exp, cursor);
-exp0.remove(entry.getKey());
-}
-
-checkForMissedKeys(ignite, exp0, all);
+assertResult(ignite, qry, testPair);
}
try (QueryCursor> cursor =
cache0.query(qry)) {
-Set exp0 = new HashSet<>(exp);
-
-List> all = new ArrayList<>();
-
-for (Cache.Entry entry :
cursor.getAll()) {
-all.add(entry);
-assertEquals(entry.getKey().toString(),
entry.getValue().field("name"));
+TestPair testPair = processExpectedWithBinary(exp, cursor);
-assertEquals(entry.getKey(),
entry.getValue().field("age"));
-
-exp0.remove(entry.getKey());
-}
-
-checkForMissedKeys(ignite, exp0, all);
+assertResult(ignite, qry, testPair);
}
}
else {
try (QueryCursor> cursor =
cache.query(qry)) {
-Set exp0 = new HashSet<>(exp);
-List> all = new ArrayList<>();
+TestPair testPair = processExpected(exp, cursor);
-for (Cache.Entry entry : cursor.getAll()) {
-all.add(entry);
+assertResult(ignite, qry, testPair);
-assertEquals(entry.getKey().toString(),
entry.getValue().name);
+}
-assertEquals(entry.getKey(),
Integer.valueOf(entry.getValue().age));
+try (QueryCursor> cursor =
cache.query(qry)) {
-exp0.remove(entry.getKey());
-}
+TestPair testPair = processExpected(exp, cursor);
-checkForMissedKeys(ignite, exp0, all);
+assertResult(ignite, qry, testPair);
}
+}
+}
-try (QueryCursor> cursor =
cache.query(qry)) {
-Set exp0 = new HashSet<>(exp);
+private static void assertResult(IgniteEx ignite, TextQuery qry,
Review comment:
Missed javadoc
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
