-
Notifications
You must be signed in to change notification settings - Fork 411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
compute: Add Vector data type and functions #9262
compute: Add Vector data type and functions #9262
Conversation
* Add Vector data type (pingcap#141) Signed-off-by: Wish <breezewish@outlook.com> * compute: Add vector functions (pingcap#146) Signed-off-by: Wish <breezewish@outlook.com> * compute: Fix vector precision (pingcap#147) Signed-off-by: Wish <breezewish@outlook.com> * compute: Fix vector distance over NULLs (pingcap#148) Signed-off-by: Wish <breezewish@outlook.com> * [cherry-pick] Storages: Make DMFile ready for new column indexes/types (pingcap#149) Signed-off-by: Wish <breezewish@outlook.com> --------- Signed-off-by: Wish <breezewish@outlook.com> Co-authored-by: Wenxuan <breezewish@outlook.com>
/run-all-tests |
1 similar comment
/run-all-tests |
Could also manually verify whether it works with these two PRs? Thank you! As in pingcap/tidb#54635 the function push down is not enabled yet, so we only need to verify TiFlash could store and read out vector data correctly. |
if (!isNullAt(n)) | ||
return getNestedColumn().getDataAt(n); | ||
|
||
throw Exception( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error code and error message should be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is by design
normb += b[i] * b[i]; | ||
} | ||
|
||
Float64 similarity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should guarantee std::sqrt(static_cast<Float64>(norma) * static_cast<Float64>(normb))
is non-zero?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In c++, float / 0 = inf
and 0.0 / 0 = nan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If norma
or normb
equal to 0, distance
must be 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add some comments.
14e5df0
to
ab8149a
Compare
Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
/test pull-unit-test |
@Lloyd-Pottiger you can revert the commit: feb28b7 I fixed it in PingCAP-QE/ci#3042 |
This reverts commit feb28b7.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: breezewish, JaySon-Huang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
0856586
into
pingcap:feature/vector-index
What problem does this PR solve?
Issue Number: ref #9032
Problem Summary:
What is changed and how it works?
Pick https://github.com/tidbcloud/tiflash-cse/pull/153
Check List
Tests
Side effects
Documentation
Release note