From ee0a91c054d9ec4a77de394463e39470a71351a3 Mon Sep 17 00:00:00 2001
From: Shai Erera <shaie@apache.org>
Date: Tue, 23 Jul 2013 06:25:37 +0000
Subject: [PATCH] LUCENE-5128: IndexSearcher.searchAfter should throw
 IllegalArgumentException if after.doc >= reader.maxDoc()

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1505909 13f79535-47bb-0310-9956-ffa450edef68
---
 lucene/CHANGES.txt                            |  4 +++
 .../apache/lucene/search/IndexSearcher.java   |  9 +++--
 .../lucene/search/TestIndexSearcher.java      | 23 +++++++++++++
 .../lucene/search/ShardSearchingTestBase.java | 33 ++++++++++++++-----
 4 files changed, 57 insertions(+), 12 deletions(-)

diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index b147e00533b..21621e5ef57 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -80,6 +80,10 @@ API Changes
 * LUCENE-5114: Remove unused boolean useCache parameter from
   TermsEnum.seekCeil and .seekExact (Mike McCandless)
 
+* LUCENE-5128: IndexSearcher.searchAfter throws IllegalArgumentException if 
+  searchAfter exceeds the number of documents in the reader. 
+  (Crocket via Shai Erera)
+
 Optimizations
 
 * LUCENE-5088: Added TermFilter to filter docs by a specific term.
diff --git a/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java b/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java
index b730e8fa76c..4a2fa0a39e6 100644
--- a/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java
+++ b/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java
@@ -430,6 +430,10 @@ public class IndexSearcher {
     if (limit == 0) {
       limit = 1;
     }
+    if (after != null && after.doc >= limit) {
+      throw new IllegalArgumentException("after.doc exceeds the number of documents in that reader: after.doc="
+          + after.doc + " limit=" + limit);
+    }
     nDocs = Math.min(nDocs, limit);
     
     if (executor == null) {
@@ -440,8 +444,7 @@ public class IndexSearcher {
       final ExecutionHelper<TopDocs> runner = new ExecutionHelper<TopDocs>(executor);
     
       for (int i = 0; i < leafSlices.length; i++) { // search each sub
-        runner.submit(
-                      new SearcherCallableNoSort(lock, this, leafSlices[i], weight, after, nDocs, hq));
+        runner.submit(new SearcherCallableNoSort(lock, this, leafSlices[i], weight, after, nDocs, hq));
       }
 
       int totalHits = 0;
@@ -920,7 +923,7 @@ public class IndexSearcher {
    */
   public TermStatistics termStatistics(Term term, TermContext context) throws IOException {
     return new TermStatistics(term.bytes(), context.docFreq(), context.totalTermFreq());
-  };
+  }
   
   /**
    * Returns {@link CollectionStatistics} for a field.
diff --git a/lucene/core/src/test/org/apache/lucene/search/TestIndexSearcher.java b/lucene/core/src/test/org/apache/lucene/search/TestIndexSearcher.java
index c2e16b17937..5c24a464417 100644
--- a/lucene/core/src/test/org/apache/lucene/search/TestIndexSearcher.java
+++ b/lucene/core/src/test/org/apache/lucene/search/TestIndexSearcher.java
@@ -29,9 +29,11 @@ import org.apache.lucene.index.RandomIndexWriter;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.util.BytesRef;
+import org.apache.lucene.util.IOUtils;
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.lucene.util.NamedThreadFactory;
 import org.apache.lucene.util._TestUtil;
+import org.junit.Test;
 
 public class TestIndexSearcher extends LuceneTestCase {
   Directory dir;
@@ -116,4 +118,25 @@ public class TestIndexSearcher extends LuceneTestCase {
     
     _TestUtil.shutdownExecutorService(service);
   }
+  
+  @Test
+  public void testSearchAfterPassedMaxDoc() throws Exception {
+    // LUCENE-5128: ensure we get a meaningful message if searchAfter exceeds maxDoc
+    Directory dir = newDirectory();
+    RandomIndexWriter w = new RandomIndexWriter(random(), dir);
+    w.addDocument(new Document());
+    IndexReader r = w.getReader();
+    w.close();
+    
+    IndexSearcher s = new IndexSearcher(r);
+    try {
+      s.searchAfter(new ScoreDoc(r.maxDoc(), 0.54f), new MatchAllDocsQuery(), 10);
+      fail("should have hit IllegalArgumentException when searchAfter exceeds maxDoc");
+    } catch (IllegalArgumentException e) {
+      // ok
+    } finally {
+      IOUtils.close(r, dir);
+    }
+  }
+  
 }
diff --git a/lucene/test-framework/src/java/org/apache/lucene/search/ShardSearchingTestBase.java b/lucene/test-framework/src/java/org/apache/lucene/search/ShardSearchingTestBase.java
index c25fe8bef0e..4b21b6d6226 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/search/ShardSearchingTestBase.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/search/ShardSearchingTestBase.java
@@ -370,20 +370,35 @@ public abstract class ShardSearchingTestBase extends LuceneTestCase {
       @Override
       public TopDocs searchAfter(ScoreDoc after, Query query, int numHits) throws IOException {
         final TopDocs[] shardHits = new TopDocs[nodeVersions.length];
+        // results are merged in that order: score, shardIndex, doc. therefore we set
+        // after to after.score and depending on the nodeID we set doc to either:
+        // - not collect any more documents with that score (only with worse score)
+        // - collect more documents with that score (and worse) following the last collected document
+        // - collect all documents with that score (and worse)
         ScoreDoc shardAfter = new ScoreDoc(after.doc, after.score);
-        for(int nodeID=0;nodeID<nodeVersions.length;nodeID++) {
+        for (int nodeID = 0; nodeID < nodeVersions.length; nodeID++) {
           if (nodeID < after.shardIndex) {
-            // If score is tied then no docs in this shard
-            // should be collected:
-            shardAfter.doc = Integer.MAX_VALUE;
+            // all documents with after.score were already collected, so collect
+            // only documents with worse scores.
+            final NodeState.ShardIndexSearcher s = nodes[nodeID].acquire(nodeVersions);
+            try {
+              // Setting after.doc to reader.maxDoc-1 is a way to tell
+              // TopScoreDocCollector that no more docs with that score should
+              // be collected. note that in practice the shard which sends the
+              // request to a remote shard won't have reader.maxDoc at hand, so
+              // it will send some arbitrary value which will be fixed on the
+              // other end.
+              shardAfter.doc = s.getIndexReader().maxDoc() - 1;
+            } finally {
+              nodes[nodeID].release(s);
+            }
           } else if (nodeID == after.shardIndex) {
-            // If score is tied then we break according to
-            // docID (like normal):  
+            // collect all documents following the last collected doc with
+            // after.score + documents with worse scores.  
             shardAfter.doc = after.doc;
           } else {
-            // If score is tied then all docs in this shard
-            // should be collected, because they come after
-            // the previous bottom:
+            // all documents with after.score (and worse) should be collected
+            // because they didn't make it to top-N in the previous round.
             shardAfter.doc = -1;
           }
           if (nodeID == myNodeID) {