From notifications-return-7533-archive-asf-public=cust-asf.ponee.io@ignite.apache.org Wed Oct 23 09:46:31 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 21569180608 for ; Wed, 23 Oct 2019 11:46:31 +0200 (CEST) Received: (qmail 69294 invoked by uid 500); 23 Oct 2019 09:46:30 -0000 Mailing-List: contact notifications-help@ignite.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@ignite.apache.org Delivered-To: mailing list notifications@ignite.apache.org Received: (qmail 69285 invoked by uid 99); 23 Oct 2019 09:46:30 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 23 Oct 2019 09:46:30 +0000 From: GitBox To: notifications@ignite.apache.org Subject: [GitHub] [ignite] AMashenkov commented on a change in pull request #6997: IGNITE-12189: Implement correct limit for TextQuery Message-ID: <157182399048.29839.5481170193392056129.gitbox@gitbox.apache.org> Date: Wed, 23 Oct 2019 09:46:30 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit 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 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. Anyway, thanks for pointing. You can make same changes in your PR along with javadoc fixes, and let go further with merging. ---------------------------------------------------------------- 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