You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by cg...@apache.org on 2021/10/26 13:27:52 UTC
[drill] branch master updated: DRILL-8017: Fix LGTM Issues Relating
to Equals and HashCode Functions (#2344)
This is an automated email from the ASF dual-hosted git repository.
cgivre pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git
The following commit(s) were added to refs/heads/master by this push:
new b6da35e DRILL-8017: Fix LGTM Issues Relating to Equals and HashCode Functions (#2344)
b6da35e is described below
commit b6da35ece5a278a5ca72ecc33bafc98ba8f861f6
Author: Charles S. Givre <cg...@apache.org>
AuthorDate: Tue Oct 26 09:27:30 2021 -0400
DRILL-8017: Fix LGTM Issues Relating to Equals and HashCode Functions (#2344)
---
.../exec/store/mapr/TableFormatPluginConfig.java | 2 +-
.../store/mapr/db/MapRDBFormatPluginConfig.java | 30 ++++++++++++++++++++++
.../db/json/JsonTableRangePartitionFunction.java | 6 +++++
.../mapr/streams/StreamsFormatPluginConfig.java | 10 ++++++++
.../exec/store/kafka/KafkaPartitionScanSpec.java | 16 ++++++++++--
.../drill/exec/planner/cost/DrillCostBase.java | 20 ++++++++++++---
.../exec/planner/index/DrillIndexDefinition.java | 4 +--
.../drill/exec/planner/logical/StoragePlugins.java | 6 +++++
.../java/org/apache/drill/exec/record/DeadBuf.java | 2 +-
.../java/org/apache/drill/exec/schema/Field.java | 13 ++++++++++
.../drill/exec/store/log/LogFormatField.java | 5 ++++
.../exec/record/metadata/AbstractPropertied.java | 2 +-
.../drill/exec/record/metadata/TupleSchema.java | 7 +++++
.../drill/exec/util/JsonStringArrayList.java | 3 +--
.../apache/drill/exec/util/JsonStringHashMap.java | 2 +-
.../apache/drill/common/graph/AdjacencyList.java | 18 ++++++++++---
.../common/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 ffd3d92..0122254 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 @@ package org.apache.drill.exec.store.mapr;
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 c17696c..9d0605f 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
@@ -54,6 +54,36 @@ public class MapRDBFormatPluginConfig extends TableFormatPluginConfig {
}
@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;
if (readAllNumbersAsDouble != other.readAllNumbersAsDouble) {
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 c97d303..4fe656c 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;
@@ -107,6 +108,11 @@ public class JsonTableRangePartitionFunction extends AbstractRangePartitionFunct
}
@Override
+ public int hashCode() {
+ return Objects.hash(refList, tableName, userName, partitionKeyVector, startKeys, stopKeys);
+ }
+
+ @Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
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 1da795c..e78f1e8 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
@@ -32,6 +32,16 @@ public class StreamsFormatPluginConfig extends TableFormatPluginConfig {
}
@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 509f77a..a620ac8 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 org.apache.drill.common.FunctionNames;
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;
@@ -91,9 +94,18 @@ public class KafkaPartitionScanSpec {
}
@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 97ee688..5a44afb 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 @@ package org.apache.drill.exec.planner.cost;
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 class DrillCostBase implements DrillRelOptCost {
@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 d756ae0..bc6fe2d 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 class DrillIndexDefinition implements IndexDefinition {
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 3b2a04f..9ec9d9b 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.HashMap;
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 class StoragePlugins implements Iterable<Map.Entry<String, StoragePluginC
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 c3c3539..dcaf226 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 class DeadBuf extends ByteBuf {
}
@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 e7bef59..eee235c 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.common.types.TypeProtos.MajorType;
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;
@@ -92,6 +94,17 @@ public abstract class Field {
}
@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 8794838..f8f2b8c 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 class LogFormatField {
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 31bf83a..a04cdd9 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 class AbstractPropertied implements Propertied {
}
@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 0fd4a5a..b5cef5a 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.JsonProperty;
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;
/**
@@ -173,6 +175,11 @@ public class TupleSchema extends AbstractPropertied implements TupleMetadata {
}
@Override
+ public int hashCode() {
+ return Objects.hash(parentMap, nameSpace);
+ }
+
+ @Override
public List<MaterializedField> toFieldList() {
List<MaterializedField> cols = new ArrayList<>();
for (ColumnMetadata md : nameSpace) {
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 695befd..9af530f 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 @@ package org.apache.drill.exec.util;
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 337858f..8536822 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 @@ import com.fasterxml.jackson.datatype.joda.JodaModule;
* 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 acfca83..fd5f671 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.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 @@ class AdjacencyList<V extends GraphValue<V>> {
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 7c6cf58..4c1f7e2 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
@@ -41,6 +41,11 @@ public abstract class LogicalOperatorBase implements LogicalOperator{
}
@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.
}