Skip to content
This repository was archived by the owner on Aug 16, 2023. It is now read-only.

Add API HasRawData() for get vector #828

Merged
merged 1 commit into from
Apr 18, 2023

Conversation

cydrain
Copy link
Collaborator

@cydrain cydrain commented Apr 18, 2023

No description provided.

@sre-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cydrain

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mergify mergify bot added the dco-passed DCO check passed. label Apr 18, 2023
@mergify
Copy link

mergify bot commented Apr 18, 2023

@cydrain Please associate the related issue to the body of your Pull Request. (eg. “issue: #”)

@cydrain
Copy link
Collaborator Author

cydrain commented Apr 18, 2023

/kind improvement

@@ -62,6 +62,15 @@ class DiskANNIndexNode : public IndexNode {
expected<DataSetPtr, Status>
GetVectorByIds(const DataSet& dataset, const Config& cfg) const override;

bool
HasRawData(const std::string& metric_type) const override {
if (metric_type == metric::L2) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just return metric_type == metric::L2?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

if constexpr (std::is_same<faiss::IndexBinaryIVF, T>::value) {
return true;
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If none of the above conditions are met, will there be a return value?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that will not happen

if constexpr (std::is_same_v<detail::raft_ivf_pq_index, T>) {
return false;
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If none of the above conditions are met, will there be a return value?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that will not happen

@cydrain cydrain force-pushed the caiyd_add_has_raw_data branch from 7a70fdf to 2ffc379 Compare April 18, 2023 09:12
@mergify mergify bot added ci-passed and removed ci-passed labels Apr 18, 2023
@cydrain cydrain force-pushed the caiyd_add_has_raw_data branch from 7c2ede8 to fc1e17b Compare April 18, 2023 10:25
@czs007
Copy link

czs007 commented Apr 18, 2023

/lgtm

@mergify mergify bot added the ci-passed label Apr 18, 2023
@sre-ci-robot sre-ci-robot merged commit 2fa9067 into milvus-io:main Apr 18, 2023
@cydrain cydrain deleted the caiyd_add_has_raw_data branch April 18, 2023 11:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants