From dc60d28e52d997c272105b8eef96e0d9b06af73e Mon Sep 17 00:00:00 2001
From: "Charles S. Givre" <cgivre@apache.org>
Date: Tue, 26 Oct 2021 09:27:30 -0400
Subject: [PATCH] DRILL-8017: Fix LGTM Issues Relating to Equals and HashCode
 Functions (#2344)

---
 .../store/mapr/TableFormatPluginConfig.java   |  2 +-
 .../mapr/db/MapRDBFormatPluginConfig.java     | 30 +++++++++++++++++++
 .../json/JsonTableRangePartitionFunction.java |  6 ++++
 .../streams/StreamsFormatPluginConfig.java    | 10 +++++++
 .../store/kafka/KafkaPartitionScanSpec.java   | 16 ++++++++--
 .../exec/planner/cost/DrillCostBase.java      | 20 +++++++++++--
 .../planner/index/DrillIndexDefinition.java   |  4 +--
 .../exec/planner/logical/StoragePlugins.java  |  6 ++++
 .../org/apache/drill/exec/record/DeadBuf.java |  2 +-
 .../org/apache/drill/exec/schema/Field.java   | 13 ++++++++
 .../drill/exec/store/log/LogFormatField.java  |  5 ++++
 .../record/metadata/AbstractPropertied.java   |  2 +-
 .../exec/record/metadata/TupleSchema.java     |  7 +++++
 .../drill/exec/util/JsonStringArrayList.java  |  3 +-
 .../drill/exec/util/JsonStringHashMap.java    |  2 +-
 .../drill/common/graph/AdjacencyList.java     | 18 +++++++++--
 .../logical/data/LogicalOperatorBase.java     |  5 ++++
 17 files changed, 135 insertions(+), 16 deletions(-)

diff --git a/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/TableFormatPluginConfig.java b/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/TableFormatPluginConfig.java
index ffd3d921509..0122254dd3b 100644
--- a/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/TableFormatPluginConfig.java
+++ b/contrib/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/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/MapRDBFormatPluginConfig.java b/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/MapRDBFormatPluginConfig.java
index c17696c0f96..9d0605ff843 100644
--- a/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/MapRDBFormatPluginConfig.java
+++ b/contrib/format-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/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/json/JsonTableRangePartitionFunction.java b/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/json/JsonTableRangePartitionFunction.java
index c97d303ceed..4fe656cf7b5 100644
--- a/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/json/JsonTableRangePartitionFunction.java
+++ b/contrib/format-maprdb/src/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/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/streams/StreamsFormatPluginConfig.java b/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/streams/StreamsFormatPluginConfig.java
index 1da795c5374..e78f1e8b43b 100644
--- a/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/streams/StreamsFormatPluginConfig.java
+++ b/contrib/format-maprdb/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/contrib/storage-kafka/src/main/java/org/apache/drill/exec/store/kafka/KafkaPartitionScanSpec.java b/contrib/storage-kafka/src/main/java/org/apache/drill/exec/store/kafka/KafkaPartitionScanSpec.java
index 509f77af56e..a620ac8e1bb 100644
--- a/contrib/storage-kafka/src/main/java/org/apache/drill/exec/store/kafka/KafkaPartitionScanSpec.java
+++ b/contrib/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
index 97ee688a820..5a44afbcc9e 100644
--- 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
index d756ae0f18a..bc6fe2d7679 100644
--- 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
index 3b2a04f1e90..9ec9d9b1e0d 100644
--- 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
index c3c3539de58..dcaf2262beb 100644
--- 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
index e7bef59b5ad..eee235cef27 100644
--- 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
index 87948380174..f8f2b8c45be 100644
--- 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
index 31bf83a690f..a04cdd9f4a3 100644
--- 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
index 0fd4a5a63e8..b5cef5ac0cd 100644
--- 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
index 695befdb85e..9af530f1650 100644
--- 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
index 337858fa6ce..85368221975 100644
--- 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
index acfca83ee1c..fd5f6712550 100644
--- 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,6 +17,10 @@
  */
 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;
@@ -24,11 +28,9 @@
 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
index 7c6cf58d67d..4c1f7e2f76b 100644
--- 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.