LUCENE-9618: Do not call IntervalIterator.nextInterval after NO_MORE_DOCS returned (#2095)

* LUCENE-9618: Do not call IntervalIterator.nextInterval after NO_MORE_DOCS is returned

lucene/CHANGES.txt

@@ -84,6 +84,8 @@ API Changes
 
 Improvements
 
+* LUCENE-9618: Do not call IntervalIterator.nextInterval after NO_MORE_DOCS is returned. (Haoyu Zhai)
+
 * LUCENE-9576: Improve ConcurrentMergeScheduler settings by default, assuming modern I/O.
   Previously Lucene was too conservative, jumping through hoops to detect if disks were SSD-backed.
   In many common modern cases (VMs, RAID arrays, containers, encrypted mounts, non-Linux OS),

lucene/queries/src/java/org/apache/lucene/queries/intervals/ConjunctionIntervalIterator.java

@@ -46,14 +46,18 @@ abstract class ConjunctionIntervalIterator extends IntervalIterator {
   @Override
   public int nextDoc() throws IOException {
     int doc = approximation.nextDoc();
-    reset();
+    if (doc != NO_MORE_DOCS) {
+      reset();
+    }
     return doc;
   }
 
   @Override
   public int advance(int target) throws IOException {
     int doc = approximation.advance(target);
-    reset();
+    if (doc != NO_MORE_DOCS) {
+      reset();
+    }
     return doc;
   }
 

lucene/queries/src/java/org/apache/lucene/queries/intervals/IntervalIterator.java

@@ -82,6 +82,12 @@ public abstract class IntervalIterator extends DocIdSetIterator {
   /**
    * Advance the iterator to the next interval
    *
+   * Should not be called after {@link DocIdSetIterator#NO_MORE_DOCS} is returned by {@link DocIdSetIterator#nextDoc()} or
+   * {@link DocIdSetIterator#advance(int)}.
+   * If that's the case in some existing code, please consider opening an issue.
+   * However, after {@link IntervalIterator#NO_MORE_INTERVALS} is returned by this method, it might be
+   * called again.
+   *
    * @return the start of the next interval, or {@link IntervalIterator#NO_MORE_INTERVALS} if
    *         there are no more intervals on the current document
    */

lucene/queries/src/test/org/apache/lucene/queries/intervals/OneTimeIntervalSource.java

@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.lucene.queries.intervals;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Collections;
+
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.search.MatchesIterator;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.QueryVisitor;
+
+/**
+ * A mock interval source that will only return a constant position for all documents
+ */
+public class OneTimeIntervalSource extends IntervalsSource {
+  @Override
+  public IntervalIterator intervals(String field, LeafReaderContext ctx) throws IOException {
+    return new IntervalIterator() {
+      int doc = -1;
+      boolean flag;
+      final int maxDoc = ctx.reader().maxDoc();
+      @Override
+      public int start() {
+        return 0;
+      }
+
+      @Override
+      public int end() {
+        return 0;
+      }
+
+      @Override
+      public int gaps() {
+        return 0;
+      }
+
+      /* only returns valid position every first time called per doc */
+      @Override
+      public int nextInterval() throws IOException {
+        if (doc != NO_MORE_DOCS) {
+          if (flag) {
+            flag = false;
+            return start();
+          } else {
+            return NO_MORE_INTERVALS;
+          }
+        }
+        throw new AssertionError("Called with docId == NO_MORE_DOCS");
+      }
+
+      @Override
+      public float matchCost() {
+        return 0;
+      }
+
+      @Override
+      public int docID() {
+        return doc;
+      }
+
+      @Override
+      public int nextDoc() throws IOException {
+        doc++;
+        if (doc >= maxDoc) {
+          doc = NO_MORE_DOCS;
+        }
+        flag = true;
+        return doc;
+      }
+
+      @Override
+      public int advance(int target) throws IOException {
+        doc = target;
+        if (doc >= maxDoc) {
+          doc = NO_MORE_DOCS;
+        }
+        flag = true;
+        return doc;
+      }
+
+      @Override
+      public long cost() {
+        return 0;
+      }
+    };
+  }
+
+  @Override
+  public IntervalMatchesIterator matches(String field, LeafReaderContext ctx, int doc) throws IOException {
+    return new IntervalMatchesIterator() {
+      boolean next = true;
+      @Override
+      public int gaps() {
+        return 0;
+      }
+
+      @Override
+      public int width() {
+        return 1;
+      }
+
+      @Override
+      public boolean next() throws IOException {
+        if (next) {
+          next = false;
+          return true;
+        }
+        return false;
+      }
+
+      @Override
+      public int startPosition() {
+        return 0;
+      }
+
+      @Override
+      public int endPosition() {
+        return 0;
+      }
+
+      @Override
+      public int startOffset() throws IOException {
+        return 0;
+      }
+
+      @Override
+      public int endOffset() throws IOException {
+        return 0;
+      }
+
+      @Override
+      public MatchesIterator getSubMatches() throws IOException {
+        return null;
+      }
+
+      @Override
+      public Query getQuery() {
+        return null;
+      }
+    };
+  }
+
+  @Override
+  public void visit(String field, QueryVisitor visitor) {
+
+  }
+
+  @Override
+  public int minExtent() {
+    return 0;
+  }
+
+  @Override
+  public Collection<IntervalsSource> pullUpDisjunctions() {
+    return Collections.singleton(this);
+  }
+
+  @Override
+  public int hashCode() {
+    return 0;
+  }
+
+  @Override
+  public boolean equals(Object other) {
+    return false;
+  }
+
+  @Override
+  public String toString() {
+    return "";
+  }
+}

lucene/queries/src/test/org/apache/lucene/queries/intervals/TestIntervalQuery.java

@@ -331,4 +331,10 @@ public class TestIntervalQuery extends LuceneTestCase {
     expectThrows(IllegalArgumentException.class, () -> new IntervalQuery(field, source, 1, -1f));
   }
 
+  public void testAdvanceBehavior() throws IOException {
+    Query q = new IntervalQuery(field,
+        Intervals.containing(Intervals.term("w1"), new OneTimeIntervalSource()));
+    checkHits(q, new int[]{ 0, 1, 2, 3});
+  }
+
 }