lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jpou...@apache.org
Subject svn commit: r1463906 - in /lucene/dev/trunk/lucene: ./ core/src/java/org/apache/lucene/index/ core/src/java/org/apache/lucene/search/ core/src/java/org/apache/lucene/search/spans/ core/src/test/org/apache/lucene/index/ core/src/test/org/apache/lucene/s...
Date Wed, 03 Apr 2013 10:15:26 GMT
Author: jpountz
Date: Wed Apr  3 10:15:25 2013
New Revision: 1463906

URL: http://svn.apache.org/r1463906
Log:
LUCENE-4888: Never call DocIdSetIterator.advance with target <= docID().

Modified:
    lucene/dev/trunk/lucene/CHANGES.txt
    lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/MultiDocsAndPositionsEnum.java
    lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/MultiDocsEnum.java
    lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SloppyPhraseScorer.java
    lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/spans/Spans.java
    lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/spans/TermSpans.java
    lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestSegmentTermDocs.java
    lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/spans/MultiSpansWrapper.java
    lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/spans/TestBasics.java
    lucene/dev/trunk/lucene/misc/src/test/org/apache/lucene/index/sorter/SorterTestBase.java
    lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/index/AssertingAtomicReader.java

Modified: lucene/dev/trunk/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/CHANGES.txt?rev=1463906&r1=1463905&r2=1463906&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/CHANGES.txt (original)
+++ lucene/dev/trunk/lucene/CHANGES.txt Wed Apr  3 10:15:25 2013
@@ -197,6 +197,11 @@ Bug Fixes
 * LUCENE-4893: Facet counts were multiplied as many times as 
   FacetsCollector.getFacetResults() is called. (Shai Erera)
 
+* LUCENE-4888: Fixed SloppyPhraseScorer, MultiDocs(AndPositions)Enum and
+  MultiSpansWrapper which happened to sometimes call DocIdSetIterator.advance
+  with target<=current (in this case the behavior of advance is undefined).
+  (Adrien Grand)
+
 Documentation
 
 * LUCENE-4841: Added example SimpleSortedSetFacetsExample to show how

Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/MultiDocsAndPositionsEnum.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/MultiDocsAndPositionsEnum.java?rev=1463906&r1=1463905&r2=1463906&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/MultiDocsAndPositionsEnum.java
(original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/MultiDocsAndPositionsEnum.java
Wed Apr  3 10:15:25 2013
@@ -89,9 +89,16 @@ public final class MultiDocsAndPositions
 
   @Override
   public int advance(int target) throws IOException {
+    assert target > doc;
     while(true) {
       if (current != null) {
-        final int doc = current.advance(target-currentBase);
+        final int doc;
+        if (target < currentBase) {
+          // target was in the previous slice but there was no matching doc after it
+          doc = current.nextDoc();
+        } else {
+          doc = current.advance(target-currentBase);
+        }
         if (doc == NO_MORE_DOCS) {
           current = null;
         } else {

Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/MultiDocsEnum.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/MultiDocsEnum.java?rev=1463906&r1=1463905&r2=1463906&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/MultiDocsEnum.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/MultiDocsEnum.java Wed Apr
 3 10:15:25 2013
@@ -90,9 +90,16 @@ public final class MultiDocsEnum extends
 
   @Override
   public int advance(int target) throws IOException {
+    assert target > doc;
     while(true) {
       if (current != null) {
-        final int doc = current.advance(target-currentBase);
+        final int doc;
+        if (target < currentBase) {
+          // target was in the previous slice but there was no matching doc after it
+          doc = current.nextDoc();
+        } else {
+          doc = current.advance(target-currentBase);
+        }
         if (doc == NO_MORE_DOCS) {
           current = null;
         } else {

Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SloppyPhraseScorer.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SloppyPhraseScorer.java?rev=1463906&r1=1463905&r2=1463906&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SloppyPhraseScorer.java
(original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SloppyPhraseScorer.java
Wed Apr  3 10:15:25 2013
@@ -561,7 +561,7 @@ final class SloppyPhraseScorer extends S
 
   @Override
   public int nextDoc() throws IOException {
-    return advance(max.doc);
+    return advance(max.doc + 1); // advance to the next doc after #docID()
   }
   
   @Override
@@ -571,27 +571,25 @@ final class SloppyPhraseScorer extends S
 
   @Override
   public int advance(int target) throws IOException {
-    sloppyFreq = 0.0f;
-    if (!advanceMin(target)) {
-      return NO_MORE_DOCS;
-    }        
-    boolean restart=false;
-    while (sloppyFreq == 0.0f) {
-      while (min.doc < max.doc || restart) {
-        restart = false;
+    assert target > docID();
+    do {
+      if (!advanceMin(target)) {
+        return NO_MORE_DOCS;
+      }
+      while (min.doc < max.doc) {
         if (!advanceMin(max.doc)) {
           return NO_MORE_DOCS;
-        }        
+        }
       }
       // found a doc with all of the terms
       sloppyFreq = phraseFreq(); // check for phrase
-      restart = true;
-    } 
+      target = min.doc + 1; // next target in case sloppyFreq is still 0
+    } while (sloppyFreq == 0f);
 
     // found a match
     return max.doc;
   }
-  
+
   @Override
   public long cost() {
     return cost;

Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/spans/Spans.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/spans/Spans.java?rev=1463906&r1=1463905&r2=1463906&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/spans/Spans.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/spans/Spans.java Wed Apr
 3 10:15:25 2013
@@ -29,7 +29,11 @@ public abstract class Spans {
   public abstract boolean next() throws IOException;
 
   /** Skips to the first match beyond the current, whose document number is
-   * greater than or equal to <i>target</i>. <p>Returns true iff there
is such
+   * greater than or equal to <i>target</i>.
+   * <p>The behavior of this method is <b>undefined</b> when called with
+   * <code> target &le; current</code>, or after the iterator has exhausted.
+   * Both cases may result in unpredicted behavior.
+   * <p>Returns true iff there is such
    * a match.  <p>Behaves as if written: <pre class="prettyprint">
    *   boolean skipTo(int target) {
    *     do {

Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/spans/TermSpans.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/spans/TermSpans.java?rev=1463906&r1=1463905&r2=1463906&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/spans/TermSpans.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/spans/TermSpans.java Wed
Apr  3 10:15:25 2013
@@ -71,6 +71,7 @@ public class TermSpans extends Spans {
 
   @Override
   public boolean skipTo(int target) throws IOException {
+    assert target > doc;
     doc = postings.advance(target);
     if (doc == DocIdSetIterator.NO_MORE_DOCS) {
       return false;

Modified: lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestSegmentTermDocs.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestSegmentTermDocs.java?rev=1463906&r1=1463905&r2=1463906&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestSegmentTermDocs.java
(original)
+++ lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestSegmentTermDocs.java
Wed Apr  3 10:15:25 2013
@@ -149,7 +149,7 @@ public class TestSegmentTermDocs extends
     assertTrue(tdocs.nextDoc() != DocIdSetIterator.NO_MORE_DOCS);
     assertEquals(1, tdocs.docID());
     assertEquals(4, tdocs.freq());
-    assertTrue(tdocs.advance(0) != DocIdSetIterator.NO_MORE_DOCS);
+    assertTrue(tdocs.advance(2) != DocIdSetIterator.NO_MORE_DOCS);
     assertEquals(2, tdocs.docID());
     assertTrue(tdocs.advance(4) != DocIdSetIterator.NO_MORE_DOCS);
     assertEquals(4, tdocs.docID());
@@ -189,7 +189,7 @@ public class TestSegmentTermDocs extends
     assertTrue(tdocs.nextDoc() != DocIdSetIterator.NO_MORE_DOCS);
     assertEquals(11, tdocs.docID());
     assertEquals(4, tdocs.freq());
-    assertTrue(tdocs.advance(5) != DocIdSetIterator.NO_MORE_DOCS);
+    assertTrue(tdocs.advance(12) != DocIdSetIterator.NO_MORE_DOCS);
     assertEquals(12, tdocs.docID());
     assertTrue(tdocs.advance(15) != DocIdSetIterator.NO_MORE_DOCS);
     assertEquals(15, tdocs.docID());
@@ -233,7 +233,7 @@ public class TestSegmentTermDocs extends
     assertTrue(tdocs.nextDoc() != DocIdSetIterator.NO_MORE_DOCS);
     assertEquals(27, tdocs.docID());
     assertEquals(4, tdocs.freq());
-    assertTrue(tdocs.advance(5) != DocIdSetIterator.NO_MORE_DOCS);
+    assertTrue(tdocs.advance(28) != DocIdSetIterator.NO_MORE_DOCS);
     assertEquals(28, tdocs.docID());
     assertTrue(tdocs.advance(40) != DocIdSetIterator.NO_MORE_DOCS);
     assertEquals(40, tdocs.docID());

Modified: lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/spans/MultiSpansWrapper.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/spans/MultiSpansWrapper.java?rev=1463906&r1=1463905&r2=1463906&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/spans/MultiSpansWrapper.java
(original)
+++ lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/spans/MultiSpansWrapper.java
Wed Apr  3 10:15:25 2013
@@ -53,7 +53,6 @@ public class MultiSpansWrapper extends S
     this.leaves = leaves;
     this.numLeaves = leaves.size();
     this.termContexts = termContexts;
-
   }
   
   public static Spans wrap(IndexReaderContext topLevelReaderContext, SpanQuery query) throws
IOException {
@@ -112,7 +111,12 @@ public class MultiSpansWrapper extends S
       current = query.getSpans(ctx, ctx.reader().getLiveDocs(), termContexts);
     }
     while (true) {
-      if (current.skipTo(target - leaves.get(leafOrd).docBase)) {
+      if (target < leaves.get(leafOrd).docBase) {
+        // target was in the previous slice
+        if (current.next()) {
+          return true;
+        }
+      } else if (current.skipTo(target - leaves.get(leafOrd).docBase)) {
         return true;
       }
       if (++leafOrd < numLeaves) {

Modified: lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/spans/TestBasics.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/spans/TestBasics.java?rev=1463906&r1=1463905&r2=1463906&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/spans/TestBasics.java (original)
+++ lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/spans/TestBasics.java Wed
Apr  3 10:15:25 2013
@@ -593,8 +593,8 @@ public class TestBasics extends LuceneTe
     boolean hasMore = true;
 
     do {
-      hasMore = skipToAccoringToJavaDocs(s1, s1.doc());
-      assertEquals(hasMore, s2.skipTo(s2.doc()));
+      hasMore = skipToAccoringToJavaDocs(s1, s1.doc() + 1);
+      assertEquals(hasMore, s2.skipTo(s2.doc() + 1));
       assertEquals(s1.doc(), s2.doc());
     } while (hasMore);
   }

Modified: lucene/dev/trunk/lucene/misc/src/test/org/apache/lucene/index/sorter/SorterTestBase.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/misc/src/test/org/apache/lucene/index/sorter/SorterTestBase.java?rev=1463906&r1=1463905&r2=1463906&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/misc/src/test/org/apache/lucene/index/sorter/SorterTestBase.java
(original)
+++ lucene/dev/trunk/lucene/misc/src/test/org/apache/lucene/index/sorter/SorterTestBase.java
Wed Apr  3 10:15:25 2013
@@ -283,7 +283,7 @@ public abstract class SorterTestBase ext
       assertTrue(((SortingDocsAndPositionsEnum) sortedPositions).reused(reuse)); // make
sure reuse worked
     }
     doc = 0;
-    while ((doc = sortedPositions.advance(doc)) != DocIdSetIterator.NO_MORE_DOCS) {
+    while ((doc = sortedPositions.advance(doc + _TestUtil.nextInt(random(), 1, 5))) != DocIdSetIterator.NO_MORE_DOCS)
{
       int freq = sortedPositions.freq();
       assertEquals("incorrect freq for doc=" + doc, sortedValues[doc].intValue() / 10 + 1,
freq);
       for (int i = 0; i < freq; i++) {

Modified: lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/index/AssertingAtomicReader.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/index/AssertingAtomicReader.java?rev=1463906&r1=1463905&r2=1463906&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/index/AssertingAtomicReader.java
(original)
+++ lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/index/AssertingAtomicReader.java
Wed Apr  3 10:15:25 2013
@@ -225,38 +225,42 @@ public class AssertingAtomicReader exten
   static enum DocsEnumState { START, ITERATING, FINISHED };
   static class AssertingDocsEnum extends FilterDocsEnum {
     private DocsEnumState state = DocsEnumState.START;
+    private int doc;
     
     public AssertingDocsEnum(DocsEnum in) {
       super(in);
       int docid = in.docID();
       assert docid == -1 || docid == DocIdSetIterator.NO_MORE_DOCS : "invalid initial doc
id: " + docid;
+      doc = -1;
     }
 
     @Override
     public int nextDoc() throws IOException {
       assert state != DocsEnumState.FINISHED : "nextDoc() called after NO_MORE_DOCS";
       int nextDoc = super.nextDoc();
-      assert nextDoc >= 0 : "invalid doc id: " + nextDoc;
+      assert nextDoc > doc : "backwards nextDoc from " + doc + " to " + nextDoc;
       if (nextDoc == DocIdSetIterator.NO_MORE_DOCS) {
         state = DocsEnumState.FINISHED;
       } else {
         state = DocsEnumState.ITERATING;
       }
-      return nextDoc;
+      assert docID() == nextDoc;
+      return doc = nextDoc;
     }
 
     @Override
     public int advance(int target) throws IOException {
       assert state != DocsEnumState.FINISHED : "advance() called after NO_MORE_DOCS";
+      assert target > doc : "target must be > docID(), got " + target + " <= " +
doc;
       int advanced = super.advance(target);
-      assert advanced >= 0 : "invalid doc id: " + advanced;
       assert advanced >= target : "backwards advance from: " + target + " to: " + advanced;
       if (advanced == DocIdSetIterator.NO_MORE_DOCS) {
         state = DocsEnumState.FINISHED;
       } else {
         state = DocsEnumState.ITERATING;
       }
-      return advanced;
+      assert docID() == advanced;
+      return doc = advanced;
     }
 
     // NOTE: We don't assert anything for docId(). Specifically DocsEnum javadocs
@@ -277,18 +281,20 @@ public class AssertingAtomicReader exten
     private DocsEnumState state = DocsEnumState.START;
     private int positionMax = 0;
     private int positionCount = 0;
+    private int doc;
 
     public AssertingDocsAndPositionsEnum(DocsAndPositionsEnum in) {
       super(in);
       int docid = in.docID();
       assert docid == -1 || docid == DocIdSetIterator.NO_MORE_DOCS : "invalid initial doc
id: " + docid;
+      doc = -1;
     }
 
     @Override
     public int nextDoc() throws IOException {
       assert state != DocsEnumState.FINISHED : "nextDoc() called after NO_MORE_DOCS";
       int nextDoc = super.nextDoc();
-      assert nextDoc >= 0 : "invalid doc id: " + nextDoc;
+      assert nextDoc > doc : "backwards nextDoc from " + doc + " to " + nextDoc;
       positionCount = 0;
       if (nextDoc == DocIdSetIterator.NO_MORE_DOCS) {
         state = DocsEnumState.FINISHED;
@@ -297,14 +303,15 @@ public class AssertingAtomicReader exten
         state = DocsEnumState.ITERATING;
         positionMax = super.freq();
       }
-      return nextDoc;
+      assert docID() == nextDoc;
+      return doc = nextDoc;
     }
 
     @Override
     public int advance(int target) throws IOException {
       assert state != DocsEnumState.FINISHED : "advance() called after NO_MORE_DOCS";
+      assert target > doc : "target must be > docID(), got " + target + " <= " +
doc;
       int advanced = super.advance(target);
-      assert advanced >= 0 : "invalid doc id: " + advanced;
       assert advanced >= target : "backwards advance from: " + target + " to: " + advanced;
       positionCount = 0;
       if (advanced == DocIdSetIterator.NO_MORE_DOCS) {
@@ -314,7 +321,8 @@ public class AssertingAtomicReader exten
         state = DocsEnumState.ITERATING;
         positionMax = super.freq();
       }
-      return advanced;
+      assert docID() == advanced;
+      return doc = advanced;
     }
 
     @Override



Mime
View raw message