From b9a70c28b697f85d6ada5eedc5c05f8a6476ae7d Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Fri, 16 Jul 2021 09:59:33 -0700 Subject: [PATCH] LUCENE-10026: Fix CombinedFieldQuery equals and hashCode (#212) The previous equals and hashCode methods only compared query terms. This meant that queries on different fields, or with different field weights, were considered the same During boolean query rewrites, duplicate clauses are removed. So because equals/ hashCode was incorrect, rewrites could accidentally drop CombinedFieldQuery clauses. --- .../sandbox/search/CombinedFieldQuery.java | 29 ++++++++++++--- .../search/TestCombinedFieldQuery.java | 36 +++++++++++++++++++ 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/lucene/sandbox/src/java/org/apache/lucene/sandbox/search/CombinedFieldQuery.java b/lucene/sandbox/src/java/org/apache/lucene/sandbox/search/CombinedFieldQuery.java index 936fc7b07b8..71a88df416d 100644 --- a/lucene/sandbox/src/java/org/apache/lucene/sandbox/search/CombinedFieldQuery.java +++ b/lucene/sandbox/src/java/org/apache/lucene/sandbox/search/CombinedFieldQuery.java @@ -24,6 +24,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.TreeMap; import org.apache.lucene.index.IndexReader; @@ -147,6 +148,19 @@ public final class CombinedFieldQuery extends Query implements Accountable { this.field = field; this.weight = weight; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + FieldAndWeight that = (FieldAndWeight) o; + return Float.compare(that.weight, weight) == 0 && Objects.equals(field, that.field); + } + + @Override + public int hashCode() { + return Objects.hash(field, weight); + } } // sorted map for fields. @@ -212,13 +226,20 @@ public final class CombinedFieldQuery extends Query implements Accountable { } @Override - public int hashCode() { - return 31 * classHash() + Arrays.hashCode(terms); + public boolean equals(Object o) { + if (this == o) return true; + if (sameClassAs(o) == false) return false; + CombinedFieldQuery that = (CombinedFieldQuery) o; + return Objects.equals(fieldAndWeights, that.fieldAndWeights) + && Arrays.equals(terms, that.terms); } @Override - public boolean equals(Object other) { - return sameClassAs(other) && Arrays.equals(terms, ((CombinedFieldQuery) other).terms); + public int hashCode() { + int result = classHash(); + result += Objects.hash(fieldAndWeights); + result = 31 * result + Arrays.hashCode(terms); + return result; } @Override diff --git a/lucene/sandbox/src/test/org/apache/lucene/sandbox/search/TestCombinedFieldQuery.java b/lucene/sandbox/src/test/org/apache/lucene/sandbox/search/TestCombinedFieldQuery.java index bb9fe6f2440..4c2c0aef4a5 100644 --- a/lucene/sandbox/src/test/org/apache/lucene/sandbox/search/TestCombinedFieldQuery.java +++ b/lucene/sandbox/src/test/org/apache/lucene/sandbox/search/TestCombinedFieldQuery.java @@ -72,6 +72,42 @@ public class TestCombinedFieldQuery extends LuceneTestCase { assertEquals(actual, query); } + public void testEqualsAndHashCode() { + CombinedFieldQuery query1 = + new CombinedFieldQuery.Builder() + .addField("field1") + .addField("field2") + .addTerm(new BytesRef("value")) + .build(); + + CombinedFieldQuery query2 = + new CombinedFieldQuery.Builder() + .addField("field1") + .addField("field2", 1.3f) + .addTerm(new BytesRef("value")) + .build(); + assertNotEquals(query1, query2); + assertNotEquals(query1.hashCode(), query2.hashCode()); + + CombinedFieldQuery query3 = + new CombinedFieldQuery.Builder() + .addField("field3") + .addField("field4") + .addTerm(new BytesRef("value")) + .build(); + assertNotEquals(query1, query3); + assertNotEquals(query1.hashCode(), query2.hashCode()); + + CombinedFieldQuery duplicateQuery1 = + new CombinedFieldQuery.Builder() + .addField("field1") + .addField("field2") + .addTerm(new BytesRef("value")) + .build(); + assertEquals(query1, duplicateQuery1); + assertEquals(query1.hashCode(), duplicateQuery1.hashCode()); + } + public void testToString() { assertEquals("CombinedFieldQuery(()())", new CombinedFieldQuery.Builder().build().toString()); CombinedFieldQuery.Builder builder = new CombinedFieldQuery.Builder();