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 2015/04/15 09:37:23 UTC
svn commit: r1673667 - in /lucene/dev/trunk/lucene: ./
core/src/java/org/apache/lucene/search/
core/src/test/org/apache/lucene/search/
spatial/src/java/org/apache/lucene/spatial/prefix/
test-framework/src/java/org/apache/lucene/search/
Author: jpountz
Date: Wed Apr 15 07:37:23 2015
New Revision: 1673667
URL: http://svn.apache.org/r1673667
Log:
LUCENE-6419: Added two-phase iteration assertions to AssertingQuery.
Modified:
lucene/dev/trunk/lucene/CHANGES.txt
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/Filter.java
lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java
lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestBooleanScorer.java
lucene/dev/trunk/lucene/spatial/src/java/org/apache/lucene/spatial/prefix/ContainsPrefixTreeFilter.java
lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingIndexSearcher.java
lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingLeafCollector.java
lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingScorer.java
Modified: lucene/dev/trunk/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/CHANGES.txt?rev=1673667&r1=1673666&r2=1673667&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/CHANGES.txt (original)
+++ lucene/dev/trunk/lucene/CHANGES.txt Wed Apr 15 07:37:23 2015
@@ -123,6 +123,11 @@ Build
* LUCENE-6420: Update forbiddenapis to v1.8 (Uwe Schindler)
+Test Framework
+
+* LUCENE-6419: Added two-phase iteration assertions to AssertingQuery.
+ (Adrien Grand)
+
======================= Lucene 5.1.0 =======================
New Features
Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/Filter.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/Filter.java?rev=1673667&r1=1673666&r2=1673667&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/Filter.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/Filter.java Wed Apr 15 07:37:23 2015
@@ -141,7 +141,6 @@ public abstract class Filter extends Que
public long cost() {
return iterator.cost();
}
-
};
}
Modified: lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java?rev=1673667&r1=1673666&r2=1673667&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java (original)
+++ lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java Wed Apr 15 07:37:23 2015
@@ -604,7 +604,9 @@ public class TestBooleanQuery extends Lu
w.commit();
DirectoryReader reader = w.getReader();
- final IndexSearcher searcher = newSearcher(reader);
+ // not LuceneTestCase.newSearcher to not have the asserting wrappers
+ // and do instanceof checks
+ final IndexSearcher searcher = new IndexSearcher(reader);
searcher.setQueryCache(null); // to still have approximations
PhraseQuery pq = new PhraseQuery();
@@ -666,7 +668,9 @@ public class TestBooleanQuery extends Lu
w.commit();
DirectoryReader reader = w.getReader();
- final IndexSearcher searcher = newSearcher(reader);
+ // not LuceneTestCase.newSearcher to not have the asserting wrappers
+ // and do instanceof checks
+ final IndexSearcher searcher = new IndexSearcher(reader);
searcher.setQueryCache(null); // to still have approximations
PhraseQuery pq = new PhraseQuery();
Modified: lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestBooleanScorer.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestBooleanScorer.java?rev=1673667&r1=1673666&r2=1673667&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestBooleanScorer.java (original)
+++ lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestBooleanScorer.java Wed Apr 15 07:37:23 2015
@@ -122,7 +122,7 @@ public class TestBooleanScorer extends L
IndexReader r = w.getReader();
w.close();
- IndexSearcher s = newSearcher(r);
+ IndexSearcher s = new IndexSearcher(r);
BooleanQuery q1 = new BooleanQuery();
q1.add(new TermQuery(new Term("field", "little")), BooleanClause.Occur.SHOULD);
q1.add(new TermQuery(new Term("field", "diseases")), BooleanClause.Occur.SHOULD);
Modified: lucene/dev/trunk/lucene/spatial/src/java/org/apache/lucene/spatial/prefix/ContainsPrefixTreeFilter.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/spatial/src/java/org/apache/lucene/spatial/prefix/ContainsPrefixTreeFilter.java?rev=1673667&r1=1673666&r2=1673667&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/spatial/src/java/org/apache/lucene/spatial/prefix/ContainsPrefixTreeFilter.java (original)
+++ lucene/dev/trunk/lucene/spatial/src/java/org/apache/lucene/spatial/prefix/ContainsPrefixTreeFilter.java Wed Apr 15 07:37:23 2015
@@ -316,10 +316,13 @@ public class ContainsPrefixTreeFilter ex
int idx = -1;
@Override
public int docID() {
- if (idx >= 0 && idx < size)
- return docs[idx];
- else
+ if (idx < 0) {
return -1;
+ } else if (idx < size) {
+ return docs[idx];
+ } else {
+ return NO_MORE_DOCS;
+ }
}
@Override
Modified: lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingIndexSearcher.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingIndexSearcher.java?rev=1673667&r1=1673666&r2=1673667&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingIndexSearcher.java (original)
+++ lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingIndexSearcher.java Wed Apr 15 07:37:23 2015
@@ -65,21 +65,6 @@ public class AssertingIndexSearcher exte
}
@Override
- public Scorer scorer(LeafReaderContext context, Bits acceptDocs) throws IOException {
- Scorer scorer = w.scorer(context, acceptDocs);
- if (scorer != null) {
- // check that scorer obeys disi contract for docID() before next()/advance
- try {
- int docid = scorer.docID();
- assert docid == -1 || docid == DocIdSetIterator.NO_MORE_DOCS;
- } catch (UnsupportedOperationException ignored) {
- // from a top-level BS1
- }
- }
- return scorer;
- }
-
- @Override
public float getValueForNormalization() {
throw new IllegalStateException("Weight already normalized.");
}
@@ -88,6 +73,12 @@ public class AssertingIndexSearcher exte
}
@Override
+ public Weight createWeight(Query query, boolean needsScores) throws IOException {
+ // this adds assertions to the inner weights/scorers too
+ return new AssertingWeight(random, super.createWeight(query, needsScores));
+ }
+
+ @Override
public Query rewrite(Query original) throws IOException {
// TODO: use the more sophisticated QueryUtils.check sometimes!
QueryUtils.check(original);
Modified: lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingLeafCollector.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingLeafCollector.java?rev=1673667&r1=1673666&r2=1673667&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingLeafCollector.java (original)
+++ lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingLeafCollector.java Wed Apr 15 07:37:23 2015
@@ -41,7 +41,7 @@ class AssertingLeafCollector extends Fil
@Override
public void setScorer(Scorer scorer) throws IOException {
this.scorer = scorer;
- super.setScorer(AssertingScorer.getAssertingScorer(random, scorer));
+ super.setScorer(AssertingScorer.wrap(random, scorer));
}
@Override
Modified: lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingScorer.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingScorer.java?rev=1673667&r1=1673666&r2=1673667&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingScorer.java (original)
+++ lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingScorer.java Wed Apr 15 07:37:23 2015
@@ -18,49 +18,20 @@ package org.apache.lucene.search;
*/
import java.io.IOException;
-import java.lang.ref.WeakReference;
import java.util.Collection;
import java.util.Collections;
-import java.util.Map;
import java.util.Random;
-import java.util.WeakHashMap;
/** Wraps a Scorer with additional checks */
public class AssertingScorer extends Scorer {
-
- // TODO: add asserts for two-phase intersection
- static enum IteratorState { START, ITERATING, FINISHED };
-
- // we need to track scorers using a weak hash map because otherwise we
- // could loose references because of eg.
- // AssertingScorer.score(Collector) which needs to delegate to work correctly
- private static Map<Scorer, WeakReference<AssertingScorer>> ASSERTING_INSTANCES = Collections.synchronizedMap(new WeakHashMap<Scorer, WeakReference<AssertingScorer>>());
+ static enum IteratorState { START, APPROXIMATING, ITERATING, FINISHED };
public static Scorer wrap(Random random, Scorer other) {
if (other == null || other instanceof AssertingScorer) {
return other;
}
- final AssertingScorer assertScorer = new AssertingScorer(random, other);
- ASSERTING_INSTANCES.put(other, new WeakReference<>(assertScorer));
- return assertScorer;
- }
-
- static Scorer getAssertingScorer(Random random, Scorer other) {
- if (other == null || other instanceof AssertingScorer) {
- return other;
- }
- final WeakReference<AssertingScorer> assertingScorerRef = ASSERTING_INSTANCES.get(other);
- final AssertingScorer assertingScorer = assertingScorerRef == null ? null : assertingScorerRef.get();
- if (assertingScorer == null) {
- // can happen in case of memory pressure or if
- // scorer1.score(collector) calls
- // collector.setScorer(scorer2) with scorer1 != scorer2, such as
- // BooleanScorer. In that case we can't enable all assertions
- return new AssertingScorer(random, other);
- } else {
- return assertingScorer;
- }
+ return new AssertingScorer(random, other);
}
final Random random;
@@ -80,12 +51,13 @@ public class AssertingScorer extends Sco
}
boolean iterating() {
+ // we cannot assert that state == ITERATING because of CachingScorerWrapper
switch (docID()) {
case -1:
case NO_MORE_DOCS:
return false;
default:
- return true;
+ return state != IteratorState.APPROXIMATING; // Matches must be confirmed before calling freq() or score()
}
}
@@ -114,6 +86,7 @@ public class AssertingScorer extends Sco
@Override
public int docID() {
+ assert state != IteratorState.APPROXIMATING : "calling docId() on the Scorer while the match has not been confirmed";
return in.docID();
}
@@ -155,5 +128,70 @@ public class AssertingScorer extends Sco
public String toString() {
return "AssertingScorer(" + in + ")";
}
+
+ @Override
+ public TwoPhaseIterator asTwoPhaseIterator() {
+ final TwoPhaseIterator in = this.in.asTwoPhaseIterator();
+ if (in == null) {
+ return null;
+ }
+ final DocIdSetIterator inApproximation = in.approximation();
+ assert inApproximation.docID() == doc;
+ final DocIdSetIterator assertingApproximation = new DocIdSetIterator() {
+
+ @Override
+ public int docID() {
+ return inApproximation.docID();
+ }
+
+ @Override
+ public int nextDoc() throws IOException {
+ assert state != IteratorState.FINISHED : "advance() called after NO_MORE_DOCS";
+ final int nextDoc = inApproximation.nextDoc();
+ assert nextDoc > doc : "backwards advance from: " + doc + " to: " + nextDoc;
+ if (nextDoc == NO_MORE_DOCS) {
+ state = IteratorState.FINISHED;
+ } else {
+ state = IteratorState.APPROXIMATING;
+ }
+ assert inApproximation.docID() == nextDoc;
+ return nextDoc;
+ }
+
+ @Override
+ public int advance(int target) throws IOException {
+ assert state != IteratorState.FINISHED : "advance() called after NO_MORE_DOCS";
+ assert target > doc : "target must be > docID(), got " + target + " <= " + doc;
+ final int advanced = inApproximation.advance(target);
+ assert advanced >= target : "backwards advance from: " + target + " to: " + advanced;
+ if (advanced == NO_MORE_DOCS) {
+ state = IteratorState.FINISHED;
+ } else {
+ state = IteratorState.APPROXIMATING;
+ }
+ assert inApproximation.docID() == advanced;
+ return advanced;
+ }
+
+ @Override
+ public long cost() {
+ return inApproximation.cost();
+ }
+
+ };
+ return new TwoPhaseIterator(assertingApproximation) {
+ @Override
+ public boolean matches() throws IOException {
+ assert state == IteratorState.APPROXIMATING;
+ final boolean matches = in.matches();
+ if (matches) {
+ assert AssertingScorer.this.in.docID() == inApproximation.docID() : "Approximation and scorer don't advance synchronously";
+ doc = inApproximation.docID();
+ state = IteratorState.ITERATING;
+ }
+ return matches;
+ }
+ };
+ }
}