zacharymorn commented on code in PR #1018:
URL: https://github.com/apache/lucene/pull/1018#discussion_r924065392


##########
lucene/core/src/java/org/apache/lucene/search/BooleanWeight.java:
##########
@@ -191,6 +191,66 @@ public long cost() {
   // or null if it is not applicable
   // pkg-private for forcing use of BooleanScorer in tests
   BulkScorer optionalBulkScorer(LeafReaderContext context) throws IOException {
+    if (scoreMode == ScoreMode.TOP_SCORES) {
+      if (query.getMinimumNumberShouldMatch() > 1 || weightedClauses.size() > 
2) {
+        return null;
+      }
+
+      List<ScorerSupplier> optional = new ArrayList<>();
+      for (WeightedBooleanClause wc : weightedClauses) {
+        Weight w = wc.weight;
+        BooleanClause c = wc.clause;
+        if (c.getOccur() != Occur.SHOULD) {
+          continue;
+        }
+        ScorerSupplier scorer = w.scorerSupplier(context);
+        if (scorer != null) {
+          optional.add(scorer);
+        }
+      }
+
+      if (optional.size() <= 1) {
+        return null;
+      }
+
+      List<Scorer> optionalScorers = new ArrayList<>();
+      for (ScorerSupplier ss : optional) {
+        optionalScorers.add(ss.get(Long.MAX_VALUE));
+      }
+
+      return new BulkScorer() {
+        final Scorer bmmScorer = new 
BlockMaxMaxscoreScorer(BooleanWeight.this, optionalScorers);
+        final int maxDoc = context.reader().maxDoc();
+        final DocIdSetIterator iterator = bmmScorer.iterator();
+
+        @Override
+        public int score(LeafCollector collector, Bits acceptDocs, int min, 
int max)
+            throws IOException {
+          collector.setScorer(bmmScorer);
+
+          int doc = min;
+          while (true) {
+            doc = iterator.advance(doc);

Review Comment:
   > If the last match of the first window is say 998 and the first match after 
the first window is 1005. Then we should make sure to score 1005 when scoring 
the second window before starting to advance.
   
   Thanks for the explanation, it makes sense! I thought the old implementation 
would take care of this boundary case as it initiated `doc` with `min` before 
advancing, but the behavior is indeed undefined if the previous call to 
`bulkScorer#score` already advanced its internal scorer past the next `min` 
used. I've updated the code with the above solution. 



##########
lucene/core/src/java/org/apache/lucene/search/BooleanWeight.java:
##########
@@ -191,6 +191,66 @@ public long cost() {
   // or null if it is not applicable
   // pkg-private for forcing use of BooleanScorer in tests
   BulkScorer optionalBulkScorer(LeafReaderContext context) throws IOException {
+    if (scoreMode == ScoreMode.TOP_SCORES) {
+      if (query.getMinimumNumberShouldMatch() > 1 || weightedClauses.size() > 
2) {
+        return null;
+      }
+
+      List<ScorerSupplier> optional = new ArrayList<>();
+      for (WeightedBooleanClause wc : weightedClauses) {
+        Weight w = wc.weight;
+        BooleanClause c = wc.clause;
+        if (c.getOccur() != Occur.SHOULD) {
+          continue;
+        }
+        ScorerSupplier scorer = w.scorerSupplier(context);
+        if (scorer != null) {
+          optional.add(scorer);
+        }
+      }
+
+      if (optional.size() <= 1) {
+        return null;
+      }
+
+      List<Scorer> optionalScorers = new ArrayList<>();
+      for (ScorerSupplier ss : optional) {
+        optionalScorers.add(ss.get(Long.MAX_VALUE));
+      }
+
+      return new BulkScorer() {
+        final Scorer bmmScorer = new 
BlockMaxMaxscoreScorer(BooleanWeight.this, optionalScorers);
+        final int maxDoc = context.reader().maxDoc();
+        final DocIdSetIterator iterator = bmmScorer.iterator();
+
+        @Override
+        public int score(LeafCollector collector, Bits acceptDocs, int min, 
int max)
+            throws IOException {
+          collector.setScorer(bmmScorer);
+
+          int doc = min;
+          while (true) {
+            doc = iterator.advance(doc);
+
+            if (doc >= max) {
+              return doc;
+            }
+
+            if (acceptDocs == null || acceptDocs.get(doc)) {
+              collector.collect(doc);
+            }
+
+            doc++;
+          }
+        }
+
+        @Override
+        public long cost() {
+          return maxDoc;

Review Comment:
   Updated.



-- 
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-unsubscr...@lucene.apache.org

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


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

Reply via email to