[SPARK-30075][CORE][TESTS] Fix the hashCode implementation of ArrayKeyIndexType correctly #26709
Conversation
HeartSaVioR
changed the title
|
I'm also seeing possible spot to touch: ArrayWrappers. ComparableXXXArray implements its own hashCode, which would work as IDE creates similar code, but how about simply using If it also makes sense, I'll address it altogether in this PR. Thanks in advance! |
|
Test build #114605 has finished for PR 26709 at commit
|
|
Retest this, please |
|
Test build #114619 has finished for PR 26709 at commit
|
| @Override | ||
| public int hashCode() { | ||
| return key.hashCode(); | ||
| return Arrays.hashCode(key); |
There was a problem hiding this comment.
Given that equals() uses id too, how about Arrays.hashCode(key) ^ Arrays.hashCode(id)?
There was a problem hiding this comment.
That might be the reason of participating only key, but your suggestion would be symmetrical and no big deal (as it had been completely broken). Thanks for the suggestion! Applied.
|
Test build #114644 has finished for PR 26709 at commit
|
|
Retest this, please |
|
retest this please |
|
Test build #114664 has finished for PR 26709 at commit
|
|
Test build #114667 has finished for PR 26709 at commit
|
|
Merged to master |
|
Thanks all for reviewing and merging! |
…yIndexType correctly ### What changes were proposed in this pull request? This patch fixes the bug on ArrayKeyIndexType.hashCode() as it is simply calling Array.hashCode() which in turn calls Object.hashCode(). That should be Arrays.hashCode() to reflect the elements in the array. ### Why are the changes needed? I've encountered the bug in apache#25811 while adding test codes for apache#25811, and I've split the fix into individual PR to speed up reviewing. Without this patch, ArrayKeyIndexType would bring various issues when it's used as type of collections. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? I've skipped adding UT as ArrayKeyIndexType is in test and the patch is pretty simple one-liner. Closes apache#26709 from HeartSaVioR/SPARK-30075. Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]> Signed-off-by: Sean Owen <[email protected]>
4 participants
What changes were proposed in this pull request?
This patch fixes the bug on ArrayKeyIndexType.hashCode() as it is simply calling Array.hashCode() which in turn calls Object.hashCode(). That should be Arrays.hashCode() to reflect the elements in the array.
Why are the changes needed?
I've encountered the bug in #25811 while adding test codes for #25811, and I've split the fix into individual PR to speed up reviewing. Without this patch, ArrayKeyIndexType would bring various issues when it's used as type of collections.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
I've skipped adding UT as ArrayKeyIndexType is in test and the patch is pretty simple one-liner.