[GitHub] [lucene] jpountz commented on a change in pull request #612: LUCENE-10002: Replace simple usages of TotalHitCountCollector with IndexSearcher#count

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

jpountz commented on a change in pull request #612:
URL: https://github.com/apache/lucene/pull/612#discussion_r788730995



##########
File path: lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java
##########
@@ -1174,7 +1172,7 @@ public void testDetectMutatedQueries() throws IOException {
 
     try {
       // trigger an eviction
-      searcher.search(new MatchAllDocsQuery(), new TotalHitCountCollector());
+      searcher.count(new MatchAllDocsQuery());

Review comment:
       Hmm these changes look like they could change the behavior since `IndexSearcher#count` might by-pass the query cache entirely when it can get counts from index statistics.

##########
File path: lucene/core/src/java/org/apache/lucene/search/TotalHitCountCollector.java
##########
@@ -16,7 +16,11 @@
  */
 package org.apache.lucene.search;
 
-/** Just counts the total number of hits. */
+/**
+ * Just counts the total number of hits. For cases when this is the only collector used, {@link
+ * IndexSearcher#count(Query)} can be called instead of {@link IndexSearcher#search(Query,
+ * Collector)} which will automatically create an instance of this collector internally

Review comment:
       I would make this statement stronger since IndexSearcher#count is sometimes going to be faster, e.g. by returning the match count directly from index statistics.

##########
File path: lucene/core/src/test/org/apache/lucene/search/TestTermQuery.java
##########
@@ -91,16 +91,12 @@ public void testCreateWeightDoesNotSeekIfScoresAreNotNeeded() throws IOException
     IndexSearcher searcher = new IndexSearcher(reader);
     // use a collector rather than searcher.count() which would just read the
     // doc freq instead of creating a scorer
-    TotalHitCountCollector collector = new TotalHitCountCollector();
-    searcher.search(query, collector);
-    assertEquals(1, collector.getTotalHits());
+    assertEquals(1, searcher.count(query));

Review comment:
       See the above comment, we want to keep calling the collector here.




-- 
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: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org