MDEV-38180: Add xxh32(), xxh3(), and xxh3_128() SQL functions#4741
MDEV-38180: Add xxh32(), xxh3(), and xxh3_128() SQL functions#4741monthdev wants to merge 3 commits intoMariaDB:mainfrom
Conversation
gkodinov
left a comment
There was a problem hiding this comment.
This is a preliminary review. Thank you for your contribution.
I think you need to figure out what is the fate of the other PR. As yours seems to mostly be a fork of it (I didn't compare, but it looks very similar). I will add Daniel as a reviewer to this one so he can work on that.
Should we decide to go ahead, please make sure your commit message must be present and must follow CODING_STANDARDS.md
Merge with main
|
@gkodinov I appreciate the review! In my latest commit, I
Please let me know of any other problems or nits! |
|
Hi, again! I looked at the failing checks, and they look unrelated to my PR to the extent that the tests do not fail on my new test file |
There was a problem hiding this comment.
Please use the my_hasher_st interface which is used by partitioning. It is collation-aware and the sql functions should be consistent with which partitions the string goes to.
Please add tests regarding collation equality too. A simple one could be something like checking that xxh3(' ') = xxh3(' '), given that the two strings are equal (and go to the same partition with PARTITION BY [LINEAR] KEY):
MariaDB [test]> select '' = ' ';
+------------+
| '' = ' ' |
+------------+
| 1 |
+------------+
1 row in set (0.001 sec)
P.S. It's not clear to me the context of choosing between proceeding with this PR vs #4500 given the latter is still open though appearing dormant. The review comment is independent of that.
|
@mariadb-YuchenPei Thanks for the input! It was my mistake opening this PR. I explained that on Zulip #general > MDEV-38180 GSoC 2026 @ 💬#general > MDEV-38180 GSoC 2026 @ 💬. But then I got a code review from another maintainer, so I got confused. Please disregard my PR in favor of #4500 which was opened first! I am not trying to take someone else's PR. |
|
@monthdev, I spoke to @grooverdan. We were going to wait for the #4500 contributor to update their work based on some current input. They haven't done so and the patch in its current form cannot be merged. We appreciate your courtesy in giving the opportunity for the #4500 author to complete their work. I think a reasonable amount of time has passed, and if you are willing to address and complete this PR, then ultimately the users of MariaDB will benefit from your labour. |
Address MDEV-38180.
There is already an open PR for this here #4500. I am just assuming it was dropped and reiterating with a narrower scope.