diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index c51f8fa4eb6..946706ec390 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -35,10 +35,16 @@ Other ======================= Lucene 8.2.0 ======================= -Bug Fixes: +Bug Fixes + * LUCENE-8785: Ensure new threadstates are locked before retrieving the number of active threadstates. This causes assertion errors and potentially broken field attributes in the IndexWriter when - IndexWriter#deleteAll is called while actively indexing. (Simon Willnauer) + IndexWriter#deleteAll is called while actively indexing. (Simon Willnauer) + +Improvements + +* LUCENE-7840: Non-scoring BooleanQuery now removes SHOULD clauses before building the scorer supplier + as opposed to eliminating them during scoring construction. (Atri Sharma via Jim Ferenczi) ======================= Lucene 8.1.0 ======================= diff --git a/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java b/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java index ff264e78095..80924a9dd29 100644 --- a/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java +++ b/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java @@ -192,18 +192,33 @@ public class BooleanQuery extends Query implements Iterable { } private BooleanQuery rewriteNoScoring() { - if (clauseSets.get(Occur.MUST).size() == 0) { + boolean keepShould = getMinimumNumberShouldMatch() > 0 + || (clauseSets.get(Occur.MUST).size() + clauseSets.get(Occur.FILTER).size() == 0); + + if (clauseSets.get(Occur.MUST).size() == 0 && keepShould) { return this; } BooleanQuery.Builder newQuery = new BooleanQuery.Builder(); + newQuery.setMinimumNumberShouldMatch(getMinimumNumberShouldMatch()); for (BooleanClause clause : clauses) { - if (clause.getOccur() == Occur.MUST) { - newQuery.add(clause.getQuery(), Occur.FILTER); - } else { - newQuery.add(clause); + switch (clause.getOccur()) { + case MUST: { + newQuery.add(clause.getQuery(), Occur.FILTER); + break; + } + case SHOULD: { + if (keepShould) { + newQuery.add(clause); + } + break; + } + default: { + newQuery.add(clause); + } } } + return newQuery.build(); } diff --git a/lucene/core/src/java/org/apache/lucene/search/BooleanWeight.java b/lucene/core/src/java/org/apache/lucene/search/BooleanWeight.java index cded651629a..ae6ccbe0cb5 100644 --- a/lucene/core/src/java/org/apache/lucene/search/BooleanWeight.java +++ b/lucene/core/src/java/org/apache/lucene/search/BooleanWeight.java @@ -387,11 +387,6 @@ final class BooleanWeight extends Weight { return null; } - // we don't need scores, so if we have required clauses, drop optional clauses completely - if (scoreMode.needsScores() == false && minShouldMatch == 0 && scorers.get(Occur.MUST).size() + scorers.get(Occur.FILTER).size() > 0) { - scorers.get(Occur.SHOULD).clear(); - } - return new Boolean2ScorerSupplier(this, scorers, scoreMode, minShouldMatch); } diff --git a/lucene/core/src/test/org/apache/lucene/search/TestBooleanRewrites.java b/lucene/core/src/test/org/apache/lucene/search/TestBooleanRewrites.java index 2120c516a25..497035b83f8 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestBooleanRewrites.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestBooleanRewrites.java @@ -544,4 +544,58 @@ public class TestBooleanRewrites extends LuceneTestCase { .build(); assertSame(query, searcher.rewrite(query)); } + + public void testDiscardShouldClauses() throws IOException { + Directory dir = newDirectory(); + RandomIndexWriter w = new RandomIndexWriter(random(), dir); + Document doc = new Document(); + Field f = newTextField("field", "a", Field.Store.NO); + doc.add(f); + w.addDocument(doc); + w.commit(); + + DirectoryReader reader = w.getReader(); + final IndexSearcher searcher = new IndexSearcher(reader); + + BooleanQuery.Builder query1 = new BooleanQuery.Builder(); + query1.add(new TermQuery(new Term("field", "a")), Occur.MUST); + query1.add(new TermQuery(new Term("field", "b")), Occur.SHOULD); + + query1.setMinimumNumberShouldMatch(0); + + Weight weight = searcher.createWeight(searcher.rewrite(query1.build()), ScoreMode.COMPLETE_NO_SCORES, 1); + + Query rewrittenQuery1 = weight.getQuery(); + + assertTrue(rewrittenQuery1 instanceof BooleanQuery); + + BooleanQuery booleanRewrittenQuery1 = (BooleanQuery) rewrittenQuery1; + + for (BooleanClause clause : booleanRewrittenQuery1.clauses()) { + assertNotEquals(clause.getOccur(), Occur.SHOULD); + } + + BooleanQuery.Builder query2 = new BooleanQuery.Builder(); + query2.add(new TermQuery(new Term("field", "a")), Occur.MUST); + query2.add(new TermQuery(new Term("field", "b")), Occur.SHOULD); + query2.add(new TermQuery(new Term("field", "c")), Occur.FILTER); + + query2.setMinimumNumberShouldMatch(0); + + weight = searcher.createWeight(searcher.rewrite(query2.build()), ScoreMode.COMPLETE_NO_SCORES, 1); + + Query rewrittenQuery2 = weight.getQuery(); + + assertTrue(rewrittenQuery2 instanceof BooleanQuery); + + BooleanQuery booleanRewrittenQuery2 = (BooleanQuery) rewrittenQuery1; + + for (BooleanClause clause : booleanRewrittenQuery2.clauses()) { + assertNotEquals(clause.getOccur(), Occur.SHOULD); + } + + reader.close(); + w.close(); + dir.close(); + } } diff --git a/lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java b/lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java index 6cc4fb9856b..9769714f678 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java @@ -826,18 +826,15 @@ public class TestLRUQueryCache extends LuceneTestCase { searcher.setQueryCachingPolicy(ALWAYS_CACHE); BooleanQuery.Builder bq = new BooleanQuery.Builder(); - TermQuery should = new TermQuery(new Term("foo", "baz")); TermQuery must = new TermQuery(new Term("foo", "bar")); TermQuery filter = new TermQuery(new Term("foo", "quux")); TermQuery mustNot = new TermQuery(new Term("foo", "foo")); - bq.add(should, Occur.SHOULD); bq.add(must, Occur.MUST); bq.add(filter, Occur.FILTER); bq.add(mustNot, Occur.MUST_NOT); // same bq but with FILTER instead of MUST BooleanQuery.Builder bq2 = new BooleanQuery.Builder(); - bq2.add(should, Occur.SHOULD); bq2.add(must, Occur.FILTER); bq2.add(filter, Occur.FILTER); bq2.add(mustNot, Occur.MUST_NOT); @@ -849,7 +846,7 @@ public class TestLRUQueryCache extends LuceneTestCase { queryCache.clear(); assertEquals(Collections.emptySet(), new HashSet<>(queryCache.cachedQueries())); searcher.search(new ConstantScoreQuery(bq.build()), 1); - assertEquals(new HashSet<>(Arrays.asList(bq2.build(), should, must, filter, mustNot)), new HashSet<>(queryCache.cachedQueries())); + assertEquals(new HashSet<>(Arrays.asList(bq2.build(), must, filter, mustNot)), new HashSet<>(queryCache.cachedQueries())); reader.close(); dir.close(); diff --git a/lucene/core/src/test/org/apache/lucene/search/TestMatchesIterator.java b/lucene/core/src/test/org/apache/lucene/search/TestMatchesIterator.java index c549c165264..45fda8cddae 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestMatchesIterator.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestMatchesIterator.java @@ -113,7 +113,7 @@ public class TestMatchesIterator extends LuceneTestCase { }; private void checkMatches(Query q, String field, int[][] expected) throws IOException { - Weight w = searcher.createWeight(searcher.rewrite(q), ScoreMode.COMPLETE_NO_SCORES, 1); + Weight w = searcher.createWeight(searcher.rewrite(q), ScoreMode.COMPLETE, 1); for (int i = 0; i < expected.length; i++) { LeafReaderContext ctx = searcher.leafContexts.get(ReaderUtil.subIndex(expected[i][0], searcher.leafContexts)); int doc = expected[i][0] - ctx.docBase; @@ -133,7 +133,7 @@ public class TestMatchesIterator extends LuceneTestCase { } private void checkLabelCount(Query q, String field, int[] expected) throws IOException { - Weight w = searcher.createWeight(searcher.rewrite(q), ScoreMode.COMPLETE_NO_SCORES, 1); + Weight w = searcher.createWeight(searcher.rewrite(q), ScoreMode.COMPLETE, 1); for (int i = 0; i < expected.length; i++) { LeafReaderContext ctx = searcher.leafContexts.get(ReaderUtil.subIndex(i, searcher.leafContexts)); int doc = i - ctx.docBase; @@ -172,7 +172,7 @@ public class TestMatchesIterator extends LuceneTestCase { } private void checkNoPositionsMatches(Query q, String field, boolean[] expected) throws IOException { - Weight w = searcher.createWeight(searcher.rewrite(q), ScoreMode.COMPLETE_NO_SCORES, 1); + Weight w = searcher.createWeight(searcher.rewrite(q), ScoreMode.COMPLETE, 1); for (int i = 0; i < expected.length; i++) { LeafReaderContext ctx = searcher.leafContexts.get(ReaderUtil.subIndex(i, searcher.leafContexts)); int doc = i - ctx.docBase; @@ -188,7 +188,7 @@ public class TestMatchesIterator extends LuceneTestCase { } private void assertIsLeafMatch(Query q, String field) throws IOException { - Weight w = searcher.createWeight(searcher.rewrite(q), ScoreMode.COMPLETE_NO_SCORES, 1); + Weight w = searcher.createWeight(searcher.rewrite(q), ScoreMode.COMPLETE, 1); for (int i = 0; i < searcher.reader.maxDoc(); i++) { LeafReaderContext ctx = searcher.leafContexts.get(ReaderUtil.subIndex(i, searcher.leafContexts)); int doc = i - ctx.docBase; @@ -207,7 +207,7 @@ public class TestMatchesIterator extends LuceneTestCase { } private void checkTermMatches(Query q, String field, TermMatch[][][] expected) throws IOException { - Weight w = searcher.createWeight(searcher.rewrite(q), ScoreMode.COMPLETE_NO_SCORES, 1); + Weight w = searcher.createWeight(searcher.rewrite(q), ScoreMode.COMPLETE, 1); for (int i = 0; i < expected.length; i++) { LeafReaderContext ctx = searcher.leafContexts.get(ReaderUtil.subIndex(i, searcher.leafContexts)); int doc = i - ctx.docBase;