You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by jp...@apache.org on 2015/10/12 23:05:03 UTC
svn commit: r1708244 - in /lucene/dev/branches/branch_5x: ./ lucene/
lucene/core/ lucene/core/src/java/org/apache/lucene/search/
lucene/core/src/test/org/apache/lucene/search/
Author: jpountz
Date: Mon Oct 12 21:05:03 2015
New Revision: 1708244
URL: http://svn.apache.org/viewvc?rev=1708244&view=rev
Log:
LUCENE-6305: BooleanQuery.equals/hashcode ignore clause order.
Added:
lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/search/Multiset.java
- copied, changed from r1708211, lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/Multiset.java
lucene/dev/branches/branch_5x/lucene/core/src/test/org/apache/lucene/search/TestMultiset.java
- copied, changed from r1708211, lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestMultiset.java
Modified:
lucene/dev/branches/branch_5x/ (props changed)
lucene/dev/branches/branch_5x/lucene/ (props changed)
lucene/dev/branches/branch_5x/lucene/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_5x/lucene/core/ (props changed)
lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java
lucene/dev/branches/branch_5x/lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java
Modified: lucene/dev/branches/branch_5x/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/CHANGES.txt?rev=1708244&r1=1708243&r2=1708244&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/lucene/CHANGES.txt (original)
+++ lucene/dev/branches/branch_5x/lucene/CHANGES.txt Mon Oct 12 21:05:03 2015
@@ -220,6 +220,9 @@ Changes in Runtime Behavior
* LUCENE-6784: IndexSearcher's query caching is enabled by default. Run
indexSearcher.setQueryCache(null) to disable. (Adrien Grand)
+* LUCENE-6305: BooleanQuery.equals and hashcode do not depend on the order of
+ clauses anymore. (Adrien Grand)
+
======================= Lucene 5.3.1 =======================
Bug Fixes
Modified: lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java?rev=1708244&r1=1708243&r2=1708244&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java (original)
+++ lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java Mon Oct 12 21:05:03 2015
@@ -20,9 +20,13 @@ package org.apache.lucene.search;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
+import java.util.EnumMap;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
+import java.util.Map;
import java.util.Objects;
import org.apache.lucene.index.IndexReader;
@@ -111,7 +115,10 @@ public class BooleanQuery extends Query
}
/**
- * Add a clause to the {@link BooleanQuery}.
+ * Add a new clause to this {@link Builder}. Note that the order in which
+ * clauses are added does not have any impact on matching documents or query
+ * performance.
+ * @throws TooManyClauses if the new number of clauses exceeds the maximum clause number
*/
public Builder add(BooleanClause clause) {
add(clause.getQuery(), clause.getOccur());
@@ -119,7 +126,9 @@ public class BooleanQuery extends Query
}
/**
- * Add a clause to the {@link BooleanQuery}.
+ * Add a new clause to this {@link Builder}. Note that the order in which
+ * clauses are added does not have any impact on matching documents or query
+ * performance.
* @throws TooManyClauses if the new number of clauses exceeds the maximum clause number
*/
public Builder add(Query query, Occur occur) {
@@ -141,7 +150,8 @@ public class BooleanQuery extends Query
private final boolean mutable;
private final boolean disableCoord;
private int minimumNumberShouldMatch;
- private List<BooleanClause> clauses;
+ private List<BooleanClause> clauses; // used for toString() and getClauses()
+ private final Map<Occur, Collection<Query>> clauseSets; // used for equals/hashcode
private BooleanQuery(boolean disableCoord, int minimumNumberShouldMatch,
BooleanClause[] clauses) {
@@ -149,6 +159,16 @@ public class BooleanQuery extends Query
this.minimumNumberShouldMatch = minimumNumberShouldMatch;
this.clauses = Collections.unmodifiableList(Arrays.asList(clauses));
this.mutable = false;
+ clauseSets = new EnumMap<>(Occur.class);
+ // duplicates matter for SHOULD and MUST
+ clauseSets.put(Occur.SHOULD, new Multiset<Query>());
+ clauseSets.put(Occur.MUST, new Multiset<Query>());
+ // but not for FILTER and MUST_NOT
+ clauseSets.put(Occur.FILTER, new HashSet<Query>());
+ clauseSets.put(Occur.MUST_NOT, new HashSet<Query>());
+ for (BooleanClause clause : clauses) {
+ clauseSets.get(clause.getOccur()).add(clause.getQuery());
+ }
}
/**
@@ -282,20 +302,68 @@ public class BooleanQuery extends Query
return buffer.toString();
}
+ /**
+ * Compares the specified object with this boolean query for equality.
+ * Returns true if and only if the provided object<ul>
+ * <li>is also a {@link BooleanQuery},</li>
+ * <li>has the same value of {@link #isCoordDisabled()}</li>
+ * <li>has the same value of {@link #getMinimumNumberShouldMatch()}</li>
+ * <li>has the same {@link Occur#SHOULD} clauses, regardless of the order</li>
+ * <li>has the same {@link Occur#MUST} clauses, regardless of the order</li>
+ * <li>has the same set of {@link Occur#FILTER} clauses, regardless of the
+ * order and regardless of duplicates</li>
+ * <li>has the same set of {@link Occur#MUST_NOT} clauses, regardless of
+ * the order and regardless of duplicates</li></ul>
+ */
@Override
public boolean equals(Object o) {
if (super.equals(o) == false) {
return false;
}
BooleanQuery that = (BooleanQuery)o;
- return this.getMinimumNumberShouldMatch() == that.getMinimumNumberShouldMatch()
- && this.disableCoord == that.disableCoord
- && clauses.equals(that.clauses);
+ if (this.getMinimumNumberShouldMatch() != that.getMinimumNumberShouldMatch()) {
+ return false;
+ }
+ if (this.disableCoord != that.disableCoord) {
+ return false;
+ }
+ if (this.mutable != that.mutable) {
+ return false;
+ }
+ if (this.mutable) {
+ // depends on order
+ return clauses.equals(that.clauses);
+ } else {
+ // does not depend on order
+ return clauseSets.equals(that.clauseSets);
+ }
+ }
+
+ private int computeHashCode() {
+ int hashCode =Objects.hash(disableCoord, minimumNumberShouldMatch, clauseSets);
+ if (hashCode == 0) {
+ hashCode = 1;
+ }
+ return hashCode;
}
+ // cached hash code is only ok for immutable queries
+ private int hashCode;
+
@Override
public int hashCode() {
- return 31 * super.hashCode() + Objects.hash(disableCoord, minimumNumberShouldMatch, clauses);
+ if (mutable) {
+ assert clauseSets == null;
+ return 31 * super.hashCode() + Objects.hash(disableCoord, minimumNumberShouldMatch, clauses);
+ }
+
+ if (hashCode == 0) {
+ // no need for synchronization, in the worst case we would just compute the hash several times
+ hashCode = computeHashCode();
+ assert hashCode != 0;
+ }
+ assert hashCode == computeHashCode();
+ return 31 * super.hashCode() + hashCode;
}
// Backward compatibility for pre-5.3 BooleanQuery APIs
@@ -340,6 +408,7 @@ public class BooleanQuery extends Query
this.disableCoord = disableCoord;
this.minimumNumberShouldMatch = 0;
this.mutable = true;
+ this.clauseSets = null;
}
private void ensureMutable(String method) {
Copied: lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/search/Multiset.java (from r1708211, lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/Multiset.java)
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/search/Multiset.java?p2=lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/search/Multiset.java&p1=lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/Multiset.java&r1=1708211&r2=1708244&rev=1708244&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/Multiset.java (original)
+++ lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/search/Multiset.java Mon Oct 12 21:05:03 2015
@@ -65,6 +65,11 @@ final class Multiset<T> extends Abstract
remaining -= 1;
return current;
}
+
+ @Override
+ public void remove() {
+ throw new UnsupportedOperationException();
+ }
};
}
@@ -81,7 +86,12 @@ final class Multiset<T> extends Abstract
@Override
public boolean add(T e) {
- map.put(e, map.getOrDefault(e, 0) + 1);
+ Integer currentFreq = map.get(e);
+ if (currentFreq == null) {
+ map.put(e, 1);
+ } else {
+ map.put(e, map.get(e) + 1);
+ }
size += 1;
return true;
}
Modified: lucene/dev/branches/branch_5x/lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java?rev=1708244&r1=1708243&r2=1708244&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java (original)
+++ lucene/dev/branches/branch_5x/lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java Mon Oct 12 21:05:03 2015
@@ -21,6 +21,7 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.BitSet;
+import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
@@ -76,6 +77,132 @@ public class TestBooleanQuery extends Lu
assertEquals(bq1.build(), bq2.build());
}
+ public void testEqualityOnDeprecatedQuery() {
+ BooleanQuery bq1 = new BooleanQuery();
+ bq1.add(new TermQuery(new Term("foo", "bar")), Occur.SHOULD);
+ bq1.add(new TermQuery(new Term("foo", "baz")), Occur.SHOULD);
+ BooleanQuery bq2 = new BooleanQuery();
+ bq2.add(new TermQuery(new Term("foo", "bar")), Occur.SHOULD);
+ bq2.add(new TermQuery(new Term("foo", "baz")), Occur.SHOULD);
+ assertEquals(bq1, bq2);
+ }
+
+ public void testEqualityMutableVsImmutable() {
+ assertFalse(new BooleanQuery().equals(new BooleanQuery.Builder().build()));
+ assertFalse(new BooleanQuery.Builder().build().equals(new BooleanQuery()));
+ }
+
+ public void testEqualityDoesNotDependOnOrder() {
+ TermQuery[] queries = new TermQuery[] {
+ new TermQuery(new Term("foo", "bar")),
+ new TermQuery(new Term("foo", "baz"))
+ };
+ for (int iter = 0; iter < 10; ++iter) {
+ List<BooleanClause> clauses = new ArrayList<>();
+ final int numClauses = random().nextInt(20);
+ for (int i = 0; i < numClauses; ++i) {
+ Query query = RandomPicks.randomFrom(random(), queries);
+ if (random().nextBoolean()) {
+ query = new BoostQuery(query, random().nextFloat());
+ }
+ Occur occur = RandomPicks.randomFrom(random(), Occur.values());
+ clauses.add(new BooleanClause(query, occur));
+ }
+
+ final boolean disableCoord = random().nextBoolean();
+ final int minShouldMatch = random().nextInt(5);
+ BooleanQuery.Builder bq1Builder = new BooleanQuery.Builder();
+ bq1Builder.setDisableCoord(disableCoord);
+ bq1Builder.setMinimumNumberShouldMatch(minShouldMatch);
+ for (BooleanClause clause : clauses) {
+ bq1Builder.add(clause);
+ }
+ final BooleanQuery bq1 = bq1Builder.build();
+
+ Collections.shuffle(clauses, random());
+ BooleanQuery.Builder bq2Builder = new BooleanQuery.Builder();
+ bq2Builder.setDisableCoord(disableCoord);
+ bq2Builder.setMinimumNumberShouldMatch(minShouldMatch);
+ for (BooleanClause clause : clauses) {
+ bq2Builder.add(clause);
+ }
+ final BooleanQuery bq2 = bq2Builder.build();
+
+ QueryUtils.checkEqual(bq1, bq2);
+ }
+ }
+
+ public void testEqualityOnDuplicateShouldClauses() {
+ BooleanQuery bq1 = new BooleanQuery.Builder()
+ .setDisableCoord(random().nextBoolean())
+ .setMinimumNumberShouldMatch(random().nextInt(2))
+ .add(new TermQuery(new Term("foo", "bar")), Occur.SHOULD)
+ .build();
+ BooleanQuery bq2 = new BooleanQuery.Builder()
+ .setDisableCoord(bq1.isCoordDisabled())
+ .setMinimumNumberShouldMatch(bq1.getMinimumNumberShouldMatch())
+ .add(new TermQuery(new Term("foo", "bar")), Occur.SHOULD)
+ .add(new TermQuery(new Term("foo", "bar")), Occur.SHOULD)
+ .build();
+ QueryUtils.checkUnequal(bq1, bq2);
+ }
+
+ public void testEqualityOnDuplicateMustClauses() {
+ BooleanQuery bq1 = new BooleanQuery.Builder()
+ .setDisableCoord(random().nextBoolean())
+ .setMinimumNumberShouldMatch(random().nextInt(2))
+ .add(new TermQuery(new Term("foo", "bar")), Occur.MUST)
+ .build();
+ BooleanQuery bq2 = new BooleanQuery.Builder()
+ .setDisableCoord(bq1.isCoordDisabled())
+ .setMinimumNumberShouldMatch(bq1.getMinimumNumberShouldMatch())
+ .add(new TermQuery(new Term("foo", "bar")), Occur.MUST)
+ .add(new TermQuery(new Term("foo", "bar")), Occur.MUST)
+ .build();
+ QueryUtils.checkUnequal(bq1, bq2);
+ }
+
+ public void testEqualityOnDuplicateFilterClauses() {
+ BooleanQuery bq1 = new BooleanQuery.Builder()
+ .setDisableCoord(random().nextBoolean())
+ .setMinimumNumberShouldMatch(random().nextInt(2))
+ .add(new TermQuery(new Term("foo", "bar")), Occur.FILTER)
+ .build();
+ BooleanQuery bq2 = new BooleanQuery.Builder()
+ .setDisableCoord(bq1.isCoordDisabled())
+ .setMinimumNumberShouldMatch(bq1.getMinimumNumberShouldMatch())
+ .add(new TermQuery(new Term("foo", "bar")), Occur.FILTER)
+ .add(new TermQuery(new Term("foo", "bar")), Occur.FILTER)
+ .build();
+ QueryUtils.checkEqual(bq1, bq2);
+ }
+
+ public void testEqualityOnDuplicateMustNotClauses() {
+ BooleanQuery bq1 = new BooleanQuery.Builder()
+ .setDisableCoord(random().nextBoolean())
+ .setMinimumNumberShouldMatch(random().nextInt(2))
+ .add(new MatchAllDocsQuery(), Occur.MUST)
+ .add(new TermQuery(new Term("foo", "bar")), Occur.FILTER)
+ .build();
+ BooleanQuery bq2 = new BooleanQuery.Builder()
+ .setDisableCoord(bq1.isCoordDisabled())
+ .setMinimumNumberShouldMatch(bq1.getMinimumNumberShouldMatch())
+ .add(new MatchAllDocsQuery(), Occur.MUST)
+ .add(new TermQuery(new Term("foo", "bar")), Occur.FILTER)
+ .add(new TermQuery(new Term("foo", "bar")), Occur.FILTER)
+ .build();
+ QueryUtils.checkEqual(bq1, bq2);
+ }
+
+ public void testHashCodeIsStable() {
+ BooleanQuery bq = new BooleanQuery.Builder()
+ .add(new TermQuery(new Term("foo", TestUtil.randomSimpleString(random()))), Occur.SHOULD)
+ .add(new TermQuery(new Term("foo", TestUtil.randomSimpleString(random()))), Occur.SHOULD)
+ .build();
+ final int hashCode = bq.hashCode();
+ assertEquals(hashCode, bq.hashCode());
+ }
+
public void testException() {
try {
BooleanQuery.setMaxClauseCount(0);
@@ -767,39 +894,6 @@ public class TestBooleanQuery extends Lu
assertEquals(new HashSet<>(Arrays.asList(a, b, c)), matchingTerms);
}
- public void test52Compat() {
- final int iters = atLeast(5);
- for (int iter = 0; iter < iters; ++iter) {
- final boolean disableCoord = random().nextBoolean();
-
- BooleanQuery.Builder q1 = new BooleanQuery.Builder();
- q1.setDisableCoord(disableCoord);
- BooleanQuery q2 = new BooleanQuery(disableCoord);
-
- final int numClauses = random().nextInt(5);
- for (int i = 0; i < numClauses; ++i) {
- final Occur occur = RandomPicks.randomFrom(random(), Occur.values());
- final Query query = new TermQuery(new Term(TestUtil.randomSimpleString(random()), TestUtil.randomSimpleString(random())));
- if (random().nextBoolean()) {
- BooleanClause clause = new BooleanClause(query, occur);
- q1.add(clause);
- q2.add(clause);
- } else {
- q1.add(query, occur);
- q2.add(query, occur);
- }
- }
-
- if (random().nextBoolean()) {
- final int numShouldMatch = random().nextInt(5);
- q1.setMinimumNumberShouldMatch(numShouldMatch);
- q2.setMinimumNumberShouldMatch(numShouldMatch);
- }
-
- assertEquals(q1.build(), q2);
- }
- }
-
public void testBuilderImmutable() {
BooleanQuery.Builder builder = new BooleanQuery.Builder();
builder.setDisableCoord(random().nextBoolean());
Copied: lucene/dev/branches/branch_5x/lucene/core/src/test/org/apache/lucene/search/TestMultiset.java (from r1708211, lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestMultiset.java)
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/core/src/test/org/apache/lucene/search/TestMultiset.java?p2=lucene/dev/branches/branch_5x/lucene/core/src/test/org/apache/lucene/search/TestMultiset.java&p1=lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestMultiset.java&r1=1708211&r2=1708244&rev=1708244&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestMultiset.java (original)
+++ lucene/dev/branches/branch_5x/lucene/core/src/test/org/apache/lucene/search/TestMultiset.java Mon Oct 12 21:05:03 2015
@@ -56,7 +56,11 @@ public class TestMultiset extends Lucene
}
private static <T> void add(Map<T, Integer> map, T element) {
- map.put(element, map.getOrDefault(element, 0) + 1);
+ Integer currentFreq = map.get(element);
+ if (currentFreq == null) {
+ currentFreq = 0;
+ }
+ map.put(element, currentFreq + 1);
}
private static <T> void remove(Map<T, Integer> map, T element) {