diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index d833d5d25ea..244ce3e327f 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -529,6 +529,9 @@ Bug fixes * LUCENE-3348: Fix thread safety hazards in IndexWriter that could rarely cause deletions to be incorrectly applied. (Yonik Seeley, Simon Willnauer, Mike McCandless) + +* LUCENE-2945: Fix hashCode/equals for surround query parser generated queries. + (Paul Elschot, Simon Rosenthal, gsingers via ehatcher) ======================= Lucene 3.x (not yet released) ================ diff --git a/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/BasicQueryFactory.java b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/BasicQueryFactory.java index 1f9aa4debee..08ff4f72ddf 100644 --- a/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/BasicQueryFactory.java +++ b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/BasicQueryFactory.java @@ -45,8 +45,19 @@ public class BasicQueryFactory { public int getNrQueriesMade() {return queriesMade;} public int getMaxBasicQueries() {return maxBasicQueries;} - private synchronized void checkMax() throws TooManyBasicQueries { - if (queriesMade >= maxBasicQueries) + public String toString() { + return getClass().getName() + + "(maxBasicQueries: " + maxBasicQueries + + ", queriesMade: " + queriesMade + + ")"; + } + + private boolean atMax() { + return queriesMade >= maxBasicQueries; + } + + protected synchronized void checkMax() throws TooManyBasicQueries { + if (atMax()) throw new TooManyBasicQueries(getMaxBasicQueries()); queriesMade++; } @@ -60,6 +71,22 @@ public class BasicQueryFactory { checkMax(); return new SpanTermQuery(term); } + + @Override + public int hashCode() { + return getClass().hashCode() ^ (atMax() ? 7 : 31*32); + } + + /** Two BasicQueryFactory's are equal when they generate + * the same types of basic queries, or both cannot generate queries anymore. + */ + @Override + public boolean equals(Object obj) { + if (! (obj instanceof BasicQueryFactory)) + return false; + BasicQueryFactory other = (BasicQueryFactory) obj; + return atMax() == other.atMax(); + } } diff --git a/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/ComposedQuery.java b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/ComposedQuery.java index 0435d09b2b8..db51d3d2277 100644 --- a/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/ComposedQuery.java +++ b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/ComposedQuery.java @@ -24,36 +24,36 @@ import org.apache.lucene.search.Query; public abstract class ComposedQuery extends SrndQuery { - public ComposedQuery(List qs, boolean operatorInfix, String opName) { + public ComposedQuery(List qs, boolean operatorInfix, String opName) { recompose(qs); this.operatorInfix = operatorInfix; this.opName = opName; } - protected void recompose(List queries) { + protected void recompose(List queries) { if (queries.size() < 2) throw new AssertionError("Too few subqueries"); this.queries = queries; } - private String opName; + protected String opName; public String getOperatorName() {return opName;} - private List queries; + protected List queries; - public Iterator getSubQueriesIterator() {return queries.listIterator();} + public Iterator getSubQueriesIterator() {return queries.listIterator();} public int getNrSubQueries() {return queries.size();} - public SrndQuery getSubQuery(int qn) {return (SrndQuery) queries.get(qn);} + public SrndQuery getSubQuery(int qn) {return queries.get(qn);} private boolean operatorInfix; public boolean isOperatorInfix() { return operatorInfix; } /* else prefix operator */ public List makeLuceneSubQueriesField(String fn, BasicQueryFactory qf) { List luceneSubQueries = new ArrayList(); - Iterator sqi = getSubQueriesIterator(); + Iterator sqi = getSubQueriesIterator(); while (sqi.hasNext()) { - luceneSubQueries.add( ((SrndQuery) sqi.next()).makeLuceneQueryField(fn, qf)); + luceneSubQueries.add( (sqi.next()).makeLuceneQueryField(fn, qf)); } return luceneSubQueries; } @@ -77,7 +77,7 @@ public abstract class ComposedQuery extends SrndQuery { protected void infixToString(StringBuilder r) { /* Brackets are possibly redundant in the result. */ - Iterator sqi = getSubQueriesIterator(); + Iterator sqi = getSubQueriesIterator(); r.append(getBracketOpen()); if (sqi.hasNext()) { r.append(sqi.next().toString()); @@ -92,7 +92,7 @@ public abstract class ComposedQuery extends SrndQuery { } protected void prefixToString(StringBuilder r) { - Iterator sqi = getSubQueriesIterator(); + Iterator sqi = getSubQueriesIterator(); r.append(getOperatorName()); /* prefix operator */ r.append(getBracketOpen()); if (sqi.hasNext()) { @@ -109,9 +109,9 @@ public abstract class ComposedQuery extends SrndQuery { @Override public boolean isFieldsSubQueryAcceptable() { /* at least one subquery should be acceptable */ - Iterator sqi = getSubQueriesIterator(); + Iterator sqi = getSubQueriesIterator(); while (sqi.hasNext()) { - if (((SrndQuery) sqi.next()).isFieldsSubQueryAcceptable()) { + if ((sqi.next()).isFieldsSubQueryAcceptable()) { return true; } } diff --git a/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/DistanceQuery.java b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/DistanceQuery.java index ed654ba314d..6d400b34693 100644 --- a/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/DistanceQuery.java +++ b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/DistanceQuery.java @@ -16,7 +16,6 @@ package org.apache.lucene.queryparser.surround.query; * limitations under the License. */ - import java.util.List; import java.util.Iterator; @@ -39,12 +38,14 @@ public class DistanceQuery extends ComposedQuery implements DistanceSubQuery { this.ordered = ordered; } + private int opDistance; public int getOpDistance() {return opDistance;} private boolean ordered; public boolean subQueriesOrdered() {return ordered;} + @Override public String distanceSubQueryNotAllowed() { Iterator sqi = getSubQueriesIterator(); while (sqi.hasNext()) { @@ -61,31 +62,14 @@ public class DistanceQuery extends ComposedQuery implements DistanceSubQuery { } return null; /* subqueries acceptable */ } - + @Override public void addSpanQueries(SpanNearClauseFactory sncf) throws IOException { Query snq = getSpanNearQuery(sncf.getIndexReader(), sncf.getFieldName(), getWeight(), sncf.getBasicQueryFactory()); - sncf.addSpanNearQuery(snq); - } - - @Override - public Query makeLuceneQueryFieldNoBoost(final String fieldName, final BasicQueryFactory qf) { - return new Query () { - - @Override - public String toString(String fn) { - return getClass().toString() + " " + fieldName + " (" + fn + "?)"; - } - - @Override - public Query rewrite(IndexReader reader) throws IOException { - return getSpanNearQuery(reader, fieldName, getBoost(), qf); - } - - }; + sncf.addSpanQuery(snq); } public Query getSpanNearQuery( @@ -93,7 +77,7 @@ public class DistanceQuery extends ComposedQuery implements DistanceSubQuery { String fieldName, float boost, BasicQueryFactory qf) throws IOException { - SpanQuery[] spanNearClauses = new SpanQuery[getNrSubQueries()]; + SpanQuery[] spanClauses = new SpanQuery[getNrSubQueries()]; Iterator sqi = getSubQueriesIterator(); int qi = 0; while (sqi.hasNext()) { @@ -108,14 +92,18 @@ public class DistanceQuery extends ComposedQuery implements DistanceSubQuery { return SrndQuery.theEmptyLcnQuery; } - spanNearClauses[qi] = sncf.makeSpanNearClause(); - + spanClauses[qi] = sncf.makeSpanClause(); qi++; } - - SpanNearQuery r = new SpanNearQuery(spanNearClauses, getOpDistance() - 1, subQueriesOrdered()); + + SpanNearQuery r = new SpanNearQuery(spanClauses, getOpDistance() - 1, subQueriesOrdered()); r.setBoost(boost); return r; } + + @Override + public Query makeLuceneQueryFieldNoBoost(final String fieldName, final BasicQueryFactory qf) { + return new DistanceRewriteQuery(this, fieldName, qf); + } } diff --git a/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/DistanceRewriteQuery.java b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/DistanceRewriteQuery.java new file mode 100644 index 00000000000..9c1129dfe28 --- /dev/null +++ b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/DistanceRewriteQuery.java @@ -0,0 +1,38 @@ +package org.apache.lucene.queryparser.surround.query; +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import java.io.IOException; + +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.search.Query; + +class DistanceRewriteQuery extends RewriteQuery { + + DistanceRewriteQuery( + DistanceQuery srndQuery, + String fieldName, + BasicQueryFactory qf) { + super(srndQuery, fieldName, qf); + } + + @Override + public Query rewrite(IndexReader reader) throws IOException { + return srndQuery.getSpanNearQuery(reader, fieldName, getBoost(), qf); + } +} + diff --git a/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/RewriteQuery.java b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/RewriteQuery.java new file mode 100644 index 00000000000..f02929387f4 --- /dev/null +++ b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/RewriteQuery.java @@ -0,0 +1,81 @@ +package org.apache.lucene.queryparser.surround.query; +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import java.io.IOException; + +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.search.Query; + +abstract class RewriteQuery extends Query { + protected final SQ srndQuery; + protected final String fieldName; + protected final BasicQueryFactory qf; + + RewriteQuery( + SQ srndQuery, + String fieldName, + BasicQueryFactory qf) { + this.srndQuery = srndQuery; + this.fieldName = fieldName; + this.qf = qf; + } + + @Override + abstract public Query rewrite(IndexReader reader) throws IOException; + + @Override + public String toString() { + return toString(null); + } + + @Override + public String toString(String field) { + return getClass().getName() + + (field == null ? "" : "(unused: " + field + ")") + + "(" + fieldName + + ", " + srndQuery.toString() + + ", " + qf.toString() + + ")"; + } + + @Override + public int hashCode() { + return getClass().hashCode() + ^ fieldName.hashCode() + ^ qf.hashCode() + ^ srndQuery.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (obj == null) + return false; + if (! getClass().equals(obj.getClass())) + return false; + RewriteQuery other = (RewriteQuery)obj; + return fieldName.equals(other.fieldName) + && qf.equals(other.qf) + && srndQuery.equals(other.srndQuery); + } + + /** @throws UnsupportedOperationException */ + @Override + public Object clone() { + throw new UnsupportedOperationException(); + } +} + diff --git a/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SimpleTerm.java b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SimpleTerm.java index 3e9c1e336d3..6a36361fd7f 100644 --- a/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SimpleTerm.java +++ b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SimpleTerm.java @@ -17,12 +17,9 @@ package org.apache.lucene.queryparser.surround.query; */ import java.io.IOException; -import java.util.ArrayList; -import java.util.List; import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.Term; -import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.Query; public abstract class SimpleTerm @@ -38,7 +35,11 @@ public abstract class SimpleTerm public String getFieldOperator() {return "/";} public abstract String toStringUnquoted(); - + + /** @deprecated (March 2011) Not normally used, to be removed from Lucene 4.0. + * This class implementing Comparable is to be removed at the same time. + */ + @Deprecated public int compareTo(SimpleTerm ost) { /* for ordering terms and prefixes before using an index, not used */ return this.toStringUnquoted().compareTo( ost.toStringUnquoted()); @@ -70,35 +71,10 @@ public abstract class SimpleTerm void visitMatchingTerm(Term t)throws IOException; } + @Override public String distanceSubQueryNotAllowed() {return null;} - @Override - public Query makeLuceneQueryFieldNoBoost(final String fieldName, final BasicQueryFactory qf) { - return new Query() { - @Override - public String toString(String fn) { - return getClass().toString() + " " + fieldName + " (" + fn + "?)"; - } - - @Override - public Query rewrite(IndexReader reader) throws IOException { - final List luceneSubQueries = new ArrayList(); - visitMatchingTerms( reader, fieldName, - new MatchingTermVisitor() { - public void visitMatchingTerm(Term term) throws IOException { - luceneSubQueries.add(qf.newTermQuery(term)); - } - }); - return (luceneSubQueries.size() == 0) ? SrndQuery.theEmptyLcnQuery - : (luceneSubQueries.size() == 1) ? luceneSubQueries.get(0) - : SrndBooleanQuery.makeBooleanQuery( - /* luceneSubQueries all have default weight */ - luceneSubQueries, BooleanClause.Occur.SHOULD); /* OR the subquery terms */ - } - }; - } - public void addSpanQueries(final SpanNearClauseFactory sncf) throws IOException { visitMatchingTerms( sncf.getIndexReader(), @@ -109,6 +85,11 @@ public abstract class SimpleTerm } }); } + + @Override + public Query makeLuceneQueryFieldNoBoost(final String fieldName, final BasicQueryFactory qf) { + return new SimpleTermRewriteQuery(this, fieldName, qf); + } } diff --git a/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SimpleTermRewriteQuery.java b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SimpleTermRewriteQuery.java new file mode 100644 index 00000000000..963db84cd9c --- /dev/null +++ b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SimpleTermRewriteQuery.java @@ -0,0 +1,52 @@ +package org.apache.lucene.queryparser.surround.query; +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import java.io.IOException; +import java.util.List; +import java.util.ArrayList; + +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.BooleanClause; +import org.apache.lucene.index.Term; + +class SimpleTermRewriteQuery extends RewriteQuery { + + SimpleTermRewriteQuery( + SimpleTerm srndQuery, + String fieldName, + BasicQueryFactory qf) { + super(srndQuery, fieldName, qf); + } + + @Override + public Query rewrite(IndexReader reader) throws IOException { + final List luceneSubQueries = new ArrayList(); + srndQuery.visitMatchingTerms(reader, fieldName, + new SimpleTerm.MatchingTermVisitor() { + public void visitMatchingTerm(Term term) throws IOException { + luceneSubQueries.add(qf.newTermQuery(term)); + } + }); + return (luceneSubQueries.size() == 0) ? SrndQuery.theEmptyLcnQuery + : (luceneSubQueries.size() == 1) ? luceneSubQueries.get(0) + : SrndBooleanQuery.makeBooleanQuery( + /* luceneSubQueries all have default weight */ + luceneSubQueries, BooleanClause.Occur.SHOULD); /* OR the subquery terms */ + } +} + diff --git a/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SpanNearClauseFactory.java b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SpanNearClauseFactory.java index 68df313f008..264cd733d6e 100644 --- a/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SpanNearClauseFactory.java +++ b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SpanNearClauseFactory.java @@ -64,7 +64,7 @@ import org.apache.lucene.search.spans.SpanQuery; import org.apache.lucene.search.spans.SpanTermQuery; -public class SpanNearClauseFactory { +public class SpanNearClauseFactory { // FIXME: rename to SpanClauseFactory public SpanNearClauseFactory(IndexReader reader, String fieldName, BasicQueryFactory qf) { this.reader = reader; this.fieldName = fieldName; @@ -100,17 +100,16 @@ public class SpanNearClauseFactory { /* CHECKME: wrap in Hashable...? */ addSpanQueryWeighted(stq, weight); } - - public void addSpanNearQuery(Query q) { + + public void addSpanQuery(Query q) { if (q == SrndQuery.theEmptyLcnQuery) return; - if (! (q instanceof SpanNearQuery)) - throw new AssertionError("Expected SpanNearQuery: " + q.toString(getFieldName())); - /* CHECKME: wrap in Hashable...? */ - addSpanQueryWeighted((SpanNearQuery)q, q.getBoost()); + if (! (q instanceof SpanQuery)) + throw new AssertionError("Expected SpanQuery: " + q.toString(getFieldName())); + addSpanQueryWeighted((SpanQuery)q, q.getBoost()); } - - public SpanQuery makeSpanNearClause() { + + public SpanQuery makeSpanClause() { SpanQuery [] spanQueries = new SpanQuery[size()]; Iterator sqi = weightBySpanQuery.keySet().iterator(); int i = 0; diff --git a/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SrndQuery.java b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SrndQuery.java index 6d913ad9637..0082ddf4508 100644 --- a/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SrndQuery.java +++ b/modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SrndQuery.java @@ -53,6 +53,9 @@ public abstract class SrndQuery implements Cloneable { public abstract Query makeLuceneQueryFieldNoBoost(String fieldName, BasicQueryFactory qf); + /** This method is used by {@link #hashCode()} and {@link #equals(Object)}, + * see LUCENE-2945. + */ @Override public abstract String toString(); @@ -66,8 +69,32 @@ public abstract class SrndQuery implements Cloneable { throw new Error(cns); } } - -/* An empty Lucene query */ + + /** For subclasses of {@link SrndQuery} within the package + * {@link org.apache.lucene.queryparser.surround.query} + * it is not necessary to override this method, + * @see #toString(). + */ + @Override + public int hashCode() { + return getClass().hashCode() ^ toString().hashCode(); + } + + /** For subclasses of {@link SrndQuery} within the package + * {@link org.apache.lucene.queryparser.surround.query} + * it is not necessary to override this method, + * @see #toString(). + */ + @Override + public boolean equals(Object obj) { + if (obj == null) + return false; + if (! getClass().equals(obj.getClass())) + return false; + return toString().equals(obj.toString()); + } + + /** An empty Lucene query */ public final static Query theEmptyLcnQuery = new BooleanQuery() { /* no changes allowed */ @Override public void setBoost(float boost) { diff --git a/modules/queryparser/src/test/org/apache/lucene/queryparser/surround/query/SrndQueryTest.java b/modules/queryparser/src/test/org/apache/lucene/queryparser/surround/query/SrndQueryTest.java new file mode 100644 index 00000000000..d0bb1dddc28 --- /dev/null +++ b/modules/queryparser/src/test/org/apache/lucene/queryparser/surround/query/SrndQueryTest.java @@ -0,0 +1,52 @@ +package org.apache.lucene.queryparser.surround.query; + +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import junit.framework.Assert; + +import org.apache.lucene.queryparser.surround.parser.QueryParser; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.QueryUtils; +import org.apache.lucene.util.LuceneTestCase; +import org.junit.Test; + +/** + * + * + **/ +public class SrndQueryTest extends LuceneTestCase { + + void checkEqualParsings(String s1, String s2) throws Exception { + String fieldName = "foo"; + BasicQueryFactory qf = new BasicQueryFactory(16); + Query lq1, lq2; + lq1 = QueryParser.parse(s1).makeLuceneQueryField(fieldName, qf); + lq2 = QueryParser.parse(s2).makeLuceneQueryField(fieldName, qf); + QueryUtils.checkEqual(lq1, lq2); + } + + @Test + public void testHashEquals() throws Exception { + //grab some sample queries from Test02Boolean and Test03Distance and + //check there hashes and equals + checkEqualParsings("word1 w word2", " word1 w word2 "); + checkEqualParsings("2N(w1,w2,w3)", " 2N(w1, w2 , w3)"); + checkEqualParsings("abc?", " abc? "); + checkEqualParsings("w*rd?", " w*rd?"); + } +}