ensure Scorer.skipTo() is interchangeable with next(): LUCENE-696

git-svn-id: https://svn.apache.org/repos/asf/lucene/java/trunk@467558 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -143,6 +143,9 @@ Bug fixes
 17. LUCENE-690: Fixed thread unsafe use of IndexInput by lazy loaded fields.
     (Yonik Seeley)
 
+18. LUCENE-696: Fix bug when scorer for DisjunctionMaxQuery has skipTo()
+    called on it before next().  (Yonik Seeley)
+
 Optimizations
 
   1. LUCENE-586: TermDocs.skipTo() is now more efficient for multi-segment

src/java/org/apache/lucene/search/DisjunctionMaxScorer.java

@@ -113,6 +113,13 @@ class DisjunctionMaxScorer extends Scorer {
      * @return true iff there is a document to be generated whose number is at least target
      */
     public boolean skipTo(int target) throws IOException {
+        if (firstTime) {
+          if (!more) return false;
+          heapify();
+          firstTime = false;
+          return true;   // more would have been false if no subScorers had any docs
+        }
+
         while (subScorers.size()>0 && ((Scorer)subScorers.get(0)).doc()<target) {
             if (((Scorer)subScorers.get(0)).skipTo(target))
                 heapAdjust(0);

src/test/org/apache/lucene/search/CheckHits.java

@@ -87,6 +87,8 @@ public class CheckHits {
         }
       });
     TestCase.assertEquals(query.toString(defaultFieldName), correct, actual);
+
+    QueryUtils.check(query,searcher);
   }
   
   /**
@@ -109,6 +111,10 @@ public class CheckHits {
         Searcher searcher,
         int[] results)
           throws IOException {
+    if (searcher instanceof IndexSearcher) {
+      QueryUtils.check(query,(IndexSearcher)searcher);
+    }
+
     Hits hits = searcher.search(query);
 
     Set correct = new TreeSet();
@@ -122,6 +128,8 @@ public class CheckHits {
     }
 
     TestCase.assertEquals(query.toString(defaultFieldName), correct, actual);
+
+    QueryUtils.check(query,searcher);
   }
 
   /** Tests that a Hits has an expected order of documents */

src/test/org/apache/lucene/search/QueryUtils.java

@@ -2,6 +2,8 @@ package org.apache.lucene.search;
 
 import junit.framework.TestCase;
 
+import java.io.IOException;
+
 /**
  * Copyright 2005 Apache Software Foundation
  *
@@ -63,4 +65,60 @@ public class QueryUtils {
     TestCase.assertTrue(q1.hashCode() != q2.hashCode());
   }
 
+
+  /** various query sanity checks on a searcher */
+  public static void check(Query q1, Searcher s) {
+    try {
+      check(q1);
+      if (s!=null && s instanceof IndexSearcher) {
+        IndexSearcher is = (IndexSearcher)s;
+        checkSkipTo(q1,is);
+      }
+    } catch (IOException e) {
+      throw new RuntimeException(e);
+    }
+  }
+
+  /** alternate scorer skipTo(),skipTo(),next(),next(),skipTo(),skipTo(), etc
+   * and ensure a hitcollector receives same docs and scores
+   */
+  public static void checkSkipTo(final Query q, final IndexSearcher s) throws IOException {
+    //System.out.println("Checking "+q);
+    final Weight w = q.weight(s);
+    final Scorer scorer = w.scorer(s.getIndexReader());
+
+    // FUTURE: ensure scorer.doc()==-1
+    
+    if (BooleanQuery.getUseScorer14()) return;  // 1.4 doesn't support skipTo
+
+    final int[] which = new int[1];
+    final int[] sdoc = new int[] {-1};
+    final float maxDiff = 1e-5f;
+    s.search(q,new HitCollector() {
+      public void collect(int doc, float score) {
+        try {
+          boolean more = (which[0]++&0x02)==0 ? scorer.skipTo(sdoc[0]+1) : scorer.next();
+          sdoc[0] = scorer.doc();
+          float scorerScore = scorer.score();
+          float scoreDiff = Math.abs(score-scorerScore);
+          scoreDiff=0; // TODO: remove this go get LUCENE-697 failures 
+          if (more==false || doc != sdoc[0] || scoreDiff>maxDiff) {
+            throw new RuntimeException("ERROR matching docs:"
+                    +"\n\tscorer.more=" + more + " doc="+sdoc[0] + " score="+scorerScore
+                    +"\n\thitCollector.doc=" + doc + " score="+score
+                    +"\n\t Scorer=" + scorer
+                    +"\n\t Query=" + q
+                    +"\n\t Searcher=" + s
+            );
+          }
+        } catch (IOException e) {
+          throw new RuntimeException(e);
+        }
+      }
+    });
+
+    // make sure next call to scorer is false.
+    TestCase.assertFalse((which[0]++&0x02)==0 ? scorer.skipTo(sdoc[0]+1) : scorer.next());
+  }
+
 }

src/test/org/apache/lucene/search/TestBoolean2.java

@@ -62,7 +62,8 @@ public class TestBoolean2 extends TestCase {
   };
 
   public Query makeQuery(String queryText) throws ParseException {
-    return (new QueryParser(field, new WhitespaceAnalyzer())).parse(queryText);
+    Query q = (new QueryParser(field, new WhitespaceAnalyzer())).parse(queryText);
+    return q;
   }
 
   public void queriesTest(String queryText, int[] expDocNrs) throws Exception {
@@ -167,6 +168,9 @@ public class TestBoolean2 extends TestCase {
         Sort sort = Sort.INDEXORDER;
 
         BooleanQuery.setUseScorer14(false);
+
+        QueryUtils.check(q1,searcher);
+
         Hits hits1 = searcher.search(q1,sort);
         if (hits1.length()>0) hits1.id(hits1.length()-1);
 

src/test/org/apache/lucene/search/TestBooleanMinShouldMatch.java

@@ -83,6 +83,7 @@ public class TestBooleanMinShouldMatch extends TestCase {
             printHits(getName(), h);
         }
         assertEquals("result count", expected, h.length());
+        QueryUtils.check(q,s);
     }
 
     public void testAllOptional() throws Exception {
@@ -316,6 +317,9 @@ public class TestBooleanMinShouldMatch extends TestCase {
         TopDocs top1 = s.search(q1,null,100);
         TopDocs top2 = s.search(q2,null,100);
 
+        QueryUtils.check(q1,s);
+        QueryUtils.check(q2,s);
+
         // The constrained query
         // should be a superset to the unconstrained query.
         if (top2.totalHits > top1.totalHits) {

src/test/org/apache/lucene/search/TestBooleanOr.java

@@ -48,6 +48,7 @@ public class TestBooleanOr extends TestCase {
 	private IndexSearcher searcher = null;
 
 	private int search(Query q) throws IOException {
+    QueryUtils.check(q,searcher);
     return searcher.search(q).length();
 	}
 

src/test/org/apache/lucene/search/TestDisjunctionMaxQuery.java

@@ -130,6 +130,7 @@ public class TestDisjunctionMaxQuery extends TestCase{
         DisjunctionMaxQuery q = new DisjunctionMaxQuery(0.0f);
         q.add(tq("hed","albino"));
         q.add(tq("hed","elephant"));
+        QueryUtils.check(q,s);
 
         Hits h = s.search(q);
 
@@ -155,6 +156,8 @@ public class TestDisjunctionMaxQuery extends TestCase{
         DisjunctionMaxQuery q = new DisjunctionMaxQuery(0.0f);
         q.add(tq("dek","albino"));
         q.add(tq("dek","elephant"));
+        QueryUtils.check(q,s);
+
 
         Hits h = s.search(q);
 
@@ -180,6 +183,8 @@ public class TestDisjunctionMaxQuery extends TestCase{
         q.add(tq("hed","elephant"));
         q.add(tq("dek","albino"));
         q.add(tq("dek","elephant"));
+        QueryUtils.check(q,s);
+
 
         Hits h = s.search(q);
 
@@ -203,6 +208,8 @@ public class TestDisjunctionMaxQuery extends TestCase{
         DisjunctionMaxQuery q = new DisjunctionMaxQuery(0.01f);
         q.add(tq("dek","albino"));
         q.add(tq("dek","elephant"));
+        QueryUtils.check(q,s);
+
 
         Hits h = s.search(q);
 
@@ -232,14 +239,18 @@ public class TestDisjunctionMaxQuery extends TestCase{
             q1.add(tq("hed","albino"));
             q1.add(tq("dek","albino"));
             q.add(q1,BooleanClause.Occur.MUST);//true,false);
+            QueryUtils.check(q1,s);
+
         }
         {
             DisjunctionMaxQuery q2 = new DisjunctionMaxQuery(0.0f);
             q2.add(tq("hed","elephant"));
             q2.add(tq("dek","elephant"));
             q.add(q2, BooleanClause.Occur.MUST);//true,false);
+           QueryUtils.check(q2,s);
         }
 
+        QueryUtils.check(q,s);
 
         Hits h = s.search(q);
 
@@ -273,6 +284,7 @@ public class TestDisjunctionMaxQuery extends TestCase{
             q2.add(tq("dek","elephant"));
             q.add(q2, BooleanClause.Occur.SHOULD);//false,false);
         }
+        QueryUtils.check(q,s);
 
 
         Hits h = s.search(q);
@@ -312,6 +324,7 @@ public class TestDisjunctionMaxQuery extends TestCase{
             q2.add(tq("dek","elephant"));
             q.add(q2, BooleanClause.Occur.SHOULD);//false,false);
         }
+        QueryUtils.check(q,s);
 
 
         Hits h = s.search(q);
@@ -370,6 +383,7 @@ public class TestDisjunctionMaxQuery extends TestCase{
             q2.add(tq("dek","elephant"));
             q.add(q2, BooleanClause.Occur.SHOULD);//false,false);
         }
+        QueryUtils.check(q,s);
 
 
         Hits h = s.search(q);

src/test/org/apache/lucene/search/TestFilteredQuery.java

@@ -95,6 +95,7 @@ extends TestCase {
     Hits hits = searcher.search (filteredquery);
     assertEquals (1, hits.length());
     assertEquals (1, hits.id(0));
+    QueryUtils.check(filteredquery,searcher);
 
     hits = searcher.search (filteredquery, new Sort("sorter"));
     assertEquals (1, hits.length());
@@ -103,15 +104,18 @@ extends TestCase {
     filteredquery = new FilteredQuery (new TermQuery (new Term ("field", "one")), filter);
     hits = searcher.search (filteredquery);
     assertEquals (2, hits.length());
+    QueryUtils.check(filteredquery,searcher);
 
     filteredquery = new FilteredQuery (new TermQuery (new Term ("field", "x")), filter);
     hits = searcher.search (filteredquery);
     assertEquals (1, hits.length());
     assertEquals (3, hits.id(0));
+    QueryUtils.check(filteredquery,searcher);
 
     filteredquery = new FilteredQuery (new TermQuery (new Term ("field", "y")), filter);
     hits = searcher.search (filteredquery);
     assertEquals (0, hits.length());
+    QueryUtils.check(filteredquery,searcher);    
   }
 
   /**
@@ -124,6 +128,7 @@ extends TestCase {
     Query filteredquery = new FilteredQuery(rq, filter);
     Hits hits = searcher.search(filteredquery);
     assertEquals(2, hits.length());
+    QueryUtils.check(filteredquery,searcher);
   }
 
   public void testBoolean() throws Exception {
@@ -136,6 +141,7 @@ extends TestCase {
     bq.add(query, BooleanClause.Occur.MUST);
     Hits hits = searcher.search(bq);
     assertEquals(0, hits.length());
+    QueryUtils.check(query,searcher);    
   }
 }
 

src/test/org/apache/lucene/search/TestPhraseQuery.java

@@ -76,6 +76,7 @@ public class TestPhraseQuery extends TestCase {
     query.add(new Term("field", "five"));
     Hits hits = searcher.search(query);
     assertEquals(0, hits.length());
+    QueryUtils.check(query,searcher);
   }
 
   public void testBarelyCloseEnough() throws Exception {
@@ -84,6 +85,7 @@ public class TestPhraseQuery extends TestCase {
     query.add(new Term("field", "five"));
     Hits hits = searcher.search(query);
     assertEquals(1, hits.length());
+    QueryUtils.check(query,searcher);
   }
 
   /**
@@ -95,12 +97,15 @@ public class TestPhraseQuery extends TestCase {
     query.add(new Term("field", "five"));
     Hits hits = searcher.search(query);
     assertEquals("exact match", 1, hits.length());
+    QueryUtils.check(query,searcher);
+
 
     query = new PhraseQuery();
     query.add(new Term("field", "two"));
     query.add(new Term("field", "one"));
     hits = searcher.search(query);
     assertEquals("reverse not exact", 0, hits.length());
+    QueryUtils.check(query,searcher);
   }
 
   public void testSlop1() throws Exception {
@@ -110,6 +115,8 @@ public class TestPhraseQuery extends TestCase {
     query.add(new Term("field", "two"));
     Hits hits = searcher.search(query);
     assertEquals("in order", 1, hits.length());
+    QueryUtils.check(query,searcher);
+
 
     // Ensures slop of 1 does not work for phrases out of order;
     // must be at least 2.
@@ -119,6 +126,7 @@ public class TestPhraseQuery extends TestCase {
     query.add(new Term("field", "one"));
     hits = searcher.search(query);
     assertEquals("reversed, slop not 2 or more", 0, hits.length());
+    QueryUtils.check(query,searcher);
   }
 
   /**
@@ -130,6 +138,8 @@ public class TestPhraseQuery extends TestCase {
     query.add(new Term("field", "one"));
     Hits hits = searcher.search(query);
     assertEquals("just sloppy enough", 1, hits.length());
+    QueryUtils.check(query,searcher);
+
 
     query = new PhraseQuery();
     query.setSlop(2);
@@ -137,6 +147,8 @@ public class TestPhraseQuery extends TestCase {
     query.add(new Term("field", "one"));
     hits = searcher.search(query);
     assertEquals("not sloppy enough", 0, hits.length());
+    QueryUtils.check(query,searcher);
+
   }
 
   /**
@@ -150,6 +162,8 @@ public class TestPhraseQuery extends TestCase {
     query.add(new Term("field", "five"));
     Hits hits = searcher.search(query);
     assertEquals("two total moves", 1, hits.length());
+    QueryUtils.check(query,searcher);
+
 
     query = new PhraseQuery();
     query.setSlop(5); // it takes six moves to match this phrase
@@ -158,10 +172,14 @@ public class TestPhraseQuery extends TestCase {
     query.add(new Term("field", "one"));
     hits = searcher.search(query);
     assertEquals("slop of 5 not close enough", 0, hits.length());
+    QueryUtils.check(query,searcher);
+
 
     query.setSlop(6);
     hits = searcher.search(query);
     assertEquals("slop of 6 just right", 1, hits.length());
+    QueryUtils.check(query,searcher);
+
   }
   
   public void testPhraseQueryWithStopAnalyzer() throws Exception {
@@ -181,6 +199,8 @@ public class TestPhraseQuery extends TestCase {
     query.add(new Term("field","words"));
     Hits hits = searcher.search(query);
     assertEquals(1, hits.length());
+    QueryUtils.check(query,searcher);
+
 
     // currently StopAnalyzer does not leave "holes", so this matches.
     query = new PhraseQuery();
@@ -188,6 +208,8 @@ public class TestPhraseQuery extends TestCase {
     query.add(new Term("field", "here"));
     hits = searcher.search(query);
     assertEquals(1, hits.length());
+    QueryUtils.check(query,searcher);
+
 
     searcher.close();
   }
@@ -215,6 +237,8 @@ public class TestPhraseQuery extends TestCase {
     phraseQuery.add(new Term("source", "info"));
     Hits hits = searcher.search(phraseQuery);
     assertEquals(2, hits.length());
+    QueryUtils.check(phraseQuery,searcher);
+
     
     TermQuery termQuery = new TermQuery(new Term("contents","foobar"));
     BooleanQuery booleanQuery = new BooleanQuery();
@@ -222,6 +246,8 @@ public class TestPhraseQuery extends TestCase {
     booleanQuery.add(phraseQuery, BooleanClause.Occur.MUST);
     hits = searcher.search(booleanQuery);
     assertEquals(1, hits.length());
+    QueryUtils.check(termQuery,searcher);
+
     
     searcher.close();
     
@@ -253,6 +279,7 @@ public class TestPhraseQuery extends TestCase {
     hits = searcher.search(phraseQuery);
     assertEquals(2, hits.length());
 
+    
     booleanQuery = new BooleanQuery();
     booleanQuery.add(termQuery, BooleanClause.Occur.MUST);
     booleanQuery.add(phraseQuery, BooleanClause.Occur.MUST);
@@ -264,6 +291,8 @@ public class TestPhraseQuery extends TestCase {
     booleanQuery.add(termQuery, BooleanClause.Occur.MUST);
     hits = searcher.search(booleanQuery);
     assertEquals(2, hits.length());
+    QueryUtils.check(booleanQuery,searcher);
+
     
     searcher.close();
     directory.close();
@@ -303,6 +332,7 @@ public class TestPhraseQuery extends TestCase {
     assertEquals(1, hits.id(1));
     assertEquals(0.31, hits.score(2), 0.01);
     assertEquals(2, hits.id(2));
+    QueryUtils.check(query,searcher);        
   }
 
   public void testWrappedPhrase() throws IOException {
@@ -314,6 +344,8 @@ public class TestPhraseQuery extends TestCase {
 
     Hits hits = searcher.search(query);
     assertEquals(0, hits.length());
+    QueryUtils.check(query,searcher);
+
   }
 
 }

src/test/org/apache/lucene/search/spans/TestBasics.java

@@ -342,6 +342,5 @@ public class TestBasics extends TestCase {
 
   private void checkHits(Query query, int[] results) throws IOException {
     CheckHits.checkHits(query, "field", searcher, results);
-    QueryUtils.check(query);
   }
 }

src/test/org/apache/lucene/search/spans/TestSpansAdvanced.java

@@ -124,6 +124,8 @@ public class TestSpansAdvanced extends TestCase {
      */
     protected static void assertHits(Searcher s, Query query, final String description, final String[] expectedIds,
             final float[] expectedScores) throws IOException {
+        QueryUtils.check(query,s);
+
         final float tolerance = 1e-5f;
 
         // Hits hits = searcher.search(query);