amogh-jahagirdar commented on code in PR #11764:
URL: https://github.com/apache/iceberg/pull/11764#discussion_r1887057599
##########
api/src/main/java/org/apache/iceberg/types/Types.java:
##########
@@ -824,7 +827,10 @@ public boolean equals(Object o) {
@Override
public int hashCode() {
- return Objects.hash(NestedField.class, Arrays.hashCode(fields));
+ if (hashCode == NO_HASHCODE) {
+ hashCode = Objects.hash(NestedField.class, Arrays.hashCode(fields));
+ }
+ return hashCode;
Review Comment:
Agreed, I don't think it's worth the complexity of double checked locking to
avoid a little bit of redundant computation in the multi-threaded case.
##########
api/src/main/java/org/apache/iceberg/types/Types.java:
##########
@@ -723,6 +723,9 @@ public int hashCode() {
public static class StructType extends NestedType {
private static final Joiner FIELD_SEP = Joiner.on(", ");
+ private static final int NO_HASHCODE = Integer.MIN_VALUE;
+
+ private transient int hashCode = NO_HASHCODE;
Review Comment:
Thanks for making this transient, when I saw this PR that's the one aspect I
wanted to make sure was the case. We really don't want any weird issues that
will happen in a distributed setting where the cached hashcode on one struct
type is different than the hashcode for the same struct type that's on a
different node (which can easily happen since hashcodes across JVMS can be
different)
Making it transient will avoid all those kinds of issues.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
