-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-40315][SQL] Add equals() and hashCode() to ArrayBasedMapData #37771
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,8 @@ | |
|
|
||
| package org.apache.spark.sql.catalyst.util | ||
|
|
||
| import scala.util.hashing.MurmurHash3 | ||
|
|
||
| import java.util.{Map => JavaMap} | ||
|
|
||
| /** | ||
|
|
@@ -35,6 +37,29 @@ class ArrayBasedMapData(val keyArray: ArrayData, val valueArray: ArrayData) exte | |
| override def toString: String = { | ||
| s"keys: $keyArray, values: $valueArray" | ||
| } | ||
|
|
||
| override def equals(obj: Any): Boolean = { | ||
| if (obj == null && this == null) { | ||
| return true | ||
| } | ||
|
|
||
| if (obj == null || !obj.isInstanceOf[ArrayBasedMapData]) { | ||
| return false | ||
| } | ||
|
|
||
| val other = obj.asInstanceOf[ArrayBasedMapData] | ||
|
|
||
| keyArray.equals(other.keyArray) && valueArray.equals(other.valueArray) | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this part is also causing
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ideally
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure how we do it in other code, but there is Arrays.deepEquals for element-wise equality
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The To be consistent, I'll change it to use |
||
|
|
||
| // Hash this class as a Product of two hashCodes. We don't know the DataType which prevents us | ||
| // from getting individual rows for hashing as a Map. | ||
| override def hashCode(): Int = { | ||
| val seed = MurmurHash3.productSeed | ||
| val keyHash = scala.util.hashing.MurmurHash3.mix(seed, keyArray.hashCode()) | ||
| val valueHash = scala.util.hashing.MurmurHash3.mix(keyHash, valueArray.hashCode()) | ||
| scala.util.hashing.MurmurHash3.finalizeHash(valueHash, 2) | ||
| } | ||
|
Comment on lines
+50
to
+55
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @cloud-fan , can you take a look? Do you think we need to also add an explicit |
||
| } | ||
|
|
||
| object ArrayBasedMapData { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how can
thisbe null?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think IDEA can generate equals and hashCode implementations pretty well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right,
thiscan't be null. I'll remove this check.The IDEA defaults in this case are not super helpful because they just defer to the parent class
super.equals(obj).