You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ji...@apache.org on 2019/05/09 07:39:29 UTC
[lucene-solr] branch master updated: LUCENE-7840: Avoid Building
Scorer Supplier For Redundant SHOULD Clauses
This is an automated email from the ASF dual-hosted git repository.
jimczi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/master by this push:
new c988b04 LUCENE-7840: Avoid Building Scorer Supplier For Redundant SHOULD Clauses
c988b04 is described below
commit c988b04b1888d5091a9455d3ae34762eb0ca0cea
Author: Atri Sharma <at...@apache.org>
AuthorDate: Mon May 6 13:49:47 2019 +0530
LUCENE-7840: Avoid Building Scorer Supplier For Redundant SHOULD Clauses
For boolean queries, we should eliminate redundant SHOULD clauses during
query rewrite and not build the scorer supplier, as opposed to
eliminating them during weight construction
Signed-off-by: jimczi <ji...@apache.org>
---
lucene/CHANGES.txt | 10 +++-
.../org/apache/lucene/search/BooleanQuery.java | 25 ++++++++--
.../org/apache/lucene/search/BooleanWeight.java | 5 --
.../apache/lucene/search/TestBooleanRewrites.java | 54 ++++++++++++++++++++++
.../apache/lucene/search/TestLRUQueryCache.java | 5 +-
.../apache/lucene/search/TestMatchesIterator.java | 10 ++--
6 files changed, 88 insertions(+), 21 deletions(-)
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index c51f8fa..946706e 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 ff264e7..80924a9 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<BooleanClause> {
}
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 cded651..ae6ccbe 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 2120c51..497035b 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 6cc4fb9..9769714 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 c549c16..45fda8c 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;