Support Java integers hashing in javaHash
#41131
Conversation
evillique
added
the
can be tested
JackyWoo
changed the title
javaHash
JackyWoo
changed the title
javaHashjavaHash
|
@alexey-milovidov Pls review the PR, when you have a changce. |
src/Functions/FunctionsHashing.h
Outdated
| static ReturnType apply(int64_t x) | ||
| { | ||
| int64_t copy = x; | ||
| copy = copy >> 32; |
There was a problem hiding this comment.
I'm not confident about the correctness of this code:
For unsigned a and for signed and non-negative a, the value of a >> b is the integer part of a/2b.
For negative a, the value of a >> b is implementation-defined (in most implementations, this performs arithmetic right shift, so that the result remains negative).
https://en.cppreference.com/w/cpp/language/operator_arithmetic
There was a problem hiding this comment.
A random page on the internet is saying that
>>>is an unsigned right shift operator
that is different from your code.
There was a problem hiding this comment.
- According the doc, Since C++20
>>is arithmetic right shift
Since C++20
The value of a >> b is a/2b, rounded down (in other words, right shift on signed a is arithmetic right shift).
>>>is implemented bycopy >> 32andcopy & 0x00000000FFFFFFFF
| 123 | ||
| 122 | ||
| -539222985 | ||
| -539222986 |
There was a problem hiding this comment.
Is there any online playground or a quick code snippet to validate that this is definitely the same result as Java gives?
There was a problem hiding this comment.
Unfortunately I did not find any online tool. But we can generate Java hash code simplly by Long.hashCode().
| { | ||
| const size_t size = sizeof(T); | ||
| const char * data = reinterpret_cast<const char *>(&x); | ||
| return apply(data, size); |
There was a problem hiding this comment.
Also not sure about this code.
Now we apply javaHash for unsigned integers as for strings for bytes in the native byte order.
How unsigned integers are typically represented in Java?
If they are represented as BigInt, let's check what the behavior should be.
If they don't exist, let's throw an exception.
There was a problem hiding this comment.
Thanks for advice. Java does not have unsigned types.
|
Ok, and let's update the test, and it is ready... |
|
The test still has to be updated. |
|
Still some test failures but may be unrelated. |
Changelog category (leave one):
Usage scenario
We use Spark to import data into CH local table and data is distributed by
id.hashcode() % shard_num. And now we want to import via CH distributed table.So we need a Java int hash function .
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Support Java integers hashing in
javaHash.