Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lucene/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,8 @@ Optimizations
* GITHUB#15732: Prevent writing vectors twice during merging HNSW graphs by allowing doing deferred work after calling
merge for vectors is finshed. (Ignacio Vera)

* GITHUB#15998: Skip scorer construction for zero-cost SHOULD clauses in BooleanScorerSupplier. (Sagar Upadhyaya)

* GITHUB#16001: IndexSearcher.count() was calling query.rewrite twice, a regression since v9.10 (David Smiley)

Bug Fixes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,17 +295,35 @@ BulkScorer optionalBulkScorer() throws IOException {
if (scoreMode == ScoreMode.TOP_SCORES && minShouldMatch <= 1) {
List<Scorer> optionalScorers = new ArrayList<>();
for (ScorerSupplier ss : subs.get(Occur.SHOULD)) {
if (ss.cost() == 0) {
continue;
}
optionalScorers.add(ss.get(Long.MAX_VALUE));
}
if (optionalScorers.isEmpty()) {
return null;
}

return new MaxScoreBulkScorer(maxDoc, optionalScorers, null);
}

long shouldCost = computeShouldCost();
List<Scorer> optional = new ArrayList<>();
for (ScorerSupplier ss : subs.get(Occur.SHOULD)) {
if (minShouldMatch <= 1 && ss.cost() == 0) {
continue;
}
optional.add(ss.get(shouldCost));
}
if (optional.isEmpty()) {
return null;
}
// BooleanScorer requires at least 2 sub-scorers.
// If only 1 remains after filtering zero-cost clauses, use it directly.
if (optional.size() == 1) {
Scorer scorer = optional.get(0);
return new DefaultBulkScorer(scorer);
}

return new BooleanScorer(optional, Math.max(1, minShouldMatch), scoreMode.needsScores());
}
Expand All @@ -319,8 +337,23 @@ BulkScorer filteredOptionalBulkScorer() throws IOException {
return null;
}
long cost = cost();
// Count non-zero-cost suppliers first to avoid calling get() on scorers we won't use.
// Calling get() consumes internal state (e.g. DocIdSetBuilder) that cannot be reused
// if we later fall back to the scorer-based path.
int nonZeroCostCount = 0;
for (ScorerSupplier ss : subs.get(Occur.SHOULD)) {
if (ss.cost() > 0) {
nonZeroCostCount++;
}
}
if (nonZeroCostCount < 2) {
return null;
}
List<Scorer> optionalScorers = new ArrayList<>();
for (ScorerSupplier ss : subs.get(Occur.SHOULD)) {
if (ss.cost() == 0) {
continue;
}
optionalScorers.add(ss.get(cost));
}
List<Scorer> filters = new ArrayList<>();
Expand Down Expand Up @@ -540,9 +573,24 @@ private Scorer opt(
} else {
final List<Scorer> optionalScorers = new ArrayList<>();
for (ScorerSupplier scorer : optional) {
// Skip zero-cost clauses: they match nothing on this segment.
// Safe when minShouldMatch <= 1 because any single matching clause suffices.
if (minShouldMatch <= 1 && scorer.cost() == 0) {
continue;
}
optionalScorers.add(scorer.get(leadCost));
}

// All clauses were zero-cost on this segment
if (optionalScorers.isEmpty()) {
return new ConstantScoreScorer(0f, scoreMode, DocIdSetIterator.empty());
}

// After filtering, only one clause remains
if (optionalScorers.size() == 1) {
return optionalScorers.get(0);
}

// Technically speaking, WANDScorer should be able to handle the following 3 conditions now
// 1. Any ScoreMode (with scoring or not)
// 2. Any minCompetitiveScore ( >= 0 )
Expand Down
102 changes: 102 additions & 0 deletions lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java
Original file line number Diff line number Diff line change
Expand Up @@ -1411,4 +1411,106 @@ public void testClauseSetsImmutability() throws Exception {
UnsupportedOperationException.class,
() -> bq.clauses().add(new BooleanClause(MatchNoDocsQuery.INSTANCE, Occur.SHOULD)));
}

// Tests for skipping zero-cost SHOULD clauses in BooleanScorerSupplier

public void testAllZeroCostShouldClauses() throws IOException {
// All SHOULD clauses match nothing — should return 0 results
try (Directory dir = newDirectory();
RandomIndexWriter w = new RandomIndexWriter(random(), dir)) {
Document doc = new Document();
doc.add(new StringField("field", "value", Store.NO));
w.addDocument(doc);

try (IndexReader reader = w.getReader()) {
IndexSearcher searcher = newSearcher(reader);
// "nonexistent1" and "nonexistent2" don't exist — zero cost
BooleanQuery query =
new BooleanQuery.Builder()
.add(new TermQuery(new Term("field", "nonexistent1")), Occur.SHOULD)
.add(new TermQuery(new Term("field", "nonexistent2")), Occur.SHOULD)
.add(new TermQuery(new Term("field", "nonexistent3")), Occur.SHOULD)
.build();
assertEquals(0, searcher.count(query));
}
}
}

public void testMixedZeroCostAndLiveShouldClauses() throws IOException {
// Some SHOULD clauses match, some don't — only live ones should contribute
try (Directory dir = newDirectory();
RandomIndexWriter w = new RandomIndexWriter(random(), dir)) {
Document doc = new Document();
doc.add(new StringField("field", "value", Store.NO));
w.addDocument(doc);

try (IndexReader reader = w.getReader()) {
IndexSearcher searcher = newSearcher(reader);
BooleanQuery query =
new BooleanQuery.Builder()
.add(new TermQuery(new Term("field", "nonexistent1")), Occur.SHOULD)
.add(new TermQuery(new Term("field", "value")), Occur.SHOULD) // matches
.add(new TermQuery(new Term("field", "nonexistent2")), Occur.SHOULD)
.build();
assertEquals(1, searcher.count(query));
}
}
}

public void testZeroCostShouldWithMustClause() throws IOException {
// MUST clause matches, all SHOULD clauses are zero-cost
// The MUST should still work; zero-cost SHOULDs just don't contribute to scoring
try (Directory dir = newDirectory();
RandomIndexWriter w = new RandomIndexWriter(random(), dir)) {
Document doc = new Document();
doc.add(new StringField("field", "value", Store.NO));
w.addDocument(doc);

try (IndexReader reader = w.getReader()) {
IndexSearcher searcher = newSearcher(reader);
BooleanQuery query =
new BooleanQuery.Builder()
.add(new TermQuery(new Term("field", "value")), Occur.MUST)
.add(new TermQuery(new Term("field", "nonexistent1")), Occur.SHOULD)
.add(new TermQuery(new Term("field", "nonexistent2")), Occur.SHOULD)
.build();
assertEquals(1, searcher.count(query));
}
}
}

public void testZeroCostShouldWithMinShouldMatch() throws IOException {
// With minShouldMatch=2, zero-cost clauses should NOT be skipped
// because we need to know the true count of matching clauses
try (Directory dir = newDirectory();
RandomIndexWriter w = new RandomIndexWriter(random(), dir)) {
Document doc = new Document();
doc.add(new StringField("field", "a", Store.NO));
doc.add(new StringField("field", "b", Store.NO));
w.addDocument(doc);

try (IndexReader reader = w.getReader()) {
IndexSearcher searcher = newSearcher(reader);
// 2 live clauses + 1 zero-cost, minShouldMatch=2 — should still match
BooleanQuery query =
new BooleanQuery.Builder()
.add(new TermQuery(new Term("field", "a")), Occur.SHOULD)
.add(new TermQuery(new Term("field", "b")), Occur.SHOULD)
.add(new TermQuery(new Term("field", "nonexistent")), Occur.SHOULD)
.setMinimumNumberShouldMatch(2)
.build();
assertEquals(1, searcher.count(query));

// 1 live clause + 2 zero-cost, minShouldMatch=2 — should NOT match
BooleanQuery query2 =
new BooleanQuery.Builder()
.add(new TermQuery(new Term("field", "a")), Occur.SHOULD)
.add(new TermQuery(new Term("field", "nonexistent1")), Occur.SHOULD)
.add(new TermQuery(new Term("field", "nonexistent2")), Occur.SHOULD)
.setMinimumNumberShouldMatch(2)
.build();
assertEquals(0, searcher.count(query2));
}
}
}
}
Loading