You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2020/10/06 17:27:10 UTC

[lucene-solr] 01/02: LUCENE-9565 Fix competitive iteration (#1952)

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

mayya pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 16d25ace3f4c273d45284c0e7ebf9915b245995a
Author: Mayya Sharipova <ma...@elastic.co>
AuthorDate: Tue Oct 6 13:22:16 2020 -0400

    LUCENE-9565 Fix competitive iteration (#1952)
    
    PR #1351 introduced a sort optimization where documents can be skipped.
    But iteration over competitive iterators was not properly organized,
    as they were not storing the current docID, and
    when competitive iterator was updated the current doc ID was lost.
    
    This patch fixed it.
    
    Relates to #1351
---
 .../src/java/org/apache/lucene/search/Weight.java  | 55 ++++++++++++++++++++--
 .../lucene/search/comparators/DocComparator.java   |  8 ++--
 .../search/comparators/NumericComparator.java      |  8 ++--
 3 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/lucene/core/src/java/org/apache/lucene/search/Weight.java b/lucene/core/src/java/org/apache/lucene/search/Weight.java
index d2a0428..75acefe 100644
--- a/lucene/core/src/java/org/apache/lucene/search/Weight.java
+++ b/lucene/core/src/java/org/apache/lucene/search/Weight.java
@@ -217,9 +217,17 @@ public abstract class Weight implements SegmentCacheable {
       collector.setScorer(scorer);
       DocIdSetIterator scorerIterator = twoPhase == null ? iterator : twoPhase.approximation();
       DocIdSetIterator collectorIterator = collector.competitiveIterator();
-      // if possible filter scorerIterator to keep only competitive docs as defined by collector
-      DocIdSetIterator filteredIterator = collectorIterator == null ? scorerIterator :
-          ConjunctionDISI.intersectIterators(Arrays.asList(scorerIterator, collectorIterator));
+      DocIdSetIterator filteredIterator;
+      if (collectorIterator == null) {
+        filteredIterator = scorerIterator;
+      } else {
+        if (scorerIterator.docID() != -1) {
+          // Wrap ScorerIterator to start from -1 for conjunction 
+          scorerIterator = new RangeDISIWrapper(scorerIterator, max);
+        }
+        // filter scorerIterator to keep only competitive docs as defined by collector
+        filteredIterator = ConjunctionDISI.intersectIterators(Arrays.asList(scorerIterator, collectorIterator));
+      }
       if (filteredIterator.docID() == -1 && min == 0 && max == DocIdSetIterator.NO_MORE_DOCS) {
         scoreAll(collector, filteredIterator, twoPhase, acceptDocs);
         return DocIdSetIterator.NO_MORE_DOCS;
@@ -279,4 +287,45 @@ public abstract class Weight implements SegmentCacheable {
     }
   }
 
+  /**
+   * Wraps an internal docIdSetIterator for it to start with docID = -1
+   */
+  protected static class RangeDISIWrapper extends DocIdSetIterator {
+    private final DocIdSetIterator in;
+    private final int min;
+    private final int max;
+    private int docID = -1;
+
+    public RangeDISIWrapper(DocIdSetIterator in, int max) {
+      this.in = in;
+      this.min = in.docID();
+      this.max = max;
+    }
+
+    @Override
+    public int docID() {
+      return docID;
+    }
+
+    @Override
+    public int nextDoc() throws IOException {
+      return advance(docID + 1);
+    }
+
+    @Override
+    public int advance(int target) throws IOException {
+      target = Math.max(min, target);
+      if (target >= max) {
+        return docID = NO_MORE_DOCS;
+      }
+      return docID = in.advance(target);
+    }
+
+    @Override
+    public long cost() {
+      return Math.min(max - min, in.cost());
+    }
+
+  }
+
 }
diff --git a/lucene/core/src/java/org/apache/lucene/search/comparators/DocComparator.java b/lucene/core/src/java/org/apache/lucene/search/comparators/DocComparator.java
index 8974ca6..c0b3e2e 100644
--- a/lucene/core/src/java/org/apache/lucene/search/comparators/DocComparator.java
+++ b/lucene/core/src/java/org/apache/lucene/search/comparators/DocComparator.java
@@ -133,14 +133,16 @@ public class DocComparator extends FieldComparator<Integer> {
                 return null;
             } else {
                 return new DocIdSetIterator() {
+                    private int docID = -1;
+
                     @Override
                     public int nextDoc() throws IOException {
-                        return competitiveIterator.nextDoc();
+                        return advance(docID + 1);
                     }
 
                     @Override
                     public int docID() {
-                        return competitiveIterator.docID();
+                        return docID;
                     }
 
                     @Override
@@ -150,7 +152,7 @@ public class DocComparator extends FieldComparator<Integer> {
 
                     @Override
                     public int advance(int target) throws IOException {
-                        return competitiveIterator.advance(target);
+                        return docID = competitiveIterator.advance(target);
                     }
                 };
             }
diff --git a/lucene/core/src/java/org/apache/lucene/search/comparators/NumericComparator.java b/lucene/core/src/java/org/apache/lucene/search/comparators/NumericComparator.java
index 7f62f66..7ed7511 100644
--- a/lucene/core/src/java/org/apache/lucene/search/comparators/NumericComparator.java
+++ b/lucene/core/src/java/org/apache/lucene/search/comparators/NumericComparator.java
@@ -228,14 +228,16 @@ public abstract class NumericComparator<T extends Number> extends FieldComparato
         public DocIdSetIterator competitiveIterator() {
             if (enableSkipping == false) return null;
             return new DocIdSetIterator() {
+                private int docID = -1;
+
                 @Override
                 public int nextDoc() throws IOException {
-                    return competitiveIterator.nextDoc();
+                    return advance(docID + 1);
                 }
 
                 @Override
                 public int docID() {
-                    return competitiveIterator.docID();
+                    return docID;
                 }
 
                 @Override
@@ -245,7 +247,7 @@ public abstract class NumericComparator<T extends Number> extends FieldComparato
 
                 @Override
                 public int advance(int target) throws IOException {
-                    return competitiveIterator.advance(target);
+                    return docID = competitiveIterator.advance(target);
                 }
             };
         }