expression: POC implementation of Vitess hashing algorithm. #20972
Conversation
ti-srebot
added
the
first-time-contributor
|
|
|
No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md |
|
The build is failing as it depends on changes to tipb and parser to be merged first. The links to respective PRs are in description. |
bezmax
changed the title
|
No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md |
|
Please follow PR Title Format:
Or if the count of mainly changed packages are more than 3, use
|
ghost
changed the title
| } | ||
|
|
||
| // evalString evals VITESS_HASH(int64|string|decimal). | ||
| func (b *builtinVitessHashSig) evalString(row chunk.Row) (string, bool, error) { |
There was a problem hiding this comment.
Should we implement the vectorized expression function for builtinVitessHashSig in expression/builtin_miscellaneous_vec.go?
There was a problem hiding this comment.
@Reminiscent Could you explain what are "vectorized expression functions" in this context?
There was a problem hiding this comment.
@Reminiscent Could you explain what are "vectorized expression functions" in this context?
You can see this blog for more information. In short, the expression function builtinVitessHashSig.evalString here processes one row of data at a time. And if you implement the vectorized expression function for builtinVitessHashSig.vecEvalString in expression/builtin_miscellaneous_vec.go, it can process a batch at a time. The blog introduces more information.
| defer s.cleanEnv(c) | ||
| tk := testkit.NewTestKit(c, s.store) | ||
| tk.MustExec("use test") | ||
| tk.MustQuery("select vitess_hash(30375298039) from t").Check(testkit.Rows("\x03\x12\x65\x66\x1E\x5F\x11\x33")) |
There was a problem hiding this comment.
Should we create a table t first?
There was a problem hiding this comment.
I did not create t for the reason that there are many other function tests that do not do that. For example tests for str_to_date, hour, time, etc. If you think it's important - I will add t.
There was a problem hiding this comment.
I think it's better to add some statements like
tk.MustExec("drop table if exists t;")
tk.MustExec("create table t(a int);")
for safty.
ti-chi-bot
added
the
needs-rebase
|
@bezmax: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@bezmax, please update your pull request. |
1 similar comment
|
@bezmax, please update your pull request. |
|
@bezmax PR closed due to no update for a long time. Feel free to reopen it anytime. |
|
Would like this PR to be re-opened. Thanks! |
|
/reopen |
|
@kolbe: Failed to re-open PR: state cannot be changed. The bezmax-vitess-hash branch was force-pushed or recreated. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Given this PR got closed, I re-created it with all the fixes applied here: #23493 |
What problem does this PR solve?
Issue Number: close #20971
Problem Summary: Implementing vitess_hash function to support efficient migration between tidb <-> vitess.
What is changed and how it works?
Proposal: #20971
What's Changed: New builtin_misc function added implementing vitess_hash algorithm.
Related changes
In order PRs need to be merged:
Check List
Tests
Release note