Apache9 commented on code in PR #7575:
URL: https://github.com/apache/hbase/pull/7575#discussion_r2649109173
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java:
##########
@@ -111,5 +111,28 @@ public String toString() {
return "class:" + className + ", jarPath:" + jarPath + ", priority:" +
priority
+ ", properties:" + properties;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (obj == null || !(obj instanceof CoprocessorDescriptor)) {
+ return false;
+ }
+ CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
+ if (
+ priority != other.getPriority() || !Objects.equals(className,
other.getClassName())
+ || !Objects.equals(getJarPath(), other.getJarPath())
+ ) {
+ return false;
+ }
+ return new TreeMap<>(getProperties()).equals(new
TreeMap<>(other.getProperties()));
Review Comment:
I think we could use equals directly here?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java:
##########
@@ -133,8 +133,8 @@ protected void preflightChecks(MasterProcedureEnv env,
Boolean enabled) throws H
"Cannot add or remove column families when this modification " +
"won't reopen regions.");
}
if (
- this.unmodifiedTableDescriptor.getCoprocessorDescriptors().hashCode()
- !=
this.modifiedTableDescriptor.getCoprocessorDescriptors().hashCode()
+ !new
HashSet<>(this.unmodifiedTableDescriptor.getCoprocessorDescriptors())
Review Comment:
This is not on the critial read/write path so maybe this is acceptable, but
a better way, is to first check size, and then, create a HashSet for one of the
descriptors, and then iterator over the other one, and check whethere the
HashSet contains it.
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java:
##########
@@ -111,5 +111,28 @@ public String toString() {
return "class:" + className + ", jarPath:" + jarPath + ", priority:" +
priority
+ ", properties:" + properties;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (obj == null || !(obj instanceof CoprocessorDescriptor)) {
+ return false;
+ }
+ CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
+ if (
+ priority != other.getPriority() || !Objects.equals(className,
other.getClassName())
+ || !Objects.equals(getJarPath(), other.getJarPath())
+ ) {
+ return false;
+ }
+ return new TreeMap<>(getProperties()).equals(new
TreeMap<>(other.getProperties()));
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(className, jarPath, priority, new
TreeMap<>(properties));
Review Comment:
Better use Objects.hash to calculate hashCode for fields other than
properties, and then use stream.sorted to iterator over properties and
calculate hashCode on the fly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
