Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support upsert #53

Merged
merged 1 commit into from
Dec 19, 2022
Merged

Support upsert #53

merged 1 commit into from
Dec 19, 2022

Conversation

smellthemoon
Copy link
Contributor

@smellthemoon smellthemoon commented Nov 23, 2022 •
edited
Loading

Signed-off-by: lixinguo [email protected]
#54

@mergify mergify bot added the dco-passed DCO check passed. label Nov 23, 2022
@mergify
Copy link

mergify bot commented Nov 23, 2022

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

yah01
yah01 reviewed Nov 23, 2022
View reviewed changes
proto/milvus.proto Outdated Show resolved Hide resolved
@mergify mergify bot added needs-dco DCO is missing in this pull request. and removed dco-passed DCO check passed. labels Nov 23, 2022
@mergify mergify bot removed the ci-passed label Nov 23, 2022
@mergify mergify bot added dco-passed DCO check passed. ci-passed and removed needs-dco DCO is missing in this pull request. labels Nov 23, 2022
@smellthemoon
Copy link
Contributor Author

/hold

@smellthemoon smellthemoon changed the title Support update Support upsert Dec 5, 2022
@smellthemoon
Copy link
Contributor Author

/unhold

czs007
czs007 reviewed Dec 15, 2022
View reviewed changes
proto/milvus.proto
common.MsgBase base = 1;
string db_name = 2;
string collection_name = 3;
string partition_name = 4;
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the semantics of upsert when the partition parameter is specified?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It follows the partition semantic logic of delete and insert.

czs007
czs007 reviewed Dec 16, 2022
View reviewed changes
proto/common.proto Outdated
PrivilegeSelectOwnership = 22;
PrivilegeManageOwnership = 23;
PrivilegeSelectUser = 24;
PrivilegeUpsert = 10;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Define PribilegeUpsert as 25 instead of 10.
Do not modify previously defined value.

Support update
3a7a4fb 
Signed-off-by: lixinguo <[email protected]>
@czs007
Copy link
Collaborator

czs007 commented Dec 19, 2022

/approve
/lgtm

@sre-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: czs007, smellthemoon

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

@sre-ci-robot sre-ci-robot merged commit e6bda72 into milvus-io:master Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@czs007 czs007 czs007 left review comments

@yah01 yah01 yah01 left review comments

@xiaofan-luan xiaofan-luan Awaiting requested review from xiaofan-luan

Assignees

@czs007 czs007

Labels
approved ci-passed dco-passed DCO check passed. lgtm size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@smellthemoon @czs007 @sre-ci-robot @yah01