From 2389fd83b52982ded4efe29f32d57a2b736c4561 Mon Sep 17 00:00:00 2001 From: Michael Marshall Date: Tue, 7 Jul 2026 21:41:03 -0500 Subject: [PATCH] Fix VectorScorer#bulk eagerly advancing its iterator 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.(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. --- lucene/CHANGES.txt | 2 ++ .../apache/lucene/search/VectorScorer.java | 10 +++--- .../lucene/search/TestVectorScorer.java | 32 +++++++++++++++++++ 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 20c0e582f184..950615c0cc80 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -215,6 +215,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) diff --git a/lucene/core/src/java/org/apache/lucene/search/VectorScorer.java b/lucene/core/src/java/org/apache/lucene/search/VectorScorer.java index f6a2c1f6d6ba..1969427ddaf5 100644 --- a/lucene/core/src/java/org/apache/lucene/search/VectorScorer.java +++ b/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; diff --git a/lucene/core/src/test/org/apache/lucene/search/TestVectorScorer.java b/lucene/core/src/test/org/apache/lucene/search/TestVectorScorer.java index 5432879a1341..de728f4db4da 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestVectorScorer.java +++ b/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 {