svn commit: r1213771 - in /lucene/dev/trunk/lucene: CHANGES.txt src/java/org/apache/lucene/search/FilteredQuery.java src/test-framework/java/org/apache/lucene/search/QueryUtils.java src/test/org/apache/lucene/search/TestFilteredQuery.java

[us...@apache.org]

Author: uschindler
Date: Tue Dec 13 16:32:24 2011
New Revision: 1213771

URL: http://svn.apache.org/viewvc?rev=1213771&view=rev
Log:
LUCENE-3643: FilteredQuery and IndexSearcher.search(Query, Filter,...) now optimize the special case "query instanceof MatchAllDocsQuery" to execute as ConstantScoreQuery

Modified:
    lucene/dev/trunk/lucene/CHANGES.txt
    lucene/dev/trunk/lucene/src/java/org/apache/lucene/search/FilteredQuery.java
    lucene/dev/trunk/lucene/src/test-framework/java/org/apache/lucene/search/QueryUtils.java
    lucene/dev/trunk/lucene/src/test/org/apache/lucene/search/TestFilteredQuery.java

Modified: lucene/dev/trunk/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/CHANGES.txt?rev=1213771&r1=1213770&r2=1213771&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/CHANGES.txt (original)
+++ lucene/dev/trunk/lucene/CHANGES.txt Tue Dec 13 16:32:24 2011
@@ -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

Modified: lucene/dev/trunk/lucene/src/java/org/apache/lucene/search/FilteredQuery.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/src/java/org/apache/lucene/search/FilteredQuery.java?rev=1213771&r1=1213770&r2=1213771&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/src/java/org/apache/lucene/search/FilteredQuery.java (original)
+++ lucene/dev/trunk/lucene/src/java/org/apache/lucene/search/FilteredQuery.java Tue Dec 13 16:32:24 2011
@@ -33,25 +33,23 @@ import java.util.Set;
  * <p>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.
- *
- * <p>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 <code>null</code>.
    * @param filter Filter to apply to query results, cannot be <code>null</code>.
    */
   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<Term> terms) {
-      getQuery().extractTerms(terms);
+    getQuery().extractTerms(terms);
   }
 
   /** Prints a user-readable version of this query. */
@@ -271,16 +283,21 @@ extends Query {
   /** Returns true iff <code>o</code> 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;
   }
 }

Modified: lucene/dev/trunk/lucene/src/test-framework/java/org/apache/lucene/search/QueryUtils.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/src/test-framework/java/org/apache/lucene/search/QueryUtils.java?rev=1213771&r1=1213770&r2=1213771&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/src/test-framework/java/org/apache/lucene/search/QueryUtils.java (original)
+++ lucene/dev/trunk/lucene/src/test-framework/java/org/apache/lucene/search/QueryUtils.java Tue Dec 13 16:32:24 2011
@@ -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.

Modified: lucene/dev/trunk/lucene/src/test/org/apache/lucene/search/TestFilteredQuery.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/src/test/org/apache/lucene/search/TestFilteredQuery.java?rev=1213771&r1=1213770&r2=1213771&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/src/test/org/apache/lucene/search/TestFilteredQuery.java (original)
+++ lucene/dev/trunk/lucene/src/test/org/apache/lucene/search/TestFilteredQuery.java Tue Dec 13 16:32:24 2011
@@ -132,6 +132,11 @@ public class TestFilteredQuery extends L
     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 L
 
   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 L
 
   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 L
     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<? extends Query> 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;