Skip to content

Commit

Permalink
Make sure to include class name in Query hash codes (elastic#75871)
Browse files Browse the repository at this point in the history
In some Query subclasses, we forgot to include the class name when computing
hashCode. This could increase the chance of collision between query hash codes.
Hash code collisions shouldn't affect correctness, but could change query
caching behavior since UsageTrackingQueryCachingPolicy tracks query frequency
based on hash codes.

The PR also simplifies some 'equals' methods to use the helper method
sameClassAs.
  • Loading branch information
jtibshirani authored and Adam Locke committed Aug 3, 2021
1 parent fc79704 commit 0a2d96b
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public String toString(String field) {
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (sameClassAs(o) == false) return false;
BinaryDocValuesRangeQuery that = (BinaryDocValuesRangeQuery) o;
return Objects.equals(fieldName, that.fieldName) &&
queryType == that.queryType &&
Expand All @@ -127,7 +127,7 @@ public boolean equals(Object o) {

@Override
public int hashCode() {
return Objects.hash(getClass(), fieldName, queryType, lengthType, from, to);
return Objects.hash(classHash(), fieldName, queryType, lengthType, from, to);
}

public enum QueryType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ public String toString(String field) {
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (sameClassAs(o) == false) return false;
ScriptScoreQuery that = (ScriptScoreQuery) o;
return shardId == that.shardId &&
subQuery.equals(that.subQuery) &&
Expand All @@ -186,7 +186,7 @@ public boolean equals(Object o) {

@Override
public int hashCode() {
return Objects.hash(subQuery, script, minScore, indexName, shardId, indexVersion);
return Objects.hash(classHash(), subQuery, script, minScore, indexName, shardId, indexVersion);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ public void visit(QueryVisitor visitor) {
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (sameClassAs(o) == false) return false;
DateRangeIncludingNowQuery that = (DateRangeIncludingNowQuery) o;
return Objects.equals(in, that.in);
}

@Override
public int hashCode() {
return Objects.hash(in);
return Objects.hash(classHash(), in);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public boolean equals(Object obj) {

@Override
public int hashCode() {
return Objects.hash(getClass(), query, path);
return Objects.hash(classHash(), query, path);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public String toString(String field) {
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (sameClassAs(o) == false) return false;

ShardSplittingQuery that = (ShardSplittingQuery) o;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public String toString(String field) {

@Override
public boolean equals(Object obj) {
if (obj == null || obj.getClass() != getClass()) {
if (sameClassAs(obj) == false) {
return false;
}
MergedPointRangeQuery other = (MergedPointRangeQuery) obj;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,12 @@ public Explanation explain(LeafReaderContext context, int doc) throws IOExceptio

@Override
public int hashCode() {
return Objects.hash(getClass(), script, fieldName);
return Objects.hash(classHash(), script, fieldName);
}

@Override
public boolean equals(Object obj) {
if (obj == null || getClass() != obj.getClass()) {
if (sameClassAs(obj) == false) {
return false;
}
AbstractScriptFieldQuery<?> other = (AbstractScriptFieldQuery<?>) obj;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,17 @@ public String toString(String field) {

@Override
public boolean equals(Object obj) {
if (obj == null || obj.getClass() != getClass()) {
if (sameClassAs(obj) == false) {
return false;
}
}
AutomatonQueryOnBinaryDv other = (AutomatonQueryOnBinaryDv) obj;
return Objects.equals(field, other.field) && Objects.equals(matchPattern, other.matchPattern)
&& Objects.equals(bytesMatcher, other.bytesMatcher);
}

@Override
public int hashCode() {
return Objects.hash(field, matchPattern, bytesMatcher);
return Objects.hash(classHash(), field, matchPattern, bytesMatcher);
}

}

0 comments on commit 0a2d96b

Please sign in to comment.