From 46428da0dc2c486388adc137e2e869a64a2773a2 Mon Sep 17 00:00:00 2001 From: "Chris M. Hostetter" Date: Tue, 8 May 2007 18:15:48 +0000 Subject: [PATCH] SOLR-115 - minor optimization, replace uses of BooleanQuery.getClauses() with BooleanQuery.clauses() git-svn-id: https://svn.apache.org/repos/asf/lucene/solr/trunk@536282 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES.txt | 7 ++++++- .../solr/request/DisMaxRequestHandler.java | 4 ++-- .../solr/search/LuceneQueryOptimizer.java | 5 ++--- .../org/apache/solr/search/QueryParsing.java | 3 +-- .../org/apache/solr/search/QueryUtils.java | 21 ++++++++----------- .../org/apache/solr/util/SolrPluginUtils.java | 21 +++++++++---------- .../apache/solr/BasicFunctionalityTest.java | 2 +- .../apache/solr/search/TestQueryUtils.java | 6 ++++-- .../apache/solr/util/SolrPluginUtilsTest.java | 14 +++++++------ 9 files changed, 43 insertions(+), 40 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 0183f6686f3..9723b1a2ea0 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -221,7 +221,12 @@ Optimizations 1. SOLR-114: HashDocSet specific implementations of union() and andNot() for a 20x performance improvement for those set operations, and a new hash algorithm speeds up exists() by 10% and intersectionSize() by 8%. - (yonik) + (yonik) + + 2. SOLR-115: Solr now uses BooleanQuery.clauses() instead of + BooleanQuery.getClauses() in any situation where there is no risk of + modifying the original query. + (hossman) Bug Fixes 1. SOLR-87: Parsing of synonym files did not correctly handle escaped diff --git a/src/java/org/apache/solr/request/DisMaxRequestHandler.java b/src/java/org/apache/solr/request/DisMaxRequestHandler.java index eb8f40d9f28..71a3190101a 100644 --- a/src/java/org/apache/solr/request/DisMaxRequestHandler.java +++ b/src/java/org/apache/solr/request/DisMaxRequestHandler.java @@ -275,8 +275,8 @@ public class DisMaxRequestHandler extends RequestHandlerBase { /* if the default boost was used, and we've got a BooleanQuery * extract the subqueries out and use them directly */ - for (BooleanClause c : ((BooleanQuery)f).getClauses()) { - query.add(c); + for (Object c : ((BooleanQuery)f).clauses()) { + query.add((BooleanClause)c); } } else { query.add(f, BooleanClause.Occur.SHOULD); diff --git a/src/java/org/apache/solr/search/LuceneQueryOptimizer.java b/src/java/org/apache/solr/search/LuceneQueryOptimizer.java index 745e769898b..df9020f2db5 100644 --- a/src/java/org/apache/solr/search/LuceneQueryOptimizer.java +++ b/src/java/org/apache/solr/search/LuceneQueryOptimizer.java @@ -24,6 +24,7 @@ package org.apache.solr.search; import org.apache.lucene.search.*; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.io.IOException; @@ -64,9 +65,7 @@ class LuceneQueryOptimizer { BooleanQuery query = new BooleanQuery(); BooleanQuery filterQuery = null; - BooleanClause[] clauses = original.getClauses(); - for (int i = 0; i < clauses.length; i++) { - BooleanClause c = clauses[i]; + for (BooleanClause c : (List)original.clauses()) { /*** System.out.println("required="+c.required); diff --git a/src/java/org/apache/solr/search/QueryParsing.java b/src/java/org/apache/solr/search/QueryParsing.java index dfdbe68f824..dd0acb64770 100644 --- a/src/java/org/apache/solr/search/QueryParsing.java +++ b/src/java/org/apache/solr/search/QueryParsing.java @@ -304,9 +304,8 @@ public class QueryParsing { if (needParens) { out.append('('); } - BooleanClause[] clauses = q.getClauses(); boolean first=true; - for (BooleanClause c : clauses) { + for (BooleanClause c : (List)q.clauses()) { if (!first) { out.append(' '); } else { diff --git a/src/java/org/apache/solr/search/QueryUtils.java b/src/java/org/apache/solr/search/QueryUtils.java index 499176e7466..b5f1a7afe5c 100755 --- a/src/java/org/apache/solr/search/QueryUtils.java +++ b/src/java/org/apache/solr/search/QueryUtils.java @@ -18,12 +18,9 @@ public class QueryUtils { static boolean isNegative(Query q) { if (!(q instanceof BooleanQuery)) return false; BooleanQuery bq = (BooleanQuery)q; - // TODO: use after next lucene update - //for (BooleanClause clause: (List )bq.clauses()) { - // if (bq.getClauses().size()==0) return false; - BooleanClause[] clauses = bq.getClauses(); - if (clauses.length==0) return false; - for (BooleanClause clause: clauses) { + List clauses = bq.clauses(); + if (clauses.size()==0) return false; + for (BooleanClause clause : clauses) { if (!clause.isProhibited()) return false; } return true; @@ -43,17 +40,17 @@ public class QueryUtils { if (!(q instanceof BooleanQuery)) return q; BooleanQuery bq = (BooleanQuery)q; - BooleanClause[] clauses = bq.getClauses(); - if (clauses.length==0) return q; + List clauses = bq.clauses(); + if (clauses.size()==0) return q; - for (BooleanClause clause: clauses) { + for (BooleanClause clause : clauses) { if (!clause.isProhibited()) return q; } - if (clauses.length==1) { + if (clauses.size()==1) { // if only one clause, dispense with the wrapping BooleanQuery - Query negClause = clauses[0].getQuery(); + Query negClause = clauses.get(0).getQuery(); // we shouldn't need to worry about adjusting the boosts since the negative // clause would have never been selected in a positive query, and hence would // not contribute to a score. @@ -64,7 +61,7 @@ public class QueryUtils { // ignore minNrShouldMatch... it doesn't make sense for a negative query // the inverse of -a -b is a OR b - for (BooleanClause clause: clauses) { + for (BooleanClause clause : clauses) { newBq.add(clause.getQuery(), BooleanClause.Occur.SHOULD); } return newBq; diff --git a/src/java/org/apache/solr/util/SolrPluginUtils.java b/src/java/org/apache/solr/util/SolrPluginUtils.java index 92f6dbeba4b..506b1510861 100644 --- a/src/java/org/apache/solr/util/SolrPluginUtils.java +++ b/src/java/org/apache/solr/util/SolrPluginUtils.java @@ -566,7 +566,7 @@ public class SolrPluginUtils { public static void setMinShouldMatch(BooleanQuery q, String spec) { int optionalClauses = 0; - for (BooleanClause c : q.getClauses()) { + for (BooleanClause c : (List)q.clauses()) { if (c.getOccur() == Occur.SHOULD) { optionalClauses++; } @@ -633,21 +633,20 @@ public class SolrPluginUtils { */ public static void flattenBooleanQuery(BooleanQuery to, BooleanQuery from) { - BooleanClause[] c = from.getClauses(); - for (int i = 0; i < c.length; i++) { + for (BooleanClause clause : (List)from.clauses()) { + + Query cq = clause.getQuery(); + cq.setBoost(cq.getBoost() * from.getBoost()); - Query ci = c[i].getQuery(); - ci.setBoost(ci.getBoost() * from.getBoost()); - - if (ci instanceof BooleanQuery - && !c[i].isRequired() - && !c[i].isProhibited()) { + if (cq instanceof BooleanQuery + && !clause.isRequired() + && !clause.isProhibited()) { /* we can recurse */ - flattenBooleanQuery(to, (BooleanQuery)ci); + flattenBooleanQuery(to, (BooleanQuery)cq); } else { - to.add(c[i]); + to.add(clause); } } } diff --git a/src/test/org/apache/solr/BasicFunctionalityTest.java b/src/test/org/apache/solr/BasicFunctionalityTest.java index 7fe44f20b56..ce6223606ba 100644 --- a/src/test/org/apache/solr/BasicFunctionalityTest.java +++ b/src/test/org/apache/solr/BasicFunctionalityTest.java @@ -281,7 +281,7 @@ public class BasicFunctionalityTest extends AbstractSolrTestCase { h.getCore().getSchema()); assertTrue("not boolean?", q instanceof BooleanQuery); assertEquals("unexpected number of stemmed synonym tokens", - 2, ((BooleanQuery) q).getClauses().length); + 2, ((BooleanQuery) q).clauses().size()); } diff --git a/src/test/org/apache/solr/search/TestQueryUtils.java b/src/test/org/apache/solr/search/TestQueryUtils.java index 0578dadc568..8933534ab2e 100755 --- a/src/test/org/apache/solr/search/TestQueryUtils.java +++ b/src/test/org/apache/solr/search/TestQueryUtils.java @@ -8,6 +8,8 @@ import org.apache.lucene.search.Query; import org.apache.lucene.index.Term; import org.apache.solr.util.AbstractSolrTestCase; +import java.util.List; + /** * @author yonik * @version $Id$ @@ -27,9 +29,9 @@ public class TestQueryUtils extends AbstractSolrTestCase { public void positive(Query q) { assertFalse(QueryUtils.isNegative(q)); assertTrue(QueryUtils.getAbs(q)==q); - BooleanClause[] clauses = (q instanceof BooleanQuery) ? ((BooleanQuery)q).getClauses() : null; + List clauses = (q instanceof BooleanQuery) ? ((BooleanQuery)q).clauses() : null; if (clauses != null) { - if (clauses.length != 0) { + if (clauses.size() != 0) { assertTrue(QueryUtils.makeQueryable(q)==q); } } else { diff --git a/src/test/org/apache/solr/util/SolrPluginUtilsTest.java b/src/test/org/apache/solr/util/SolrPluginUtilsTest.java index 05dff252dbc..2413517c64e 100644 --- a/src/test/org/apache/solr/util/SolrPluginUtilsTest.java +++ b/src/test/org/apache/solr/util/SolrPluginUtilsTest.java @@ -186,14 +186,15 @@ public class SolrPluginUtilsTest extends AbstractSolrTestCase { out instanceof BooleanQuery); { BooleanQuery bq = (BooleanQuery)out; + List clauses = bq.clauses(); assertEquals(t+" wrong number of clauses", 2, - bq.getClauses().length); - Query sub = bq.getClauses()[0].getQuery(); + clauses.size()); + Query sub = clauses.get(0).getQuery(); assertTrue(t+" first wasn't a DMQ:" + sub.getClass(), sub instanceof DisjunctionMaxQuery); assertEquals(t+" first had wrong number of clauses", 4, countItems(((DisjunctionMaxQuery)sub).iterator())); - sub = bq.getClauses()[1].getQuery(); + sub = clauses.get(1).getQuery(); assertTrue(t+" second wasn't a DMQ:" + sub.getClass(), sub instanceof DisjunctionMaxQuery); assertEquals(t+" second had wrong number of clauses", 1, @@ -208,14 +209,15 @@ public class SolrPluginUtilsTest extends AbstractSolrTestCase { out instanceof BooleanQuery); { BooleanQuery bq = (BooleanQuery)out; + List clauses = bq.clauses(); assertEquals(t+" wrong number of clauses", 2, - bq.getClauses().length); - Query sub = bq.getClauses()[0].getQuery(); + clauses.size()); + Query sub = clauses.get(0).getQuery(); assertTrue(t+" first wasn't a DMQ:" + sub.getClass(), sub instanceof DisjunctionMaxQuery); assertEquals(t+" first had wrong number of clauses", 4, countItems(((DisjunctionMaxQuery)sub).iterator())); - sub = bq.getClauses()[1].getQuery(); + sub = clauses.get(1).getQuery(); assertTrue(t+" second wasn't a DMQ:" + sub.getClass(), sub instanceof DisjunctionMaxQuery); assertEquals(t+" second had wrong number of clauses (stop words)", 2,