You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by jp...@apache.org on 2023/01/13 13:44:23 UTC

[lucene] branch main updated: MultiCollector shouldn't report that scores are needed when they're not. (#12083)

This is an automated email from the ASF dual-hosted git repository.

jpountz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/lucene.git


The following commit(s) were added to refs/heads/main by this push:
     new b5062a28581 MultiCollector shouldn't report that scores are needed when they're not. (#12083)
b5062a28581 is described below

commit b5062a28581215fd6b3bde76cf4bf7d1c1e6160c
Author: Adrien Grand <jp...@gmail.com>
AuthorDate: Fri Jan 13 14:44:17 2023 +0100

    MultiCollector shouldn't report that scores are needed when they're not. (#12083)
    
    When sub collectors don't agree on their `ScoreMode`, `MultiCollector`
    currently returns `COMPLETE`. This makes sense when assuming that there is
    likely one collector computing top hits (`TOP_SCORES`) and another one
    computing facets (`COMPLETE_NO_SCORES`) so `COMPLETE` makes sense. However it
    is also possible to have one collector computing top hits by field (`TOP_DOCS`)
    and another one doing facets (`COMPLETE_NO_SCORES`), and `MultiCollector`
    shouldn't report that scores are needed in that case.
---
 .../java/org/apache/lucene/search/MultiCollector.java   |  8 +++++++-
 .../org/apache/lucene/search/TestMultiCollector.java    | 17 +++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/lucene/core/src/java/org/apache/lucene/search/MultiCollector.java b/lucene/core/src/java/org/apache/lucene/search/MultiCollector.java
index 5452c0f8d69..7f8e8121734 100644
--- a/lucene/core/src/java/org/apache/lucene/search/MultiCollector.java
+++ b/lucene/core/src/java/org/apache/lucene/search/MultiCollector.java
@@ -108,7 +108,13 @@ public class MultiCollector implements Collector {
       if (scoreMode == null) {
         scoreMode = collector.scoreMode();
       } else if (scoreMode != collector.scoreMode()) {
-        return ScoreMode.COMPLETE;
+        // If score modes disagree, we don't try to be smart and just use one of the COMPLETE score
+        // modes depending on whether scores are needed or not.
+        if (scoreMode.needsScores() || collector.scoreMode().needsScores()) {
+          scoreMode = ScoreMode.COMPLETE;
+        } else {
+          scoreMode = ScoreMode.COMPLETE_NO_SCORES;
+        }
       }
     }
     return scoreMode;
diff --git a/lucene/core/src/test/org/apache/lucene/search/TestMultiCollector.java b/lucene/core/src/test/org/apache/lucene/search/TestMultiCollector.java
index a8fc829bcc1..5e9a910f126 100644
--- a/lucene/core/src/test/org/apache/lucene/search/TestMultiCollector.java
+++ b/lucene/core/src/test/org/apache/lucene/search/TestMultiCollector.java
@@ -594,6 +594,23 @@ public class TestMultiCollector extends LuceneTestCase {
     dir.close();
   }
 
+  public void testMergeScoreModes() {
+    for (ScoreMode sm1 : ScoreMode.values()) {
+      for (ScoreMode sm2 : ScoreMode.values()) {
+        Collector c1 = new TerminatingDummyCollector(0, sm1);
+        Collector c2 = new TerminatingDummyCollector(0, sm2);
+        Collector c = MultiCollector.wrap(c1, c2);
+        if (sm1 == sm2) {
+          assertEquals(sm1, c.scoreMode());
+        } else if (sm1.needsScores() || sm2.needsScores()) {
+          assertEquals(ScoreMode.COMPLETE, c.scoreMode());
+        } else {
+          assertEquals(ScoreMode.COMPLETE_NO_SCORES, c.scoreMode());
+        }
+      }
+    }
+  }
+
   private static class TerminatingDummyCollector extends DummyCollector {
 
     private final int terminateOnDoc;