From 0d86474d3c59396d27add2263b742e988ad9ddca Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Tue, 13 Dec 2011 16:32:24 +0000 Subject: [PATCH] LUCENE-3643: FilteredQuery and IndexSearcher.search(Query, Filter,...) now optimize the special case "query instanceof MatchAllDocsQuery" to execute as ConstantScoreQuery git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1213771 13f79535-47bb-0310-9956-ffa450edef68 --- lucene/CHANGES.txt | 4 + .../apache/lucene/search/FilteredQuery.java | 63 +++++++++----- .../org/apache/lucene/search/QueryUtils.java | 4 +- .../lucene/search/TestFilteredQuery.java | 83 ++++++++++++++++++- 4 files changed, 125 insertions(+), 29 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index ee5f23aa6be..5c8727fcbea 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -638,6 +638,10 @@ Optimizations boolean clauses are required and instances of TermQuery. (Simon Willnauer, Robert Muir) +* LUCENE-3643: FilteredQuery and IndexSearcher.search(Query, Filter,...) + now optimize the special case query instanceof MatchAllDocsQuery to + execute as ConstantScoreQuery. (Uwe Schindler) + Bug fixes * LUCENE-2803: The FieldCache can miss values if an entry for a reader diff --git a/lucene/src/java/org/apache/lucene/search/FilteredQuery.java b/lucene/src/java/org/apache/lucene/search/FilteredQuery.java index fc3a7a78378..87624d04942 100644 --- a/lucene/src/java/org/apache/lucene/search/FilteredQuery.java +++ b/lucene/src/java/org/apache/lucene/search/FilteredQuery.java @@ -33,25 +33,23 @@ import java.util.Set; *

Note: the bits are retrieved from the filter each time this * query is used in a search - use a CachingWrapperFilter to avoid * regenerating the bits every time. - * - *

Created: Apr 20, 2004 8:58:29 AM - * * @since 1.4 * @see CachingWrapperFilter */ -public class FilteredQuery -extends Query { +public class FilteredQuery extends Query { - Query query; - Filter filter; + private final Query query; + private final Filter filter; /** * Constructs a new query which applies a filter to the results of the original query. - * Filter.getDocIdSet() will be called every time this query is used in a search. + * {@link Filter#getDocIdSet} will be called every time this query is used in a search. * @param query Query to be filtered, cannot be null. * @param filter Filter to apply to query results, cannot be null. */ public FilteredQuery (Query query, Filter filter) { + if (query == null || filter == null) + throw new IllegalArgumentException("Query and filter cannot be null."); this.query = query; this.filter = filter; } @@ -229,31 +227,45 @@ extends Query { }; } - /** Rewrites the wrapped query. */ + /** Rewrites the query. If the wrapped is an instance of + * {@link MatchAllDocsQuery} it returns a {@link ConstantScoreQuery}. Otherwise + * it returns a new {@code FilteredQuery} wrapping the rewritten query. */ @Override public Query rewrite(IndexReader reader) throws IOException { - Query rewritten = query.rewrite(reader); - if (rewritten != query) { - FilteredQuery clone = (FilteredQuery)this.clone(); - clone.query = rewritten; - return clone; + final Query queryRewritten = query.rewrite(reader); + + if (queryRewritten instanceof MatchAllDocsQuery) { + // Special case: If the query is a MatchAllDocsQuery, we only + // return a CSQ(filter). + final Query rewritten = new ConstantScoreQuery(filter); + // Combine boost of MatchAllDocsQuery and the wrapped rewritten query: + rewritten.setBoost(this.getBoost() * queryRewritten.getBoost()); + return rewritten; + } + + if (queryRewritten != query) { + // rewrite to a new FilteredQuery wrapping the rewritten query + final Query rewritten = new FilteredQuery(queryRewritten, filter); + rewritten.setBoost(this.getBoost()); + return rewritten; } else { + // nothing to rewrite, we are done! return this; } } - public Query getQuery() { + public final Query getQuery() { return query; } - public Filter getFilter() { + public final Filter getFilter() { return filter; } // inherit javadoc @Override public void extractTerms(Set terms) { - getQuery().extractTerms(terms); + getQuery().extractTerms(terms); } /** Prints a user-readable version of this query. */ @@ -271,16 +283,21 @@ extends Query { /** Returns true iff o is equal to this. */ @Override public boolean equals(Object o) { - if (o instanceof FilteredQuery) { - FilteredQuery fq = (FilteredQuery) o; - return (query.equals(fq.query) && filter.equals(fq.filter) && getBoost()==fq.getBoost()); - } - return false; + if (o == this) + return true; + if (!super.equals(o)) + return false; + assert o instanceof FilteredQuery; + final FilteredQuery fq = (FilteredQuery) o; + return fq.query.equals(this.query) && fq.filter.equals(this.filter); } /** Returns a hash code value for this object. */ @Override public int hashCode() { - return query.hashCode() ^ filter.hashCode() + Float.floatToRawIntBits(getBoost()); + int hash = super.hashCode(); + hash = hash * 31 + query.hashCode(); + hash = hash * 31 + filter.hashCode(); + return hash; } } diff --git a/lucene/src/test-framework/java/org/apache/lucene/search/QueryUtils.java b/lucene/src/test-framework/java/org/apache/lucene/search/QueryUtils.java index 5c489f29888..187700cd6f9 100644 --- a/lucene/src/test-framework/java/org/apache/lucene/search/QueryUtils.java +++ b/lucene/src/test-framework/java/org/apache/lucene/search/QueryUtils.java @@ -79,8 +79,8 @@ public class QueryUtils { } public static void checkUnequal(Query q1, Query q2) { - Assert.assertTrue(!q1.equals(q2)); - Assert.assertTrue(!q2.equals(q1)); + Assert.assertFalse(q1 + " equal to " + q2, q1.equals(q2)); + Assert.assertFalse(q2 + " equal to " + q1, q2.equals(q1)); // possible this test can fail on a hash collision... if that // happens, please change test to use a different example. diff --git a/lucene/src/test/org/apache/lucene/search/TestFilteredQuery.java b/lucene/src/test/org/apache/lucene/search/TestFilteredQuery.java index a8ef3f3f41d..004a9601346 100644 --- a/lucene/src/test/org/apache/lucene/search/TestFilteredQuery.java +++ b/lucene/src/test/org/apache/lucene/search/TestFilteredQuery.java @@ -132,6 +132,11 @@ public class TestFilteredQuery extends LuceneTestCase { assertEquals (2, hits.length); QueryUtils.check(random, filteredquery,searcher); + filteredquery = new FilteredQueryRA(new MatchAllDocsQuery(), filter, useRandomAccess); + hits = searcher.search (filteredquery, null, 1000).scoreDocs; + assertEquals (2, hits.length); + QueryUtils.check(random, filteredquery,searcher); + filteredquery = new FilteredQueryRA(new TermQuery (new Term ("field", "x")), filter, useRandomAccess); hits = searcher.search (filteredquery, null, 1000).scoreDocs; assertEquals (1, hits.length); @@ -220,9 +225,9 @@ public class TestFilteredQuery extends LuceneTestCase { private void tBooleanMUST(final boolean useRandomAccess) throws Exception { BooleanQuery bq = new BooleanQuery(); - Query query = new FilteredQueryRA(new MatchAllDocsQuery(), new SingleDocTestFilter(0), useRandomAccess); + Query query = new FilteredQueryRA(new TermQuery(new Term("field", "one")), new SingleDocTestFilter(0), useRandomAccess); bq.add(query, BooleanClause.Occur.MUST); - query = new FilteredQueryRA(new MatchAllDocsQuery(), new SingleDocTestFilter(1), useRandomAccess); + query = new FilteredQueryRA(new TermQuery(new Term("field", "one")), new SingleDocTestFilter(1), useRandomAccess); bq.add(query, BooleanClause.Occur.MUST); ScoreDoc[] hits = searcher.search(bq, null, 1000).scoreDocs; assertEquals(0, hits.length); @@ -238,9 +243,9 @@ public class TestFilteredQuery extends LuceneTestCase { private void tBooleanSHOULD(final boolean useRandomAccess) throws Exception { BooleanQuery bq = new BooleanQuery(); - Query query = new FilteredQueryRA(new MatchAllDocsQuery(), new SingleDocTestFilter(0), useRandomAccess); + Query query = new FilteredQueryRA(new TermQuery(new Term("field", "one")), new SingleDocTestFilter(0), useRandomAccess); bq.add(query, BooleanClause.Occur.SHOULD); - query = new FilteredQueryRA(new MatchAllDocsQuery(), new SingleDocTestFilter(1), useRandomAccess); + query = new FilteredQueryRA(new TermQuery(new Term("field", "one")), new SingleDocTestFilter(1), useRandomAccess); bq.add(query, BooleanClause.Occur.SHOULD); ScoreDoc[] hits = searcher.search(bq, null, 1000).scoreDocs; assertEquals(2, hits.length); @@ -288,6 +293,76 @@ public class TestFilteredQuery extends LuceneTestCase { assertEquals(1, hits.length); QueryUtils.check(random, query, searcher); } + + public void testEqualsHashcode() throws Exception { + // some tests before, if the used queries and filters work: + assertEquals(new PrefixFilter(new Term("field", "o")), new PrefixFilter(new Term("field", "o"))); + assertFalse(new PrefixFilter(new Term("field", "a")).equals(new PrefixFilter(new Term("field", "o")))); + QueryUtils.checkHashEquals(new TermQuery(new Term("field", "one"))); + QueryUtils.checkUnequal( + new TermQuery(new Term("field", "one")), new TermQuery(new Term("field", "two")) + ); + // now test FilteredQuery equals/hashcode: + QueryUtils.checkHashEquals(new FilteredQuery(new TermQuery(new Term("field", "one")), new PrefixFilter(new Term("field", "o")))); + QueryUtils.checkUnequal( + new FilteredQuery(new TermQuery(new Term("field", "one")), new PrefixFilter(new Term("field", "o"))), + new FilteredQuery(new TermQuery(new Term("field", "two")), new PrefixFilter(new Term("field", "o"))) + ); + QueryUtils.checkUnequal( + new FilteredQuery(new TermQuery(new Term("field", "one")), new PrefixFilter(new Term("field", "a"))), + new FilteredQuery(new TermQuery(new Term("field", "one")), new PrefixFilter(new Term("field", "o"))) + ); + } + + public void testInvalidArguments() throws Exception { + try { + new FilteredQuery(null, null); + fail("Should throw IllegalArgumentException"); + } catch (IllegalArgumentException iae) { + // pass + } + try { + new FilteredQuery(new TermQuery(new Term("field", "one")), null); + fail("Should throw IllegalArgumentException"); + } catch (IllegalArgumentException iae) { + // pass + } + try { + new FilteredQuery(null, new PrefixFilter(new Term("field", "o"))); + fail("Should throw IllegalArgumentException"); + } catch (IllegalArgumentException iae) { + // pass + } + } + + private void assertRewrite(FilteredQuery fq, Class clazz) throws Exception { + // assign crazy boost to FQ + final float boost = random.nextFloat() * 100.f; + fq.setBoost(boost); + + // assign crazy boost to inner + final float innerBoost = random.nextFloat() * 100.f; + fq.getQuery().setBoost(innerBoost); + + // check the class and boosts of rewritten query + final Query rewritten = searcher.rewrite(fq); + assertTrue("is not instance of " + clazz.getName(), clazz.isInstance(rewritten)); + if (rewritten instanceof FilteredQuery) { + assertEquals(boost, rewritten.getBoost(), 1.E-5f); + assertEquals(innerBoost, ((FilteredQuery) rewritten).getQuery().getBoost(), 1.E-5f); + } else { + assertEquals(boost * innerBoost, rewritten.getBoost(), 1.E-5f); + } + + // check that the original query was not modified + assertEquals(boost, fq.getBoost(), 1.E-5f); + assertEquals(innerBoost, fq.getQuery().getBoost(), 1.E-5f); + } + + public void testRewrite() throws Exception { + assertRewrite(new FilteredQuery(new TermQuery(new Term("field", "one")), new PrefixFilter(new Term("field", "o"))), FilteredQuery.class); + assertRewrite(new FilteredQuery(new MatchAllDocsQuery(), new PrefixFilter(new Term("field", "o"))), ConstantScoreQuery.class); + } public static final class FilteredQueryRA extends FilteredQuery { private final boolean useRandomAccess;