From e130bf244338b0d3337066c2d03f67723efd4c80 Mon Sep 17 00:00:00 2001 From: Sagar Upadhyaya Date: Wed, 29 Apr 2026 12:07:13 -0700 Subject: [PATCH 1/4] Skipping scorer construction for clauses with zero cost --- .../lucene/search/BooleanScorerSupplier.java | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) 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 480a524de544..696ee3a043e8 100644 --- a/lucene/core/src/java/org/apache/lucene/search/BooleanScorerSupplier.java +++ b/lucene/core/src/java/org/apache/lucene/search/BooleanScorerSupplier.java @@ -295,8 +295,14 @@ BulkScorer optionalBulkScorer() throws IOException { if (scoreMode == ScoreMode.TOP_SCORES && minShouldMatch <= 1) { List 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); } @@ -304,8 +310,18 @@ BulkScorer optionalBulkScorer() throws IOException { long shouldCost = computeShouldCost(); List 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 (optional.size() == 1) { + return null; + } return new BooleanScorer(optional, Math.max(1, minShouldMatch), scoreMode.needsScores()); } @@ -321,8 +337,20 @@ BulkScorer filteredOptionalBulkScorer() throws IOException { long cost = cost(); List optionalScorers = new ArrayList<>(); for (ScorerSupplier ss : subs.get(Occur.SHOULD)) { + if (ss.cost() == 0) { + continue; + } optionalScorers.add(ss.get(cost)); } + if (optionalScorers.isEmpty()) { + return null; + } + // After filtering zero-cost clauses, if only 1 scorer remains, + // fall back to null so the caller uses a different code path. + // DisjunctionSumScorer requires at least 2 sub-scorers. + if (optionalScorers.size() <= 1) { + return null; + } List filters = new ArrayList<>(); for (ScorerSupplier ss : subs.get(Occur.FILTER)) { filters.add(ss.get(cost)); @@ -540,9 +568,24 @@ private Scorer opt( } else { final List 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 ) From 7e0be7adb10b1ca7b93d8a7529145d02950b55c5 Mon Sep 17 00:00:00 2001 From: Sagar Upadhyaya Date: Wed, 29 Apr 2026 13:32:49 -0700 Subject: [PATCH 2/4] Fixing failing test, add more UTs --- .../lucene/search/BooleanScorerSupplier.java | 9 +- .../lucene/search/TestBooleanQuery.java | 102 ++++++++++++++++++ 2 files changed, 107 insertions(+), 4 deletions(-) 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 696ee3a043e8..794e17294e12 100644 --- a/lucene/core/src/java/org/apache/lucene/search/BooleanScorerSupplier.java +++ b/lucene/core/src/java/org/apache/lucene/search/BooleanScorerSupplier.java @@ -318,9 +318,11 @@ BulkScorer optionalBulkScorer() throws IOException { if (optional.isEmpty()) { return null; } - // BooleanScorer requires at least 2 sub-scorers + // BooleanScorer requires at least 2 sub-scorers. + // If only 1 remains after filtering zero-cost clauses, use it directly. if (optional.size() == 1) { - return null; + Scorer scorer = optional.get(0); + return new DefaultBulkScorer(scorer); } return new BooleanScorer(optional, Math.max(1, minShouldMatch), scoreMode.needsScores()); @@ -347,8 +349,7 @@ BulkScorer filteredOptionalBulkScorer() throws IOException { } // After filtering zero-cost clauses, if only 1 scorer remains, // fall back to null so the caller uses a different code path. - // DisjunctionSumScorer requires at least 2 sub-scorers. - if (optionalScorers.size() <= 1) { + if (optionalScorers.size() == 1) { return null; } List filters = new ArrayList<>(); diff --git a/lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java b/lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java index b0ed0b89a5f4..ac8d4544e9b6 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java @@ -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)); + } + } + } } From 5f94d7dd4af895a05596e707a71295af2df8ff45 Mon Sep 17 00:00:00 2001 From: Sagar Upadhyaya Date: Wed, 29 Apr 2026 13:37:50 -0700 Subject: [PATCH 3/4] Add changelog entry --- lucene/CHANGES.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index f0bb778d3859..43c67c3ab950 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -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) + Bug Fixes --------------------- * GITHUB#15754: Fix HTMLStripCharFilter to prevent tags from incorrectly consuming subsequent From 51ba6a1ec61f3ee55adf045fc87020f362ec1b48 Mon Sep 17 00:00:00 2001 From: Sagar Upadhyaya Date: Mon, 4 May 2026 15:42:20 -0700 Subject: [PATCH 4/4] Fix unit test --- .../lucene/search/BooleanScorerSupplier.java | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) 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 794e17294e12..d487bcd63f64 100644 --- a/lucene/core/src/java/org/apache/lucene/search/BooleanScorerSupplier.java +++ b/lucene/core/src/java/org/apache/lucene/search/BooleanScorerSupplier.java @@ -337,6 +337,18 @@ 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 optionalScorers = new ArrayList<>(); for (ScorerSupplier ss : subs.get(Occur.SHOULD)) { if (ss.cost() == 0) { @@ -344,14 +356,6 @@ BulkScorer filteredOptionalBulkScorer() throws IOException { } optionalScorers.add(ss.get(cost)); } - if (optionalScorers.isEmpty()) { - return null; - } - // After filtering zero-cost clauses, if only 1 scorer remains, - // fall back to null so the caller uses a different code path. - if (optionalScorers.size() == 1) { - return null; - } List filters = new ArrayList<>(); for (ScorerSupplier ss : subs.get(Occur.FILTER)) { filters.add(ss.get(cost));