Skip to content

Speedup equals #126394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -112,18 +112,15 @@ protected Expression canonicalize() {
}

@Override
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
public int hashCode() {
return Objects.hash(super.hashCode(), nullability);
}

@Override
public boolean equals(Object obj) {
if (super.equals(obj)) {
Attribute other = (Attribute) obj;
return Objects.equals(nullability, other.nullability);
}

return false;
protected boolean innerEquals(Object o) {
var other = (Attribute) o;
return super.innerEquals(other) && Objects.equals(nullability, other.nullability);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,20 +60,13 @@ protected NodeInfo<? extends Expression> info() {
}

@Override
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
public int hashCode() {
return EmptyAttribute.class.hashCode();
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}

if (obj == null || getClass() != obj.getClass()) {
return false;
}

protected boolean innerEquals(Object o) {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ public boolean equals(Object obj) {
if (obj == null || getClass() != obj.getClass()) {
return false;
}

EntryExpression other = (EntryExpression) obj;
return Objects.equals(key, other.key) && Objects.equals(value, other.value);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,15 +223,15 @@ public Attribute withDataType(DataType type) {
}

@Override
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
public int hashCode() {
return Objects.hash(super.hashCode(), parentName, field);
}

@Override
public boolean equals(Object obj) {
return super.equals(obj)
&& Objects.equals(parentName, ((FieldAttribute) obj).parentName)
&& Objects.equals(field, ((FieldAttribute) obj).field);
protected boolean innerEquals(Object o) {
var other = (FieldAttribute) o;
return super.innerEquals(other) && Objects.equals(parentName, other.parentName) && Objects.equals(field, other.field);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,16 +173,14 @@ public static boolean isSupported(String name) {
}

@Override
public boolean equals(Object obj) {
if (false == super.equals(obj)) {
return false;
}
MetadataAttribute other = (MetadataAttribute) obj;
return searchable == other.searchable;
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
public int hashCode() {
return Objects.hash(super.hashCode(), searchable);
}

@Override
public int hashCode() {
return Objects.hash(super.hashCode(), searchable);
protected boolean innerEquals(Object o) {
var other = (MetadataAttribute) o;
return super.innerEquals(other) && searchable == other.searchable;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,26 @@ public int hashCode() {
return Objects.hash(super.hashCode(), name, synthetic);
}

/**
* Polymorphic equality is a pain and are likely slower than a regular ones.
* This equals shortcuts `this == o` and type checks (important when we expect only a few non-equal objects).
* Here equals is final to ensure we are not duplicating those checks.
* For actual equality check override `innerEquals` instead.
*/
@Override
public boolean equals(Object obj) {
if (this == obj) {
public final boolean equals(Object o) {
if (this == o) {
return true;
}
if (obj == null || getClass() != obj.getClass()) {
if (o == null || getClass() != o.getClass()) {
return false;
}
return innerEquals(o);
}

NamedExpression other = (NamedExpression) obj;
return Objects.equals(synthetic, other.synthetic)
protected boolean innerEquals(Object o) {
var other = (NamedExpression) o;
return synthetic == other.synthetic
/*
* It is important that the line below be `name`
* and not `name()` because subclasses might override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,14 @@ public DataType dataType() {
}

@Override
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
public int hashCode() {
return Objects.hash(super.hashCode(), dataType);
}

@Override
public boolean equals(Object obj) {
return super.equals(obj) && Objects.equals(dataType, ((TypedAttribute) obj).dataType);
protected boolean innerEquals(Object o) {
var other = (TypedAttribute) o;
return super.innerEquals(other) && dataType == other.dataType;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@

// unfortunately we can't use UnresolvedNamedExpression
public class UnresolvedAttribute extends Attribute implements Unresolvable {
private final String unresolvedMsg;
private final boolean customMessage;
private final String unresolvedMsg;
private final Object resolutionMetadata;

public UnresolvedAttribute(Source source, String name) {
Expand Down Expand Up @@ -119,16 +119,16 @@ public static String errorMessage(String name, List<String> potentialMatches) {
}

@Override
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
public int hashCode() {
return Objects.hash(super.hashCode(), resolutionMetadata, unresolvedMsg);
}

@Override
public boolean equals(Object obj) {
if (super.equals(obj)) {
UnresolvedAttribute ua = (UnresolvedAttribute) obj;
return Objects.equals(resolutionMetadata, ua.resolutionMetadata) && Objects.equals(unresolvedMsg, ua.unresolvedMsg);
}
return false;
protected boolean innerEquals(Object o) {
var other = (UnresolvedAttribute) o;
return super.innerEquals(other)
&& Objects.equals(resolutionMetadata, other.resolutionMetadata)
&& Objects.equals(unresolvedMsg, other.unresolvedMsg);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,25 +57,15 @@ public UnresolvedAttribute qualifier() {
}

@Override
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
public int hashCode() {
return Objects.hash(qualifier);
}

@Override
public boolean equals(Object obj) {
/*
* Intentionally not calling the superclass
* equals because it uses id which we always
* mutate when we make a clone. So we need
* to ignore it in equals for the transform
* tests to pass.
*/
if (obj == null || obj.getClass() != getClass()) {
return false;
}

UnresolvedStar other = (UnresolvedStar) obj;
return Objects.equals(qualifier, other.qualifier);
protected boolean innerEquals(Object o) {
var other = (UnresolvedStar) o;
return super.innerEquals(other) && Objects.equals(qualifier, other.qualifier);
}

private String message() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,15 @@ public Nullability nullable() {
}

@Override
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
public int hashCode() {
return Objects.hash(super.hashCode(), pattern);
}

@Override
public boolean equals(Object obj) {
if (super.equals(obj)) {
UnresolvedNamePattern ua = (UnresolvedNamePattern) obj;
return Objects.equals(pattern, ua.pattern);
}
return false;
protected boolean innerEquals(Object o) {
var other = (UnresolvedNamePattern) o;
return super.innerEquals(other) && Objects.equals(pattern, other.pattern);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ public final class UnsupportedAttribute extends FieldAttribute implements Unreso
UnsupportedAttribute::readFrom
);

private final String message;
private final boolean hasCustomMessage; // TODO remove me and just use message != null?
private final String message;

private static String errorMessage(String name, UnsupportedEsField field) {
return "Cannot use field [" + name + "] with unsupported type [" + String.join(",", field.getOriginalTypes()) + "]";
Expand Down Expand Up @@ -163,17 +163,15 @@ public boolean hasCustomMessage() {
}

@Override
@SuppressWarnings("checkstyle:EqualsHashCode")// equals is implemented in parent. See innerEquals instead
public int hashCode() {
return Objects.hash(super.hashCode(), hasCustomMessage, message);
}

@Override
public boolean equals(Object obj) {
if (super.equals(obj)) {
var ua = (UnsupportedAttribute) obj;
return Objects.equals(hasCustomMessage, ua.hasCustomMessage) && Objects.equals(message, ua.message);
}
return false;
protected boolean innerEquals(Object o) {
var other = (UnsupportedAttribute) o;
return super.innerEquals(other) && hasCustomMessage == other.hasCustomMessage && Objects.equals(message, other.message);
}

@Override
Expand Down