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.

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

fix(server): hmget non uniqe keys response #4745

Merged
merged 2 commits into from
Mar 12, 2025
Merged

fix(server): hmget non uniqe keys response #4745

merged 2 commits into from
Mar 12, 2025

Conversation

adiholden
Copy link
Collaborator

fixes #4743

Verified

This commit was signed with the committer’s verified signature.
albertvillanova Albert Villanova del Moral
Signed-off-by: adi_holden <adi@dragonflydb.io>
@adiholden adiholden requested a review from romange March 11, 2025 07:24
@@ -408,10 +408,14 @@ OpResult<vector<OptStr>> OpHMGet(const OpArgs& op_args, std::string_view key, Cm
if (pv.Encoding() == kEncodingListPack) {
uint8_t* lp = (uint8_t*)pv.RObjPtr();

absl::flat_hash_map<string_view, unsigned> reverse;
absl::flat_hash_map<string_view, std::vector<size_t>> reverse;
Copy link
Collaborator

Choose a reason for hiding this comment

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

it's gonna be more cpu intensive because for 99.999% of the usecases vector is not needed.
Can you please use InlinedVector instead?

reverse.reserve(fields.size() + 1);
for (size_t i = 0; i < fields.size(); ++i) {
reverse.emplace(ArgS(fields, i), i); // map fields to their index.
auto [res_it, is_inserted] =
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can achieve the same with reverse[ArgS(fields, i)].push_back(i)

Verified

This commit was signed with the committer’s verified signature.
albertvillanova Albert Villanova del Moral
Signed-off-by: adi_holden <adi@dragonflydb.io>
@adiholden adiholden requested a review from romange March 12, 2025 11:58
@adiholden adiholden enabled auto-merge (squash) March 12, 2025 12:50
@adiholden adiholden merged commit 0e35f78 into main Mar 12, 2025
10 checks passed
@adiholden adiholden deleted the fix_4743 branch March 12, 2025 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] HMGET with none-unique keys
2 participants