Fix VectorScorer#bulk eagerly advancing its iterator (#16377)

The default bulk() implementation advanced its iterator as a side
effect of construction, unlike Bulk.fromRandomScorerDense/Sparse,
which defer positioning to the first nextDocsAndScores call. Move
the positioning into the returned Bulk to match, and update the
javadoc.

The current behavior is problematic when combining the iterator
with others via conjunction. Given that this API is evolving
and the behavior is split I propose adopting the lazy behavior.

In one case, I hit the following error:

  Caused by: java.lang.IllegalArgumentException: Sub-iterators of ConjunctionDISI are not on the same document!
        at ConjunctionDISI.throwSubIteratorsNotOnSameDocument(ConjunctionDISI.java:149)
        at ConjunctionDISI.createConjunction(ConjunctionDISI.java:104)
        at ConjunctionUtils.intersectScorers(ConjunctionUtils.java:44)
        at ConjunctionScorer.<init>(ConjunctionScorer.java:36)
        at BooleanScorerSupplier.requiredBulkScorer(BooleanScorerSupplier.java:453)
        at BooleanScorerSupplier.booleanScorer(BooleanScorerSupplier.java:219)
        at BooleanScorerSupplier.bulkScorer(BooleanScorerSupplier.java:177)

The tests don't hit this error because it's trivial to confirm
that the bulk call is side effect free.

Author: michaeljmarshall

lucene/CHANGES.txt

@@ -224,6 +224,8 @@ Bug Fixes
 
 * GITHUB#16303: Add suppressed exception details when lock acquisition fails in NativeFSLockFactory. (Bharathi-Kanna)
 
+* Make VectorScorer#bulk default method lazily position its iterator on first use. (Michael Marshall)
+
 Changes in Runtime Behavior
 ---------------------
 * GITHUB#14187: The query cache is now disabled by default. (Adrien Grand)

lucene/core/src/java/org/apache/lucene/search/VectorScorer.java

@@ -50,8 +50,8 @@ public interface VectorScorer {
    * docs. The iterator of this instance of VectorScorer should be used and iterated in conjunction
    * with the provided matchingDocs iterator to score only the documents that are present in both
    * iterators. If the provided matchingDocs iterator is null, then all documents should be scored.
-   * Additionally, if the iterators are unpositioned (docID() == -1), this method should position
-   * them to the first document.
+   * Additionally, if the iterators are unpositioned (docID() == -1), the first call to {@link
+   * Bulk#nextDocsAndScores} should position them to the first document.
    *
    * @param matchingDocs Optional filter to iterate over the documents to score
    * @return a {@link Bulk} scorer
@@ -63,11 +63,11 @@ default Bulk bulk(DocIdSetIterator matchingDocs) throws IOException {
         matchingDocs == null
             ? iterator()
             : ConjunctionUtils.createConjunction(List.of(matchingDocs, iterator()), List.of());
-    if (iterator.docID() == -1) {
-      iterator.nextDoc();
-    }
     return (upTo, liveDocs, buffer) -> {
       assert upTo > 0;
+      if (iterator.docID() == -1) {
+        iterator.nextDoc();
+      }
       buffer.growNoCopy(DEFAULT_BULK_BATCH_SIZE);
       int size = 0;
       float maxScore = Float.NEGATIVE_INFINITY;

lucene/core/src/test/org/apache/lucene/search/TestVectorScorer.java

@@ -32,6 +32,8 @@
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.tests.index.RandomIndexWriter;
 import org.apache.lucene.tests.util.LuceneTestCase;
+import org.apache.lucene.util.BitSetIterator;
+import org.apache.lucene.util.FixedBitSet;
 
 public class TestVectorScorer extends LuceneTestCase {
 
@@ -64,6 +66,36 @@ public void testFindAll() throws IOException {
     }
   }
 
+  public void testBulkDoesNotEagerlyAdvanceIterator() throws IOException {
+    FixedBitSet bits = new FixedBitSet(10);
+    bits.set(2);
+    bits.set(5);
+    bits.set(7);
+    BitSetIterator sharedIterator = new BitSetIterator(bits, bits.cardinality());
+    VectorScorer scorer =
+        new VectorScorer() {
+          @Override
+          public float score() {
+            return 1f;
+          }
+
+          @Override
+          public DocIdSetIterator iterator() {
+            return sharedIterator;
+          }
+        };
+
+    assertEquals(-1, scorer.iterator().docID());
+    scorer.bulk(null); // constructing a Bulk must not have any observable side effect
+    assertEquals(-1, scorer.iterator().docID());
+
+    FixedBitSet matchBits = new FixedBitSet(10);
+    matchBits.set(2);
+    matchBits.set(7);
+    scorer.bulk(new BitSetIterator(matchBits, matchBits.cardinality()));
+    assertEquals(-1, scorer.iterator().docID());
+  }
+
   /** Creates a new directory and adds documents with the given vectors as kNN vector fields */
   private Directory getIndexStore(String field, VectorEncoding encoding, float[]... contents)
       throws IOException {