[Python] support BucketByteBuffer and BucketUUID

[jun-he]

Follow up #2689 (comment)

[jun-he]

@TGooch44 and @rymurr can you help to review it? Thanks.

[rymurr]

Thanks @jun-he ! This looks good. However I am worried about #2837 and if python and Java might return different results. What do you think?

[TGooch44]

Thanks @jun-he ! This looks good. However I am worried about #2837 and if python and Java might return different results. What do you think?

It looks like the following matches the expected output(which I guess is different than the Java output):

>>> import mmh3
>>> (mmh3.hash("💰".encode("utf-8")) & 2147483647) % 32
12

[jun-he]

@rymurr @TGooch44 they might mismatch. I will take a look at all the hashcode generated in Python versus Java. This might be a problem for many.

[rymurr]

I guess python is doing the 'right' thing. The question for me is: will Java start doing the right thing or maintain the wrong thing for backwards compatibility. Python should, I think, be consistent w/ Java.

[TGooch44]

@rymurr Unfortunately, I think you're right that matching Java is more important than being correct according to spec. We may want to wait on this until the java community decides if they are going to fix this or not. Seems like they're trying to decide how likely existing users are to have been impacted.

…

On Tue, Jul 20, 2021, 2:06 AM Ryan Murray ***@***.***> wrote: I guess python is doing the 'right' thing. The question for me is: will Java start doing the right thing or maintain the wrong thing for backwards compatibility. Python should, I think, be consistent w/ Java. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2836 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMETHZDOQENBDO5ZRB3JCLTYU4ANANCNFSM5ARSWF7A> .

[findepi]

Elevating the 'wrong' behavior to be 'the standard' will be hard for non-Java languages.
Please take a look at my proposed fix in Guava google/guava#5649 for this.
In order to match Iceberg Java implementation, one would perhaps need to translate that code (without the fix) into Python.

Waiting for Java bucketing version to be fixed seems reasonable though. This avoids correctness issues -- no support for bucketing in Python, means no bugs at all.

FWIW, in Trino, bucketing did not have the bug (that's how we found #2837), so 'correctly' bucketed data can be out there too.

[rymurr]

Elevating the 'wrong' behavior to be 'the standard' will be hard for non-Java languages.

I agree. I think the last thing we want to do is start duplicating guava bugs in other languages. It is hard and unnecessary.

Shall we wait for the guava fix to be merged and propagated to Iceberg then?

[TGooch44]

Waiting seems like the best option here.

[findepi]

Shall we wait for the guava fix to be merged and propagated to Iceberg then?

Per @rdblue 's #2837 (comment) i posted a proposed fix #2849.
If it gets merged, the work here would be unblocked.

[jun-he]

Thanks @findepi for the fix.

@TGooch44 @rymurr I think python should just hash the UTF-8 bytes. I will run a quick check to see if Java and Python match in various cases.

[findepi]

code lgtm

is there any place in tests where hash values could be asserted?

[jun-he]

@findepi I added additional tests to check the hash values.
It matched the result in Java test