-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
Support for Merge in Integrated BlobDB with base values #8292
Conversation
fa7ef06
to
e582a81
Compare
@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
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.
Thanks for the PR @akankshamahajan15! This is awesome
e582a81
to
f3c2581
Compare
@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing. |
1 similar comment
@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing. |
7719d6d
to
29cf180
Compare
@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing. |
29cf180
to
9a1a28d
Compare
@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing. |
9a1a28d
to
d111120
Compare
@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing. |
d111120
to
10233b2
Compare
@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing. |
10233b2
to
9928766
Compare
@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing. |
@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
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.
Thanks again @akankshamahajan15 ! The patch looks good to me, just some minor comments
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:
Summary: Test Plan: Add new unit test Reviewers: Subscribers: Tasks: Tags:
Summary: Test Plan: Add new unit test Reviewers: Subscribers: Tasks: Tags:
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:
Summary: Test Plan: Add new test Reviewers: Subscribers: Tasks: Tags:
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:
Summary: Add Callback class to get the blob value and support DB::GetMergeOperands Test Plan: Add unit test Reviewers: Subscribers: Tasks: Tags:
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:
9928766
to
d0084ca
Compare
@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing. |
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:
d0084ca
to
6bb45ed
Compare
@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing. |
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:
@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing. |
@akankshamahajan15 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
@akankshamahajan15 merged this pull request in 3897ce3. |
Summary: This PR add support for Merge operation in Integrated BlobDB with base values(i.e DB::Put). Merged values can be retrieved through DB::Get, DB::MultiGet, DB::GetMergeOperands and Iterator operation.
Test Plan: Add new unit tests
Reviewers:
Subscribers:
Tasks:
Tags: