DRILL-8017: Fix LGTM Issues Relating to Equals and HashCode Functions (…

…#2344)
apache · Oct 26, 2021 · dc60d28 · dc60d28
1 parent 374db31
commit dc60d28
Show file tree

Hide file tree

Showing 17 changed files with 135 additions and 16 deletions.
diff --git a/...format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/TableFormatPluginConfig.java b/...format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/TableFormatPluginConfig.java
@@ -20,7 +20,7 @@
 import org.apache.drill.common.logical.FormatPluginConfig;
 
 public abstract class TableFormatPluginConfig implements FormatPluginConfig {
-
+  //lgtm [java/inconsistent-equals-and-hashcode]
   @Override
   public boolean equals(Object obj) {
     if (this == obj) {

diff --git a/...at-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/MapRDBFormatPluginConfig.java b/...at-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/MapRDBFormatPluginConfig.java
@@ -53,6 +53,36 @@ public int hashCode() {
     return result;
   }
 
+  @Override
+  public boolean equals(Object obj) {
+    if (this == obj) {
+      return true;
+    } else if (obj == null || getClass() != obj.getClass()) {
+      return false;
+    }
+
+    MapRDBFormatPluginConfig other = (MapRDBFormatPluginConfig) obj;
+    if (readAllNumbersAsDouble != other.readAllNumbersAsDouble) {
+      return false;
+    } else if (allTextMode != other.allTextMode) {
+      return false;
+    } else if (ignoreSchemaChange != other.ignoreSchemaChange) {
+      return false;
+    } else if (enablePushdown != other.enablePushdown) {
+      return false;
+    } else if (disableCountOptimization != other.disableCountOptimization) {
+      return false;
+    } else if (nonExistentFieldSupport != other.nonExistentFieldSupport) {
+      return false;
+    } else if (!index.equals(other.index)) {
+      return false;
+    } else if (readTimestampWithZoneOffset != other.readTimestampWithZoneOffset) {
+      return false;
+    }
+    return true;
+  }
+
+
   @Override
   protected boolean impEquals(Object obj) {
     MapRDBFormatPluginConfig other = (MapRDBFormatPluginConfig) obj;

diff --git a/...c/main/java/org/apache/drill/exec/store/mapr/db/json/JsonTableRangePartitionFunction.java b/...c/main/java/org/apache/drill/exec/store/mapr/db/json/JsonTableRangePartitionFunction.java
@@ -18,6 +18,7 @@
 package org.apache.drill.exec.store.mapr.db.json;
 
 import java.util.List;
+import java.util.Objects;
 
 import org.apache.drill.common.expression.FieldReference;
 import org.apache.drill.exec.planner.physical.AbstractRangePartitionFunction;
@@ -106,6 +107,11 @@ public void setup(List<VectorWrapper<?>> partitionKeys) {
     Preconditions.checkArgument(partitionKeyVector != null, "Found null partitionKeVector.");
   }
 
+  @Override
+  public int hashCode() {
+    return Objects.hash(refList, tableName, userName, partitionKeyVector, startKeys, stopKeys);
+  }
+
   @Override
   public boolean equals(Object obj) {
     if (this == obj) {

diff --git a/...rdb/src/main/java/org/apache/drill/exec/store/mapr/streams/StreamsFormatPluginConfig.java b/...rdb/src/main/java/org/apache/drill/exec/store/mapr/streams/StreamsFormatPluginConfig.java
@@ -31,6 +31,16 @@ public int hashCode() {
     return 47;
   }
 
+  @Override
+  public boolean equals(Object that) {
+    if (this == that) {
+      return true;
+    } else if (that == null || getClass() != that.getClass()) {
+      return false;
+    }
+    return impEquals(that);
+  }
+
   @Override
   protected boolean impEquals(Object obj) {
     return true; // TODO: compare custom properties once added

diff --git a/...storage-kafka/src/main/java/org/apache/drill/exec/store/kafka/KafkaPartitionScanSpec.java b/...storage-kafka/src/main/java/org/apache/drill/exec/store/kafka/KafkaPartitionScanSpec.java
@@ -21,6 +21,9 @@
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.drill.common.PlanStringBuilder;
+
+import java.util.Objects;
 
 public class KafkaPartitionScanSpec {
   private final String topicName;
@@ -90,10 +93,19 @@ public boolean equals(Object obj) {
     return false;
   }
 
+  @Override
+  public int hashCode() {
+    return Objects.hash(topicName, partitionId, startOffset, endOffset);
+  }
+
   @Override
   public String toString() {
-    return "KafkaPartitionScanSpec [topicName=" + topicName + ", partitionId=" + partitionId + ", startOffset="
-               + startOffset + ", endOffset=" + endOffset + "]";
+    return new PlanStringBuilder(this)
+      .field("topicName", topicName)
+      .field("partitionId", partitionId)
+      .field("startOffset", startOffset)
+      .field("endOffset", endOffset)
+      .toString();
   }
 
   @Override

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/cost/DrillCostBase.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/cost/DrillCostBase.java
@@ -19,7 +19,8 @@
 
 import org.apache.calcite.plan.RelOptCost;
 import org.apache.calcite.plan.RelOptUtil;
-import org.apache.calcite.runtime.Utilities;
+
+import java.util.Objects;
 
 /**
  * Implementation of the DrillRelOptCost, modeled similar to VolcanoCost
@@ -159,8 +160,21 @@ public double getMemory() {
 
   @Override
   public int hashCode() {
-    return Utilities.hashCode(rowCount) + Utilities.hashCode(cpu)
-        + Utilities.hashCode(io) + Utilities.hashCode(network);
+    return Objects.hash(rowCount, cpu, io, network);
+  }
+
+  @Override
+  public boolean equals(Object that) {
+    if (this == that) {
+      return true;
+    } else if (that == null || getClass() != that.getClass()) {
+      return false;
+    }
+    DrillCostBase thatConfig = (DrillCostBase) that;
+    return Objects.equals(rowCount, thatConfig.rowCount) &&
+      Objects.equals(cpu, thatConfig.cpu) &&
+      Objects.equals(io, thatConfig.io) &&
+      Objects.equals(network, thatConfig.network);
   }
 
   @Override

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/index/DrillIndexDefinition.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/index/DrillIndexDefinition.java
@@ -198,10 +198,10 @@ public String toString() {
   public boolean equals(Object o) {
     if (this == o) {
       return true;
-    }
-    if (o == null) {
+    }  else if (o == null || getClass() != o.getClass()) {
       return false;
     }
+
     DrillIndexDefinition index1 = (DrillIndexDefinition) o;
     return tableName.equals(index1.tableName)
         && indexName.equals(index1.indexName)

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/StoragePlugins.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/StoragePlugins.java
@@ -22,6 +22,7 @@
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Objects;
 import java.util.Optional;
 
 import org.apache.drill.common.logical.StoragePluginConfig;
@@ -90,6 +91,11 @@ public boolean equals(Object obj) {
     return storage.equals(((StoragePlugins) obj).getStorage());
   }
 
+  @Override
+  public int hashCode() {
+    return Objects.hash(storage);
+  }
+
   /**
    * Put one plugin into current storage plugins map
    *

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/DeadBuf.java b/exec/java-exec/src/main/java/org/apache/drill/exec/record/DeadBuf.java
@@ -917,7 +917,7 @@ public ByteBuf asReadOnly() {
   }
 
   @Override
-  public boolean equals(Object arg0) {
+  public boolean equals(Object arg0) { //lgtm [java/unchecked-cast-in-equals]
     return false;
   }
 

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/schema/Field.java b/exec/java-exec/src/main/java/org/apache/drill/exec/schema/Field.java
@@ -22,6 +22,8 @@
 import org.apache.drill.shaded.guava.com.google.common.base.MoreObjects;
 import org.apache.drill.shaded.guava.com.google.common.base.Strings;
 
+import java.util.Objects;
+
 public abstract class Field {
   final String prefixFieldName;
   MajorType fieldType;
@@ -91,6 +93,17 @@ public void setFieldType(MajorType fieldType) {
     this.fieldType = fieldType;
   }
 
+  @Override
+  public boolean equals(Object that) {
+    if (this == that) {
+      return true;
+    } else if (that == null || getClass() != that.getClass()) {
+      return false;
+    }
+    Field thatField = (Field)that;
+    return Objects.equals(getFullFieldName(), thatField.getFullFieldName());
+  }
+
   @Override
   public int hashCode() {
     return getFullFieldName().hashCode();

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/log/LogFormatField.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/log/LogFormatField.java
@@ -80,4 +80,9 @@ public boolean equals(Object o) {
            Objects.equals(fieldType, other.fieldType) &&
            Objects.equals(format, other.format);
   }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(fieldName, fieldType, format);
+  }
 }
diff --git a/exec/vector/src/main/java/org/apache/drill/exec/record/metadata/AbstractPropertied.java b/exec/vector/src/main/java/org/apache/drill/exec/record/metadata/AbstractPropertied.java
@@ -120,7 +120,7 @@ public void removeProperty(String key) {
   }
 
   @Override
-  public boolean equals(Object o) {
+  public boolean equals(Object o) { //lgtm [java/unchecked-cast-in-equals]
     if (o == this) {
       return true;
     }

diff --git a/exec/vector/src/main/java/org/apache/drill/exec/record/metadata/TupleSchema.java b/exec/vector/src/main/java/org/apache/drill/exec/record/metadata/TupleSchema.java
@@ -24,10 +24,12 @@
 import com.fasterxml.jackson.annotation.JsonPropertyOrder;
 import org.apache.drill.exec.record.MaterializedField;
 
+
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.stream.Collectors;
 
 /**
@@ -172,6 +174,11 @@ public boolean equals(Object o) {
     return isEquivalent((TupleMetadata) o);
   }
 
+  @Override
+  public int hashCode() {
+    return Objects.hash(parentMap, nameSpace);
+  }
+
   @Override
   public List<MaterializedField> toFieldList() {
     List<MaterializedField> cols = new ArrayList<>();

diff --git a/exec/vector/src/main/java/org/apache/drill/exec/util/JsonStringArrayList.java b/exec/vector/src/main/java/org/apache/drill/exec/util/JsonStringArrayList.java
@@ -19,12 +19,11 @@
 
 import java.util.ArrayList;
 import java.util.List;
-
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 
 public class JsonStringArrayList<E> extends ArrayList<E> {
-
+//lgtm [java/inconsistent-equals-and-hashcode]
   private static ObjectMapper mapper;
 
   static {

diff --git a/exec/vector/src/main/java/org/apache/drill/exec/util/JsonStringHashMap.java b/exec/vector/src/main/java/org/apache/drill/exec/util/JsonStringHashMap.java
@@ -30,7 +30,7 @@
  * toString() method of the HashMap class to produce a JSON string instead
  */
 public class JsonStringHashMap<K, V> extends LinkedHashMap<K, V> {
-
+  // lgtm [java/inconsistent-equals-and-hashcode]
   private static final ObjectMapper mapper = new ObjectMapper()
       .registerModule(SerializationModule.getModule())
       .registerModule(new JodaModule());

diff --git a/logical/src/main/java/org/apache/drill/common/graph/AdjacencyList.java b/logical/src/main/java/org/apache/drill/common/graph/AdjacencyList.java
@@ -17,18 +17,20 @@
  */
 package org.apache.drill.common.graph;
 
+import org.apache.drill.shaded.guava.com.google.common.collect.ArrayListMultimap;
+import org.apache.drill.shaded.guava.com.google.common.collect.ListMultimap;
+import org.apache.drill.shaded.guava.com.google.common.collect.Multimaps;
+
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Objects;
 import java.util.Set;
 
-import org.apache.drill.shaded.guava.com.google.common.collect.ArrayListMultimap;
-import org.apache.drill.shaded.guava.com.google.common.collect.ListMultimap;
-import org.apache.drill.shaded.guava.com.google.common.collect.Multimaps;
 
 class AdjacencyList<V extends GraphValue<V>> {
   private Set<Node> allNodes = new HashSet<Node>();
@@ -167,6 +169,16 @@ public int hashCode() {
       return nodeValue.hashCode();
     }
 
+    @Override
+    public boolean equals(Object that) {
+      if (this == that) {
+        return true;
+      } else if (that == null || getClass() != that.getClass()) {
+        return false;
+      }
+      return Objects.equals(nodeValue, ((Node) that).getNodeValue());
+    }
+
     public V getNodeValue() {
       return nodeValue;
     }

diff --git a/logical/src/main/java/org/apache/drill/common/logical/data/LogicalOperatorBase.java b/logical/src/main/java/org/apache/drill/common/logical/data/LogicalOperatorBase.java
@@ -40,6 +40,11 @@ public final int hashCode() {
     return super.hashCode();
   }
 
+  @Override
+  public boolean equals(Object obj) {
+    return super.equals(obj);
+  }
+
   @Override
   public void setupAndValidate(List<LogicalOperator> operators, Collection<ValidationError> errors) {
     // TODO: remove this and implement individually.