From 69d3a1d6af8e16f5eceac5eb90d1cb99e17ab7af Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Wed, 9 Feb 2022 19:02:54 +0100 Subject: [PATCH] LUCENE-10412: Improve handling of MatchNoDocsQuery in rewrites. (#664) --- lucene/CHANGES.txt | 3 ++ .../apache/lucene/search/BooleanQuery.java | 16 +++++- .../org/apache/lucene/search/BoostQuery.java | 5 ++ .../lucene/search/ConstantScoreQuery.java | 5 ++ .../lucene/search/TestBooleanRewrites.java | 50 +++++++++++++++++++ .../apache/lucene/search/TestBoostQuery.java | 11 +++- .../lucene/search/TestConstantScoreQuery.java | 8 +++ .../lucene/search/TestMatchNoDocsQuery.java | 2 +- 8 files changed, 95 insertions(+), 5 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 73909964b22..8e4b743c047 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -204,6 +204,9 @@ Optimizations * LUCENE-10367: Optimize CoveringQuery for the case when the minimum number of matching clauses is a constant. (LuYunCheng via Adrien Grand) +* LUCENE-10412: More `Query#rewrite` optimizations for MatchNoDocsQuery. + (Adrien Grand) + Changes in runtime behavior --------------------- 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 9d095e50c1c..1b066d7ad5e 100644 --- a/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java +++ b/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java @@ -275,10 +275,22 @@ public class BooleanQuery extends Query implements Iterable { for (BooleanClause clause : this) { Query query = clause.getQuery(); Query rewritten = query.rewrite(reader); - if (rewritten != query) { + if (rewritten != query || query.getClass() == MatchNoDocsQuery.class) { // rewrite clause actuallyRewritten = true; - builder.add(rewritten, clause.getOccur()); + if (rewritten.getClass() == MatchNoDocsQuery.class) { + switch (clause.getOccur()) { + case SHOULD: + case MUST_NOT: + // the clause can be safely ignored + break; + case MUST: + case FILTER: + return rewritten; + } + } else { + builder.add(rewritten, clause.getOccur()); + } } else { // leave as-is builder.add(clause); diff --git a/lucene/core/src/java/org/apache/lucene/search/BoostQuery.java b/lucene/core/src/java/org/apache/lucene/search/BoostQuery.java index 172ea0051e7..47c0f5f6f28 100644 --- a/lucene/core/src/java/org/apache/lucene/search/BoostQuery.java +++ b/lucene/core/src/java/org/apache/lucene/search/BoostQuery.java @@ -85,6 +85,11 @@ public final class BoostQuery extends Query { return new BoostQuery(in.query, boost * in.boost); } + if (rewritten.getClass() == MatchNoDocsQuery.class) { + // bubble up MatchNoDocsQuery + return rewritten; + } + if (boost == 0f && rewritten.getClass() != ConstantScoreQuery.class) { // so that we pass needScores=false return new BoostQuery(new ConstantScoreQuery(rewritten), 0f); diff --git a/lucene/core/src/java/org/apache/lucene/search/ConstantScoreQuery.java b/lucene/core/src/java/org/apache/lucene/search/ConstantScoreQuery.java index 4be41494092..fec12c6a910 100644 --- a/lucene/core/src/java/org/apache/lucene/search/ConstantScoreQuery.java +++ b/lucene/core/src/java/org/apache/lucene/search/ConstantScoreQuery.java @@ -43,6 +43,11 @@ public final class ConstantScoreQuery extends Query { public Query rewrite(IndexReader reader) throws IOException { Query rewritten = query.rewrite(reader); + if (rewritten.getClass() == MatchNoDocsQuery.class) { + // bubble up MatchNoDocsQuery + return rewritten; + } + if (rewritten != query) { return new ConstantScoreQuery(rewritten); } 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 c282f511dcb..93da1ba69c6 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestBooleanRewrites.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestBooleanRewrites.java @@ -663,4 +663,54 @@ public class TestBooleanRewrites extends LuceneTestCase { w.close(); dir.close(); } + + public void testShouldMatchNoDocsQuery() throws IOException { + IndexSearcher searcher = newSearcher(new MultiReader()); + + BooleanQuery query = + new BooleanQuery.Builder() + .add(new TermQuery(new Term("foo", "bar")), Occur.SHOULD) + .add(new MatchNoDocsQuery(), Occur.SHOULD) + .build(); + assertEquals(new TermQuery(new Term("foo", "bar")), searcher.rewrite(query)); + } + + public void testMustNotMatchNoDocsQuery() throws IOException { + IndexSearcher searcher = newSearcher(new MultiReader()); + + BooleanQuery query = + new BooleanQuery.Builder() + .add(new TermQuery(new Term("foo", "bar")), Occur.SHOULD) + .add(new MatchNoDocsQuery(), Occur.MUST_NOT) + .build(); + assertEquals(new TermQuery(new Term("foo", "bar")), searcher.rewrite(query)); + } + + public void testMustMatchNoDocsQuery() throws IOException { + IndexSearcher searcher = newSearcher(new MultiReader()); + + BooleanQuery query = + new BooleanQuery.Builder() + .add(new TermQuery(new Term("foo", "bar")), Occur.MUST) + .add(new MatchNoDocsQuery(), Occur.MUST) + .build(); + assertEquals(new MatchNoDocsQuery(), searcher.rewrite(query)); + } + + public void testFilterMatchNoDocsQuery() throws IOException { + IndexSearcher searcher = newSearcher(new MultiReader()); + + BooleanQuery query = + new BooleanQuery.Builder() + .add(new TermQuery(new Term("foo", "bar")), Occur.MUST) + .add(new MatchNoDocsQuery(), Occur.FILTER) + .build(); + assertEquals(new MatchNoDocsQuery(), searcher.rewrite(query)); + } + + public void testEmptyBoolean() throws IOException { + IndexSearcher searcher = newSearcher(new MultiReader()); + BooleanQuery query = new BooleanQuery.Builder().build(); + assertEquals(new MatchNoDocsQuery(), searcher.rewrite(query)); + } } diff --git a/lucene/core/src/test/org/apache/lucene/search/TestBoostQuery.java b/lucene/core/src/test/org/apache/lucene/search/TestBoostQuery.java index 6da06785ebf..79e3a58939a 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestBoostQuery.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestBoostQuery.java @@ -73,8 +73,8 @@ public class TestBoostQuery extends LuceneTestCase { IndexSearcher searcher = new IndexSearcher(new MultiReader()); // inner queries are rewritten - Query q = new BoostQuery(new BooleanQuery.Builder().build(), 2); - assertEquals(new BoostQuery(new MatchNoDocsQuery(), 2), searcher.rewrite(q)); + Query q = new BoostQuery(new PhraseQuery("foo", "bar"), 2); + assertEquals(new BoostQuery(new TermQuery(new Term("foo", "bar")), 2), searcher.rewrite(q)); // boosts are merged q = new BoostQuery(new BoostQuery(new MatchAllDocsQuery(), 3), 2); @@ -85,4 +85,11 @@ public class TestBoostQuery extends LuceneTestCase { assertEquals( new BoostQuery(new ConstantScoreQuery(new MatchAllDocsQuery()), 0), searcher.rewrite(q)); } + + public void testRewriteBubblesUpMatchNoDocsQuery() throws IOException { + IndexSearcher searcher = newSearcher(new MultiReader()); + + Query query = new BoostQuery(new MatchNoDocsQuery(), 2f); + assertEquals(new MatchNoDocsQuery(), searcher.rewrite(query)); + } } diff --git a/lucene/core/src/test/org/apache/lucene/search/TestConstantScoreQuery.java b/lucene/core/src/test/org/apache/lucene/search/TestConstantScoreQuery.java index 703cc81ce8a..b553debc0f9 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestConstantScoreQuery.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestConstantScoreQuery.java @@ -23,6 +23,7 @@ import org.apache.lucene.document.Document; import org.apache.lucene.document.Field; import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.MultiReader; import org.apache.lucene.index.Term; import org.apache.lucene.search.BooleanClause.Occur; import org.apache.lucene.store.Directory; @@ -254,4 +255,11 @@ public class TestConstantScoreQuery extends LuceneTestCase { w.close(); dir.close(); } + + public void testRewriteBubblesUpMatchNoDocsQuery() throws IOException { + IndexSearcher searcher = newSearcher(new MultiReader()); + + Query query = new ConstantScoreQuery(new MatchNoDocsQuery()); + assertEquals(new MatchNoDocsQuery(), searcher.rewrite(query)); + } } diff --git a/lucene/core/src/test/org/apache/lucene/search/TestMatchNoDocsQuery.java b/lucene/core/src/test/org/apache/lucene/search/TestMatchNoDocsQuery.java index d8e558b450e..e1facc81e61 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestMatchNoDocsQuery.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestMatchNoDocsQuery.java @@ -89,7 +89,7 @@ public class TestMatchNoDocsQuery extends LuceneTestCase { hits = searcher.search(query, 1000).scoreDocs; Query rewrite = query.rewrite(ir); assertEquals(1, hits.length); - assertEquals(rewrite.toString(), "key:one MatchNoDocsQuery(\"field not found\")"); + assertEquals(rewrite.toString(), "key:one"); iw.close(); ir.close();