[GitHub] [ignite] AMashenkov commented on a change in pull request #6997: IGNITE-12189: Implement correct limit for TextQuery

[GitBox <gi...@apache.org>]

AMashenkov commented on a change in pull request #6997: IGNITE-12189: Implement correct limit for TextQuery
URL: https://github.com/apache/ignite/pull/6997#discussion_r337949185
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryFutureAdapter.java
 ##########
 @@ -57,6 +57,9 @@
     /** Logger reference. */
     private static final AtomicReference<IgniteLogger> logRef = new AtomicReference<>();
 
+    /** */
+    private static final int NO_LIMIT = -1;
 
 Review comment:
   Actually this magic constant belong to Lucene code and can be changed in any next Lucene version (e.g. to Long.MAX_VALUE). From architectural point Ignite code shouldn't depends on this constant (and any other Lucene abstracts as well) because this may lead us to unwanted changes and issues in future if Lucene guys will change their contracts.
   
   I'd remove any mention of special value Integer.MAX_VALUE from javadocs and bound magic constant usage to method with Lucene call only. In Ignite code we can use 'zero' for 'unlimit'.
   
   What about 'GridCacheQueryFutureAdapter', NO_LIMIT=-1 is a poison value here for private usage only. This can be safely changed to some flag that will indicate 'unlimited' e.g. if negative values will make sense at once. Thanks to incapsulation principle. 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services