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 2013/04/03 12:15:26 UTC
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...
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 ≤ 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