diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 3ce8e12813cc..2a5fcd9310ae 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -226,6 +226,11 @@ Bug Fixes * GITHUB#16565: IndexWriter#updateBinaryDocValue now rejects updating a field that is part of the index sort, matching IndexWriter#updateNumericDocValue. (Jim Ferenczi) +* GITHUB#15239: BatchScoreBulkScorer and DenseConjunctionBulkScorer no longer skip hits from + setMinCompetitiveScore when the search ScoreMode is exhaustive. Nested TopScoreDocCollectors can + still track a competitive threshold without dropping matches from an outer COMPLETE collector. + (Shrey Narayan, NextBrick) + * GITHUB#14049: Randomize KNN codec params in RandomCodec. Fixes scalar quantization div-by-zero when all values are identical. (Mike Sokolov) @@ -483,6 +488,11 @@ Optimizations Bug Fixes --------------------- +* GITHUB#15239: BatchScoreBulkScorer and DenseConjunctionBulkScorer no longer skip hits from + setMinCompetitiveScore when the search ScoreMode is exhaustive. Nested TopScoreDocCollectors can + still track a competitive threshold without dropping matches from an outer COMPLETE collector. + (Shrey Narayan, NextBrick) + * GITHUB#16546: Prevent RangeBulkScorer from passing empty ranges to LeafCollector. (jxy) * GITHUB#16450: Fix DocValuesRangeIterator.docIDRunEnd() returning incorrect run boundaries, diff --git a/lucene/core/src/java/org/apache/lucene/search/BatchScoreBulkScorer.java b/lucene/core/src/java/org/apache/lucene/search/BatchScoreBulkScorer.java index d1412fb5e45e..79489d052d09 100644 --- a/lucene/core/src/java/org/apache/lucene/search/BatchScoreBulkScorer.java +++ b/lucene/core/src/java/org/apache/lucene/search/BatchScoreBulkScorer.java @@ -28,9 +28,13 @@ class BatchScoreBulkScorer extends BulkScorer { private final SimpleScorable scorable = new SimpleScorable(); private final DocAndFloatFeatureBuffer buffer = new DocAndFloatFeatureBuffer(); private final Scorer scorer; + private final boolean applyMinCompetitiveScore; - BatchScoreBulkScorer(Scorer scorer) { + BatchScoreBulkScorer(Scorer scorer, ScoreMode scoreMode) { this.scorer = scorer; + // Exhaustive collection (COMPLETE*) must visit every match even if a nested + // collector calls setMinCompetitiveScore (GITHUB#15239). + this.applyMinCompetitiveScore = scoreMode.isExhaustive() == false; } @Override @@ -40,7 +44,9 @@ public int score(LeafCollector collector, Bits acceptDocs, int min, int max) thr } collector.setScorer(scorable); - scorer.setMinCompetitiveScore(scorable.minCompetitiveScore); + if (applyMinCompetitiveScore) { + scorer.setMinCompetitiveScore(scorable.minCompetitiveScore); + } if (scorer.docID() < min) { scorer.iterator().advance(min); @@ -51,11 +57,13 @@ public int score(LeafCollector collector, Bits acceptDocs, int min, int max) thr scorer.nextDocsAndScores(max, acceptDocs, buffer)) { for (int i = 0, size = buffer.size; i < size; i++) { float score = scorable.score = buffer.features[i]; - if (score >= scorable.minCompetitiveScore) { + if (applyMinCompetitiveScore == false || score >= scorable.minCompetitiveScore) { collector.collect(buffer.docs[i]); } } - scorer.setMinCompetitiveScore(scorable.minCompetitiveScore); + if (applyMinCompetitiveScore) { + scorer.setMinCompetitiveScore(scorable.minCompetitiveScore); + } } return scorer.docID(); diff --git a/lucene/core/src/java/org/apache/lucene/search/BooleanScorerSupplier.java b/lucene/core/src/java/org/apache/lucene/search/BooleanScorerSupplier.java index dbac4e610505..5781787ce81d 100644 --- a/lucene/core/src/java/org/apache/lucene/search/BooleanScorerSupplier.java +++ b/lucene/core/src/java/org/apache/lucene/search/BooleanScorerSupplier.java @@ -361,7 +361,7 @@ BulkScorer filteredOptionalBulkScorer() throws IOException { if (maxDoc >= DenseConjunctionBulkScorer.WINDOW_SIZE && cost >= maxDoc / DenseConjunctionBulkScorer.DENSITY_THRESHOLD_INVERSE) { - return DenseConjunctionBulkScorer.of(filters, maxDoc, 0f); + return DenseConjunctionBulkScorer.of(filters, maxDoc, 0f, scoreMode); } Scorer scorer = new ConjunctionScorer(filters, Collections.emptyList()); @@ -434,7 +434,7 @@ private BulkScorer requiredBulkScorer() throws IOException { if (requiredScoring.isEmpty() && maxDoc >= DenseConjunctionBulkScorer.WINDOW_SIZE && leadCost >= maxDoc / DenseConjunctionBulkScorer.DENSITY_THRESHOLD_INVERSE) { - return DenseConjunctionBulkScorer.of(requiredNoScoring, maxDoc, 0f); + return DenseConjunctionBulkScorer.of(requiredNoScoring, maxDoc, 0f, scoreMode); } else if (requiredNoScoring.stream() .map(Scorer::twoPhaseIterator) .allMatch(Objects::isNull)) { diff --git a/lucene/core/src/java/org/apache/lucene/search/CombinedFieldQuery.java b/lucene/core/src/java/org/apache/lucene/search/CombinedFieldQuery.java index 5adb17d7e61a..1824280d2b6a 100644 --- a/lucene/core/src/java/org/apache/lucene/search/CombinedFieldQuery.java +++ b/lucene/core/src/java/org/apache/lucene/search/CombinedFieldQuery.java @@ -265,12 +265,14 @@ class CombinedFieldWeight extends Weight { private final IndexSearcher searcher; private final TermStates[] termStates; private final Similarity.SimScorer simWeight; + private final ScoreMode scoreMode; CombinedFieldWeight(Query query, IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { super(query); assert scoreMode.needsScores(); this.searcher = searcher; + this.scoreMode = scoreMode; long docFreq = 0; long totalTermFreq = 0; termStates = new TermStates[fieldTerms.length]; @@ -394,7 +396,7 @@ public long cost() { @Override public BulkScorer bulkScorer() throws IOException { - return new BatchScoreBulkScorer(get(Long.MAX_VALUE)); + return new BatchScoreBulkScorer(get(Long.MAX_VALUE), scoreMode); } }; } diff --git a/lucene/core/src/java/org/apache/lucene/search/ConstantScoreScorerSupplier.java b/lucene/core/src/java/org/apache/lucene/search/ConstantScoreScorerSupplier.java index a8ff43ef9f2f..eec1711d7ee5 100644 --- a/lucene/core/src/java/org/apache/lucene/search/ConstantScoreScorerSupplier.java +++ b/lucene/core/src/java/org/apache/lucene/search/ConstantScoreScorerSupplier.java @@ -89,7 +89,7 @@ public final BulkScorer bulkScorer() throws IOException { iterators = Collections.emptyList(); twoPhases = Collections.singletonList(twoPhase); } - return new DenseConjunctionBulkScorer(iterators, twoPhases, maxDoc, score); + return new DenseConjunctionBulkScorer(iterators, twoPhases, maxDoc, score, scoreMode); } else if (scoreMode.needsScores() == false) { // Collect window-by-window via intoBitSet. For a two-phase iterator this confirms matches in // its (possibly bulk) intoBitSet; the only overhead over a plain leap-frog is the reusable diff --git a/lucene/core/src/java/org/apache/lucene/search/DenseConjunctionBulkScorer.java b/lucene/core/src/java/org/apache/lucene/search/DenseConjunctionBulkScorer.java index 9f59eb2c2bba..4092f066930f 100644 --- a/lucene/core/src/java/org/apache/lucene/search/DenseConjunctionBulkScorer.java +++ b/lucene/core/src/java/org/apache/lucene/search/DenseConjunctionBulkScorer.java @@ -72,6 +72,7 @@ void intoBitSet(int upTo, FixedBitSet bitSet, int offset) throws IOException { private final int maxDoc; private final List iterators; private final SimpleScorable scorable; + private final boolean applyMinCompetitiveScore; private final FixedBitSet windowMatches = new FixedBitSet(WINDOW_SIZE); private final FixedBitSet clauseWindowMatches = new FixedBitSet(WINDOW_SIZE); @@ -80,7 +81,8 @@ void intoBitSet(int upTo, FixedBitSet bitSet, int offset) throws IOException { private final List windowApproximations = new ArrayList<>(); private final List windowTwoPhases = new ArrayList<>(); - static DenseConjunctionBulkScorer of(List filters, int maxDoc, float constantScore) { + static DenseConjunctionBulkScorer of( + List filters, int maxDoc, float constantScore, ScoreMode scoreMode) { List iterators = new ArrayList<>(); List twoPhases = new ArrayList<>(); for (Scorer filter : filters) { @@ -91,14 +93,15 @@ static DenseConjunctionBulkScorer of(List filters, int maxDoc, float con iterators.add(filter.iterator()); } } - return new DenseConjunctionBulkScorer(iterators, twoPhases, maxDoc, constantScore); + return new DenseConjunctionBulkScorer(iterators, twoPhases, maxDoc, constantScore, scoreMode); } DenseConjunctionBulkScorer( List iterators, List twoPhases, int maxDoc, - float constantScore) { + float constantScore, + ScoreMode scoreMode) { if (iterators.isEmpty() && twoPhases.isEmpty()) { throw new IllegalArgumentException("Expected one or more iterators, got 0"); } @@ -121,6 +124,9 @@ static DenseConjunctionBulkScorer of(List filters, int maxDoc, float con .thenComparingDouble(w -> w.twoPhase() == null ? 0 : w.twoPhase().matchCost())); this.scorable = new SimpleScorable(); scorable.score = constantScore; + // Exhaustive collection must visit every match even if a nested collector calls + // setMinCompetitiveScore (GITHUB#15239). + this.applyMinCompetitiveScore = scoreMode.isExhaustive() == false; } @Override @@ -145,7 +151,7 @@ public int score(LeafCollector collector, Bits acceptDocs, int min, int max) thr } while (min < max) { - if (scorable.minCompetitiveScore > scorable.score) { + if (applyMinCompetitiveScore && scorable.minCompetitiveScore > scorable.score) { return DocIdSetIterator.NO_MORE_DOCS; } min = scoreWindow(collector, acceptDocs, iterators, min, max); diff --git a/lucene/core/src/java/org/apache/lucene/search/ScoreMode.java b/lucene/core/src/java/org/apache/lucene/search/ScoreMode.java index 90ddf4ac52a0..dec7e3c43cf6 100644 --- a/lucene/core/src/java/org/apache/lucene/search/ScoreMode.java +++ b/lucene/core/src/java/org/apache/lucene/search/ScoreMode.java @@ -19,7 +19,12 @@ /** Different modes of search. */ public enum ScoreMode { - /** Produced scorers will allow visiting all matches and get their score. */ + /** + * Produced scorers will allow visiting all matches and get their score. This score mode is + * exhaustive: a call to {@link Scorable#setMinCompetitiveScore(float)} must never cause a match + * to be skipped, even if a nested collector makes one in violation of the contract documented on + * that method. + */ COMPLETE(true, true), /** Produced scorers will allow visiting all matches but scores won't be available. */ diff --git a/lucene/core/src/java/org/apache/lucene/search/TermQuery.java b/lucene/core/src/java/org/apache/lucene/search/TermQuery.java index 290beb1acc1c..905af9486b50 100644 --- a/lucene/core/src/java/org/apache/lucene/search/TermQuery.java +++ b/lucene/core/src/java/org/apache/lucene/search/TermQuery.java @@ -172,7 +172,7 @@ public BulkScorer bulkScorer() throws IOException { return ConstantScoreScorerSupplier.fromIterator(iterator, 0f, scoreMode, maxDoc) .bulkScorer(); } - return new BatchScoreBulkScorer(get(Long.MAX_VALUE)); + return new BatchScoreBulkScorer(get(Long.MAX_VALUE), scoreMode); } @Override diff --git a/lucene/core/src/test/org/apache/lucene/search/ReadAheadMatchAllDocsQuery.java b/lucene/core/src/test/org/apache/lucene/search/ReadAheadMatchAllDocsQuery.java index 4e62b5ae70f5..4bdaa2c66eb0 100644 --- a/lucene/core/src/test/org/apache/lucene/search/ReadAheadMatchAllDocsQuery.java +++ b/lucene/core/src/test/org/apache/lucene/search/ReadAheadMatchAllDocsQuery.java @@ -78,7 +78,7 @@ public BulkScorer bulkScorer() throws IOException { List clauses = Collections.singletonList(DocIdSetIterator.all(context.reader().maxDoc())); return new DenseConjunctionBulkScorer( - clauses, Collections.emptyList(), context.reader().maxDoc(), score()); + clauses, Collections.emptyList(), context.reader().maxDoc(), score(), scoreMode); } @Override diff --git a/lucene/core/src/test/org/apache/lucene/search/TestDenseConjunctionBulkScorer.java b/lucene/core/src/test/org/apache/lucene/search/TestDenseConjunctionBulkScorer.java index e482b8e129c7..93c4d907ad86 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestDenseConjunctionBulkScorer.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestDenseConjunctionBulkScorer.java @@ -47,7 +47,8 @@ public void testSameMatches() throws IOException { new BitSetIterator(clause3, clause3.approximateCardinality())), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); // AssertingBulkScorer randomly splits the scored range into smaller ranges scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet result = new FixedBitSet(maxDoc); @@ -76,7 +77,8 @@ public void collect(int doc) throws IOException { new BitSetIterator(clause3, clause3.approximateCardinality())), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, null, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(clause1.cardinality(), collector.count); @@ -99,7 +101,8 @@ public void testApplyAcceptDocs() throws IOException { new BitSetIterator(clause2, clause2.approximateCardinality())), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); // AssertingBulkScorer randomly splits the scored range into smaller ranges scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet result = new FixedBitSet(maxDoc); @@ -127,7 +130,8 @@ public void collect(int doc) throws IOException { new BitSetIterator(clause2, clause2.approximateCardinality())), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, acceptDocs, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(acceptDocs.cardinality(), collector.count); @@ -148,7 +152,8 @@ public void testEmptyIntersection() throws IOException { new BitSetIterator(clause2, clause2.approximateCardinality())), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); // AssertingBulkScorer randomly splits the scored range into smaller ranges scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet result = new FixedBitSet(maxDoc); @@ -176,7 +181,8 @@ public void collect(int doc) throws IOException { new BitSetIterator(clause2, clause2.approximateCardinality())), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, null, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(0, collector.count); @@ -198,7 +204,8 @@ public void testClustered() throws IOException { new BitSetIterator(clause3, clause3.approximateCardinality())), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); // AssertingBulkScorer randomly splits the scored range into smaller ranges scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet result = new FixedBitSet(maxDoc); @@ -230,7 +237,8 @@ public void collect(int doc) throws IOException { new BitSetIterator(clause3, clause3.approximateCardinality())), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, null, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(expected.cardinality(), collector.count); @@ -261,7 +269,8 @@ public void testSparseAfter2ndClause() throws IOException { new BitSetIterator(clause3, clause3.approximateCardinality())), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); // AssertingBulkScorer randomly splits the scored range into smaller ranges scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet result = new FixedBitSet(maxDoc); @@ -294,7 +303,8 @@ public void collect(int doc) throws IOException { new BitSetIterator(clause3, clause3.approximateCardinality())), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, null, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(expected.cardinality(), collector.count); @@ -307,7 +317,8 @@ public void testMatchAllNoLiveDocs() throws IOException { Collections.singletonList(DocIdSetIterator.all(maxDoc)), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); // AssertingBulkScorer randomly splits the scored range into smaller ranges scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet result = new FixedBitSet(maxDoc); @@ -334,7 +345,8 @@ public void collect(int doc) throws IOException { Collections.singletonList(DocIdSetIterator.all(maxDoc)), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, null, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(maxDoc, collector.count); @@ -347,7 +359,8 @@ public void testMatchAllWithLiveDocs() throws IOException { Collections.singletonList(DocIdSetIterator.all(maxDoc)), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); // AssertingBulkScorer randomly splits the scored range into smaller ranges scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet acceptDocs = new FixedBitSet(maxDoc); @@ -378,7 +391,8 @@ public void collect(int doc) throws IOException { Collections.singletonList(DocIdSetIterator.all(maxDoc)), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, acceptDocs, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(acceptDocs.cardinality(), collector.count); @@ -396,7 +410,8 @@ public void testOneClauseNoLiveDocs() throws IOException { new BitSetIterator(clause1, clause1.approximateCardinality())), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); // AssertingBulkScorer randomly splits the scored range into smaller ranges scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet result = new FixedBitSet(maxDoc); @@ -423,7 +438,8 @@ public void collect(int doc) throws IOException { new BitSetIterator(clause1, clause1.approximateCardinality())), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, null, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(clause1.cardinality(), collector.count); @@ -441,7 +457,8 @@ public void testOneClauseWithLiveDocs() throws IOException { new BitSetIterator(clause1, clause1.approximateCardinality())), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); // AssertingBulkScorer randomly splits the scored range into smaller ranges scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet acceptDocs = new FixedBitSet(maxDoc); @@ -476,7 +493,8 @@ public void collect(int doc) throws IOException { new BitSetIterator(clause1, clause1.approximateCardinality())), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, acceptDocs, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(expected.cardinality(), collector.count); @@ -499,7 +517,8 @@ public void testStopOnMinCompetitiveScore() throws IOException { new BitSetIterator(clause2, clause2.approximateCardinality())), Collections.emptyList(), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); // AssertingBulkScorer randomly splits the scored range into smaller ranges scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet result = new FixedBitSet(maxDoc); @@ -560,7 +579,11 @@ public int docIDRunEnd() throws IOException { }; BulkScorer scorer = new DenseConjunctionBulkScorer( - Collections.emptyList(), Collections.singletonList(twoPhase), maxDoc, 0f); + Collections.emptyList(), + Collections.singletonList(twoPhase), + maxDoc, + 0f, + ScoreMode.TOP_SCORES); int[] collectedViaRange = {0}; scorer.score( new LeafCollector() { @@ -644,7 +667,11 @@ public float matchCost() { } }; return new DenseConjunctionBulkScorer( - Collections.emptyList(), Collections.singletonList(twoPhase), maxDoc, 0f); + Collections.emptyList(), + Collections.singletonList(twoPhase), + maxDoc, + 0f, + ScoreMode.TOP_SCORES); } /** A collector that records every collected doc, whichever collection path delivers it. */ @@ -691,7 +718,8 @@ public void testRangeIntersection() throws IOException { Collections.shuffle(clauses, random()); BulkScorer scorer = - new DenseConjunctionBulkScorer(clauses, Collections.emptyList(), maxDoc, 0f); + new DenseConjunctionBulkScorer( + clauses, Collections.emptyList(), maxDoc, 0f, ScoreMode.TOP_SCORES); // Matches are collected as a single DocIdStream scorer.score( new LeafCollector() { @@ -730,7 +758,9 @@ public void finish() throws IOException { clause2 = DocIdSetIterator.range(30_000, 80_000); clauses = Arrays.asList(clause1, clause2); Collections.shuffle(clauses, random()); - scorer = new DenseConjunctionBulkScorer(clauses, Collections.emptyList(), maxDoc, 0f); + scorer = + new DenseConjunctionBulkScorer( + clauses, Collections.emptyList(), maxDoc, 0f, ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, null, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(30_000, collector.count); @@ -749,7 +779,8 @@ public void testRangeIntersectionWithLiveDocs() throws IOException { } BulkScorer scorer = - new DenseConjunctionBulkScorer(clauses, Collections.emptyList(), maxDoc, 0f); + new DenseConjunctionBulkScorer( + clauses, Collections.emptyList(), maxDoc, 0f, ScoreMode.TOP_SCORES); // AssertingBulkScorer randomly splits the scored range into smaller ranges scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet result = new FixedBitSet(maxDoc); @@ -778,7 +809,9 @@ public void collect(int doc) throws IOException { clause2 = DocIdSetIterator.range(30_000, 80_000); clauses = Arrays.asList(clause1, clause2); Collections.shuffle(clauses, random()); - scorer = new DenseConjunctionBulkScorer(clauses, Collections.emptyList(), maxDoc, 0f); + scorer = + new DenseConjunctionBulkScorer( + clauses, Collections.emptyList(), maxDoc, 0f, ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, acceptDocs, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(expected.cardinality(), collector.count); @@ -807,7 +840,8 @@ public void intoBitSet(int upTo, FixedBitSet bitSet, int offset) throws IOExcept Arrays.asList(rangeIterator, new BitSetIterator(clause2, 40_000)); Collections.shuffle(clauses, random()); BulkScorer scorer = - new DenseConjunctionBulkScorer(clauses, Collections.emptyList(), maxDoc, 0f); + new DenseConjunctionBulkScorer( + clauses, Collections.emptyList(), maxDoc, 0f, ScoreMode.TOP_SCORES); FixedBitSet result = new FixedBitSet(maxDoc); scorer.score( new LeafCollector() { @@ -833,7 +867,9 @@ public void collect(int doc) throws IOException { clauses = Arrays.asList(DocIdSetIterator.range(10_000, 50_000), new BitSetIterator(clause2, 40_000)); Collections.shuffle(clauses, random()); - scorer = new DenseConjunctionBulkScorer(clauses, Collections.emptyList(), maxDoc, 0f); + scorer = + new DenseConjunctionBulkScorer( + clauses, Collections.emptyList(), maxDoc, 0f, ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, null, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(expected.cardinality(), collector.count); @@ -855,7 +891,8 @@ public void testMixedRangeIntersectionWithLiveDocs() throws IOException { } BulkScorer scorer = - new DenseConjunctionBulkScorer(clauses, Collections.emptyList(), maxDoc, 0f); + new DenseConjunctionBulkScorer( + clauses, Collections.emptyList(), maxDoc, 0f, ScoreMode.TOP_SCORES); // AssertingBulkScorer randomly splits the scored range into smaller ranges scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet result = new FixedBitSet(maxDoc); @@ -883,7 +920,9 @@ public void collect(int doc) throws IOException { clauses = Arrays.asList(DocIdSetIterator.range(10_000, 60_000), new BitSetIterator(clause2, 50_000)); Collections.shuffle(clauses, random()); - scorer = new DenseConjunctionBulkScorer(clauses, Collections.emptyList(), maxDoc, 0f); + scorer = + new DenseConjunctionBulkScorer( + clauses, Collections.emptyList(), maxDoc, 0f, ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, acceptDocs, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(expected.cardinality(), collector.count); @@ -914,7 +953,8 @@ public void testTwoPhaseIterators() throws IOException { new RandomTwoPhaseView( random(), new BitSetIterator(clause3, clause3.approximateCardinality()))), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet result = new FixedBitSet(maxDoc); scorer.score( @@ -949,7 +989,8 @@ public void collect(int doc) throws IOException { new RandomTwoPhaseView( random(), new BitSetIterator(clause3, clause3.approximateCardinality()))), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, null, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(expected.cardinality(), collector.count); @@ -978,7 +1019,8 @@ public void testTwoPhaseIteratorsWithLiveDocs() throws IOException { new RandomTwoPhaseView( random(), new BitSetIterator(clause2, clause2.approximateCardinality()))), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet result = new FixedBitSet(maxDoc); scorer.score( @@ -1011,7 +1053,8 @@ public void collect(int doc) throws IOException { new RandomTwoPhaseView( random(), new BitSetIterator(clause2, clause2.approximateCardinality()))), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, acceptDocs, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(expected.cardinality(), collector.count); @@ -1041,7 +1084,8 @@ public void testMixedTwoPhaseIterators() throws IOException { new RandomTwoPhaseView( random(), new BitSetIterator(clause3, clause3.approximateCardinality()))), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet result = new FixedBitSet(maxDoc); scorer.score( @@ -1075,7 +1119,8 @@ public void collect(int doc) throws IOException { new RandomTwoPhaseView( random(), new BitSetIterator(clause3, clause3.approximateCardinality()))), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, null, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(expected.cardinality(), collector.count); @@ -1094,7 +1139,8 @@ public void testTwoPhaseRangeIntersection() throws IOException { Collections.shuffle(clauses, random()); BulkScorer scorer = - new DenseConjunctionBulkScorer(Collections.emptyList(), clauses, maxDoc, 0f); + new DenseConjunctionBulkScorer( + Collections.emptyList(), clauses, maxDoc, 0f, ScoreMode.TOP_SCORES); // Matches arrive in order, as ranges and/or bit-set windows. scorer.score( new LeafCollector() { @@ -1136,7 +1182,9 @@ public void finish() throws IOException { new RandomTwoPhaseView(random(), clause3)); Collections.shuffle(clauses, random()); - scorer = new DenseConjunctionBulkScorer(Collections.emptyList(), clauses, maxDoc, 0f); + scorer = + new DenseConjunctionBulkScorer( + Collections.emptyList(), clauses, maxDoc, 0f, ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, null, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(30_000, collector.count); @@ -1193,7 +1241,11 @@ public float matchCost() { BulkScorer scorer = new DenseConjunctionBulkScorer( - Collections.singletonList(lead), Arrays.asList(expensive, cheap), maxDoc, 0f); + Collections.singletonList(lead), + Arrays.asList(expensive, cheap), + maxDoc, + 0f, + ScoreMode.TOP_SCORES); FixedBitSet result = new FixedBitSet(maxDoc); scorer.score( new LeafCollector() { @@ -1240,7 +1292,8 @@ public void testMixedTwoPhaseRangeIntersection() throws IOException { new RandomTwoPhaseView(random(), clause2), new RandomTwoPhaseView(random(), clause3)), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); // Matches arrive in order, as ranges and/or bit-set windows. scorer.score( new LeafCollector() { @@ -1283,7 +1336,8 @@ public void finish() throws IOException { new RandomTwoPhaseView(random(), clause2), new RandomTwoPhaseView(random(), clause3)), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, null, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(30_000, collector.count); @@ -1302,7 +1356,8 @@ public void testMixedRangeIntersectionTwoPhase1() throws IOException { Collections.singletonList(new BitSetIterator(clause2, clause2.cardinality())), Collections.singletonList(new RandomTwoPhaseView(random(), clause1)), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); // AssertingBulkScorer randomly splits the scored range into smaller ranges scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet result = new FixedBitSet(maxDoc); @@ -1333,7 +1388,8 @@ public void collect(int doc) throws IOException { Collections.singletonList(new BitSetIterator(clause2, clause2.cardinality())), Collections.singletonList(new RandomTwoPhaseView(random(), clause1)), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, null, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(expected.cardinality(), collector.count); @@ -1355,7 +1411,8 @@ public void testMixedRangeIntersectionTwoPhase2() throws IOException { new RandomTwoPhaseView( random(), new BitSetIterator(clause2, clause2.cardinality()))), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); // AssertingBulkScorer randomly splits the scored range into smaller ranges scorer = AssertingBulkScorer.wrap(random(), scorer, maxDoc); FixedBitSet result = new FixedBitSet(maxDoc); @@ -1389,7 +1446,8 @@ public void collect(int doc) throws IOException { new RandomTwoPhaseView( random(), new BitSetIterator(clause2, clause2.cardinality()))), maxDoc, - 0f); + 0f, + ScoreMode.TOP_SCORES); CountingLeafCollector collector = new CountingLeafCollector(); scorer.score(collector, null, 0, DocIdSetIterator.NO_MORE_DOCS); assertEquals(expected.cardinality(), collector.count); diff --git a/lucene/core/src/test/org/apache/lucene/search/TestExhaustiveScoreModeNoPruning.java b/lucene/core/src/test/org/apache/lucene/search/TestExhaustiveScoreModeNoPruning.java new file mode 100644 index 000000000000..ad69d1f791ee --- /dev/null +++ b/lucene/core/src/test/org/apache/lucene/search/TestExhaustiveScoreModeNoPruning.java @@ -0,0 +1,432 @@ +/* + * 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.search; + +import java.io.IOException; +import java.util.concurrent.atomic.AtomicInteger; +import org.apache.lucene.document.Document; +import org.apache.lucene.document.Field.Store; +import org.apache.lucene.document.IntPoint; +import org.apache.lucene.document.NumericDocValuesField; +import org.apache.lucene.document.StringField; +import org.apache.lucene.document.TextField; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.IndexWriter; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.Term; +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.BytesRef; + +/** + * An exhaustive {@link ScoreMode} ({@link ScoreMode#COMPLETE} or {@link + * ScoreMode#COMPLETE_NO_SCORES}) must visit every match, even when a nested collector calls {@link + * Scorable#setMinCompetitiveScore(float)} in violation of the contract documented on that method. + * + *

These tests exercise every bulk scorer that reads {@code SimpleScorable#minCompetitiveScore}: + * {@code BatchScoreBulkScorer} (term and combined-field queries) and {@code + * DenseConjunctionBulkScorer}, which is reached both from {@code BooleanScorerSupplier} for a + * FILTER-only conjunction and from {@code ConstantScoreScorerSupplier}, which backs {@link + * MatchAllDocsQuery}, {@link FieldExistsQuery} and point range queries. + */ +public class TestExhaustiveScoreModeNoPruning extends LuceneTestCase { + + /** + * Index size used throughout. Comfortably above {@code DenseConjunctionBulkScorer.WINDOW_SIZE} + * (4096) so that collection spans several windows, which is all these tests need: the failure + * mode is identical at 20k and at 100k documents. + */ + private static final int NUM_DOCS = 20_000; + + /** + * Outer collector that declares an exhaustive score mode and forwards to an inner + * TopScoreDocCollector, which will call setMinCompetitiveScore once its queue is full. Counts + * every collect() call so that we can assert nothing was pruned. + */ + private static final class CountingWrapper extends SimpleCollector { + final AtomicInteger totalCalls = new AtomicInteger(); + private final TopScoreDocCollector in; + private final ScoreMode scoreMode; + private LeafCollector leafIn; + + CountingWrapper(TopScoreDocCollector in, ScoreMode scoreMode) { + this.in = in; + this.scoreMode = scoreMode; + } + + @Override + protected void doSetNextReader(LeafReaderContext context) throws IOException { + leafIn = in.getLeafCollector(context); + } + + @Override + public void collect(int doc) throws IOException { + totalCalls.incrementAndGet(); + leafIn.collect(doc); + } + + @Override + public void setScorer(Scorable scorer) throws IOException { + super.setScorer(scorer); + leafIn.setScorer(scorer); + } + + @Override + public ScoreMode scoreMode() { + return scoreMode; + } + } + + private static int runCollect(IndexSearcher searcher, Query query, ScoreMode mode) + throws IOException { + TopScoreDocCollector in = new TopScoreDocCollectorManager(1, 1).newCollector(); + CountingWrapper out = new CountingWrapper(in, mode); + searcher.search(query, out); + return out.totalCalls.intValue(); + } + + /** Single-segment, single-threaded searcher so that bulk scorer selection is deterministic. */ + private static IndexSearcher plainSearcher(IndexReader reader) { + IndexSearcher searcher = new IndexSearcher(reader); + searcher.setQueryCache(null); + return searcher; + } + + /** A body value whose term count varies with {@code i}, so documents get different norms. */ + private static String body(int i) { + return ("hit " + "pad ".repeat(1 + (i % 8))).trim(); + } + + // --------------------------------------------------------------------------------------------- + // 1. BatchScoreBulkScorer via TermQuery + // --------------------------------------------------------------------------------------------- + + public void testTermQueryCompleteVisitsEveryMatch() throws Exception { + try (Directory dir = newDirectory(); + RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + for (int i = 0; i < NUM_DOCS; ++i) { + Document doc = new Document(); + // Varying term counts give the docs different norms, so scores differ and the inner + // TopScoreDocCollector really does raise a competitive threshold. + doc.add(new TextField("body", body(i), Store.NO)); + w.addDocument(doc); + } + w.forceMerge(1); + try (IndexReader reader = w.getReader()) { + IndexSearcher searcher = plainSearcher(reader); + Query q = new TermQuery(new Term("body", "hit")); + assertEquals( + "COMPLETE pruned matches", NUM_DOCS, runCollect(searcher, q, ScoreMode.COMPLETE)); + assertEquals( + "COMPLETE_NO_SCORES pruned matches", + NUM_DOCS, + runCollect(searcher, q, ScoreMode.COMPLETE_NO_SCORES)); + // TOP_DOCS_WITH_SCORES is not exhaustive, so pruning is expected there. + int topDocsWithScores = runCollect(searcher, q, ScoreMode.TOP_DOCS_WITH_SCORES); + assertTrue( + "TOP_DOCS_WITH_SCORES unexpectedly exhaustive, collected=" + topDocsWithScores, + topDocsWithScores < NUM_DOCS); + } + } + } + + /** + * Same as above, but through newSearcher so that reader wrapping and intra-segment concurrency + * are exercised too. + * + *

wrapWithAssertions is deliberately false: {@link CountingWrapper} violates the {@link + * Scorable#setMinCompetitiveScore(float)} contract on purpose, and AssertingScorer asserts that + * only {@link ScoreMode#TOP_SCORES} may call it. + */ + public void testTermQueryCompleteWithConcurrentSearcher() throws Exception { + try (Directory dir = newDirectory(); + RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + for (int i = 0; i < NUM_DOCS; ++i) { + Document doc = new Document(); + doc.add(new TextField("body", body(i), Store.NO)); + w.addDocument(doc); + } + w.forceMerge(1); + try (IndexReader reader = w.getReader()) { + IndexSearcher searcher = newSearcher(reader, true, false, true); + searcher.setQueryCache(null); + assertEquals( + NUM_DOCS, + runCollect(searcher, new TermQuery(new Term("body", "hit")), ScoreMode.COMPLETE)); + } + } + } + + /** TOP_SCORES must keep pruning: the fix must not disable the optimization. */ + public void testTopScoresStillPrunes() throws Exception { + try (Directory dir = newDirectory(); + RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + for (int i = 0; i < NUM_DOCS; ++i) { + Document doc = new Document(); + doc.add(new TextField("body", body(i), Store.NO)); + w.addDocument(doc); + } + w.forceMerge(1); + try (IndexReader reader = w.getReader()) { + IndexSearcher searcher = plainSearcher(reader); + int collected = + runCollect(searcher, new TermQuery(new Term("body", "hit")), ScoreMode.TOP_SCORES); + assertTrue( + "TOP_SCORES should still skip non-competitive hits, collected=" + collected, + collected < NUM_DOCS); + } + } + } + + // --------------------------------------------------------------------------------------------- + // 2. DenseConjunctionBulkScorer via a FILTER-only BooleanQuery + // --------------------------------------------------------------------------------------------- + + public void testFilterOnlyBooleanQueryCompleteVisitsEveryMatch() throws Exception { + try (Directory dir = newDirectory(); + RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + for (int i = 0; i < NUM_DOCS; ++i) { + Document doc = new Document(); + doc.add(new StringField("a", "x", Store.NO)); + doc.add(new StringField("b", "y", Store.NO)); + w.addDocument(doc); + } + w.forceMerge(1); + try (IndexReader reader = w.getReader()) { + IndexSearcher searcher = plainSearcher(reader); + BooleanQuery q = + new BooleanQuery.Builder() + .add(new TermQuery(new Term("a", "x")), BooleanClause.Occur.FILTER) + .add(new TermQuery(new Term("b", "y")), BooleanClause.Occur.FILTER) + .build(); + assertEquals( + "FILTER-only COMPLETE pruned matches", + NUM_DOCS, + runCollect(searcher, q, ScoreMode.COMPLETE)); + } + } + } + + /** Same, with three FILTER clauses and a sparser match set that still clears the density bar. */ + public void testFilterOnlyBooleanQueryThreeClausesSparse() throws Exception { + try (Directory dir = newDirectory(); + RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + int expected = 0; + for (int i = 0; i < NUM_DOCS; ++i) { + Document doc = new Document(); + doc.add(new StringField("a", "x", Store.NO)); + doc.add(new StringField("b", i % 2 == 0 ? "y" : "n", Store.NO)); + doc.add(new StringField("c", i % 3 == 0 ? "z" : "n", Store.NO)); + if (i % 2 == 0 && i % 3 == 0) { + expected++; + } + w.addDocument(doc); + } + w.forceMerge(1); + try (IndexReader reader = w.getReader()) { + IndexSearcher searcher = plainSearcher(reader); + BooleanQuery q = + new BooleanQuery.Builder() + .add(new TermQuery(new Term("a", "x")), BooleanClause.Occur.FILTER) + .add(new TermQuery(new Term("b", "y")), BooleanClause.Occur.FILTER) + .add(new TermQuery(new Term("c", "z")), BooleanClause.Occur.FILTER) + .build(); + assertEquals(expected, runCollect(searcher, q, ScoreMode.COMPLETE)); + } + } + } + + // --------------------------------------------------------------------------------------------- + // 3. BatchScoreBulkScorer via CombinedFieldQuery (the second call site the PR patches) + // --------------------------------------------------------------------------------------------- + + public void testCombinedFieldQueryCompleteVisitsEveryMatch() throws Exception { + try (Directory dir = newDirectory(); + RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + for (int i = 0; i < NUM_DOCS; ++i) { + Document doc = new Document(); + doc.add(new TextField("t", body(i), Store.NO)); + doc.add(new TextField("u", ("hit " + "pad ".repeat(1 + (i % 5))).trim(), Store.NO)); + w.addDocument(doc); + } + w.forceMerge(1); + try (IndexReader reader = w.getReader()) { + IndexSearcher searcher = plainSearcher(reader); + Query q = + new CombinedFieldQuery.Builder(new BytesRef("hit")) + .addField("t", 1.0f) + .addField("u", 1.0f) + .build(); + assertEquals( + "CombinedFieldQuery under COMPLETE pruned matches", + NUM_DOCS, + runCollect(searcher, q, ScoreMode.COMPLETE)); + } + } + } + + // --------------------------------------------------------------------------------------------- + // 4. Cross-check: totalHits from a COMPLETE count must equal the number of collect() calls + // --------------------------------------------------------------------------------------------- + + public void testCountAgreesWithCollectCalls() throws Exception { + try (Directory dir = newDirectory(); + RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + for (int i = 0; i < NUM_DOCS; ++i) { + Document doc = new Document(); + doc.add(new StringField("a", "x", Store.NO)); + doc.add(new StringField("b", "y", Store.NO)); + doc.add(new TextField("body", body(i), Store.NO)); + w.addDocument(doc); + } + w.forceMerge(1); + try (DirectoryReader reader = w.getReader()) { + IndexSearcher searcher = plainSearcher(reader); + Query bq = + new BooleanQuery.Builder() + .add(new TermQuery(new Term("a", "x")), BooleanClause.Occur.FILTER) + .add(new TermQuery(new Term("b", "y")), BooleanClause.Occur.FILTER) + .build(); + Query tq = new TermQuery(new Term("body", "hit")); + for (Query q : new Query[] {bq, tq}) { + assertEquals( + "count() and COMPLETE collection disagree for " + q, + searcher.count(q), + runCollect(searcher, q, ScoreMode.COMPLETE)); + } + } + } + } + + // --------------------------------------------------------------------------------------------- + // 5. Constant-score queries that reach DenseConjunctionBulkScorer via + // ConstantScoreScorerSupplier#bulkScorer with a non-zero constant score. + // --------------------------------------------------------------------------------------------- + + public void testConstantScoreQueriesVisitEveryMatch() throws Exception { + try (Directory dir = newDirectory(); + RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + for (int i = 0; i < NUM_DOCS; ++i) { + Document doc = new Document(); + doc.add(new StringField("a", "x", Store.NO)); + doc.add(new NumericDocValuesField("n", i)); + doc.add(new IntPoint("p", i)); + w.addDocument(doc); + } + w.forceMerge(1); + try (IndexReader reader = w.getReader()) { + IndexSearcher searcher = plainSearcher(reader); + Query[] queries = { + new MatchAllDocsQuery(), + new FieldExistsQuery("n"), + IntPoint.newRangeQuery("p", 0, NUM_DOCS), + // ConstantScoreQuery is safe on its own: it wraps the inner bulk scorer in + // ConstantBulkScorer. Kept here as a regression guard on that wrapping. + new ConstantScoreQuery(new TermQuery(new Term("a", "x"))), + }; + for (Query q : queries) { + assertEquals( + "COMPLETE pruned matches for " + q, + NUM_DOCS, + runCollect(searcher, q, ScoreMode.COMPLETE)); + } + } + } + } + + // --------------------------------------------------------------------------------------------- + // 6. With deletions (acceptDocs != null) the "whole run matches" shortcut in + // DenseConjunctionBulkScorer#scoreWindow no longer applies, so MatchAllDocsQuery, + // FieldExistsQuery and point ranges go window-by-window and hit the pruning check. + // A single deleted document is enough. + // + // These two tests use a plain IndexWriter rather than RandomIndexWriter: the latter's + // getReader() calls doRandomForceMerge(), which may expunge the deletion and put us back on + // the collectRange() shortcut, so the deletions coverage would silently disappear. The + // hasDeletions() assertions below make that failure loud if it ever comes back. + // --------------------------------------------------------------------------------------------- + + public void testConstantScoreQueriesWithDeletionsUnderComplete() throws Exception { + try (Directory dir = newDirectory(); + IndexWriter w = new IndexWriter(dir, newIndexWriterConfig())) { + for (int i = 0; i < NUM_DOCS; ++i) { + Document doc = new Document(); + doc.add(new StringField("a", "x", Store.NO)); + doc.add(new StringField("b", "y", Store.NO)); + doc.add(new NumericDocValuesField("n", i)); + doc.add(new IntPoint("p", i)); + if (i == 7) { + doc.add(new StringField("del", "yes", Store.NO)); + } + w.addDocument(doc); + } + w.forceMerge(1); + w.deleteDocuments(new Term("del", "yes")); + try (DirectoryReader reader = DirectoryReader.open(w)) { + assertTrue("the deletion must survive: it is what this test covers", reader.hasDeletions()); + IndexSearcher searcher = plainSearcher(reader); + final int live = NUM_DOCS - 1; + assertEquals(live, reader.numDocs()); + Query[] queries = { + new MatchAllDocsQuery(), + new FieldExistsQuery("n"), + IntPoint.newRangeQuery("p", 0, NUM_DOCS), + new BooleanQuery.Builder() + .add(new TermQuery(new Term("a", "x")), BooleanClause.Occur.FILTER) + .add(new TermQuery(new Term("b", "y")), BooleanClause.Occur.FILTER) + .build(), + }; + for (Query q : queries) { + assertEquals( + "COMPLETE pruned matches with deletions for " + q, + live, + runCollect(searcher, q, ScoreMode.COMPLETE)); + assertEquals( + "COMPLETE_NO_SCORES pruned matches with deletions for " + q, + live, + runCollect(searcher, q, ScoreMode.COMPLETE_NO_SCORES)); + } + } + } + } + + /** Deletions + TOP_SCORES: pruning must still happen, so the fix costs nothing. */ + public void testMatchAllWithDeletionsStillPrunesUnderTopScores() throws Exception { + try (Directory dir = newDirectory(); + IndexWriter w = new IndexWriter(dir, newIndexWriterConfig())) { + for (int i = 0; i < NUM_DOCS; ++i) { + Document doc = new Document(); + doc.add(new StringField("a", "x", Store.NO)); + if (i == 7) { + doc.add(new StringField("del", "yes", Store.NO)); + } + w.addDocument(doc); + } + w.forceMerge(1); + w.deleteDocuments(new Term("del", "yes")); + try (DirectoryReader reader = DirectoryReader.open(w)) { + assertTrue("the deletion must survive: it is what this test covers", reader.hasDeletions()); + IndexSearcher searcher = plainSearcher(reader); + int collected = runCollect(searcher, new MatchAllDocsQuery(), ScoreMode.TOP_SCORES); + assertTrue( + "TOP_SCORES should still prune, collected=" + collected, collected < NUM_DOCS - 1); + } + } + } +} diff --git a/lucene/core/src/test/org/apache/lucene/search/TestMultiCollector.java b/lucene/core/src/test/org/apache/lucene/search/TestMultiCollector.java index 8abc94aab407..c2be8e40fe01 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestMultiCollector.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestMultiCollector.java @@ -25,11 +25,15 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import org.apache.lucene.document.Document; +import org.apache.lucene.document.Field.Store; +import org.apache.lucene.document.StringField; import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.Term; import org.apache.lucene.store.Directory; import org.apache.lucene.tests.index.RandomIndexWriter; import org.apache.lucene.tests.search.DummyTotalHitCountCollector; @@ -38,6 +42,65 @@ public class TestMultiCollector extends LuceneTestCase { + /** + * GITHUB#15239: wrapping TopScoreDocCollector in a COMPLETE SimpleCollector must still visit + * every match, even if the inner collector calls setMinCompetitiveScore. + */ + public void testSimpleCollectorWrappingTopScoreDocCollector() throws Exception { + try (Directory dir = newDirectory(); + RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + // The Document is deliberately reused and never cleared: document i ends up carrying + // bar0..bari, so "bar1" matches 999 documents, each with a different norm and therefore a + // different score. That is what makes the inner TopScoreDocCollector raise a competitive + // threshold part-way through collection. + final Document doc = new Document(); + for (int i = 0; i < 1000; ++i) { + doc.add(new StringField("foo", "bar" + i, Store.NO)); + w.addDocument(doc); + } + try (IndexReader reader = w.getReader()) { + // wrapWithAssertions must stay false: the collector below deliberately violates the + // Scorable#setMinCompetitiveScore contract (that is the whole point of the test), and + // AssertingScorer asserts that only TOP_SCORES may call it. Reader wrapping and + // intra-segment concurrency are still exercised. + final IndexSearcher searcher = newSearcher(reader, true, false, true); + final TopScoreDocCollector in = new TopScoreDocCollectorManager(1, 1).newCollector(); + final AtomicInteger totalCalls = new AtomicInteger(); + final Collector out = + new SimpleCollector() { + protected LeafCollector leafIn; + + @Override + protected void doSetNextReader(LeafReaderContext context) throws IOException { + leafIn = in.getLeafCollector(context); + } + + @Override + public void collect(int collectDoc) throws IOException { + int soFar = totalCalls.incrementAndGet(); + if (soFar > 1) { + leafIn.collect(collectDoc); + } + } + + @Override + public void setScorer(Scorable scorer) throws IOException { + super.setScorer(scorer); + leafIn.setScorer(scorer); + } + + @Override + public ScoreMode scoreMode() { + return ScoreMode.COMPLETE; + } + }; + searcher.search(new TermQuery(new Term("foo", "bar1")), out); + assertEquals("not enough collect calls", 999, totalCalls.intValue()); + assertEquals("not enough hits reported by inner collector", 998, in.getTotalHits()); + } + } + } + private static class TerminateAfterCollector extends FilterCollector { private int count = 0;