From d626fa5cbd58af62d5d737ab90a009784a7f5550 Mon Sep 17 00:00:00 2001 From: longfar Date: Thu, 21 Dec 2023 22:25:20 +0800 Subject: [PATCH 01/13] feat: hrandfield -- without count and values --- src/base_cmd.h | 1 + src/cmd_hash.cc | 64 ++++++++++++++++++++++++++++++++++++++++ src/cmd_hash.h | 14 +++++++++ src/cmd_table_manager.cc | 1 + 4 files changed, 80 insertions(+) diff --git a/src/base_cmd.h b/src/base_cmd.h index ba720d7ef..b0d3bb7f8 100644 --- a/src/base_cmd.h +++ b/src/base_cmd.h @@ -68,6 +68,7 @@ const std::string kCmdNameHGetAll = "hgetall"; const std::string kCmdNameHKeys = "hkeys"; const std::string kCmdNameHLen = "hlen"; const std::string kCmdNameHStrLen = "hstrlen"; +const std::string kCmdNameHRandField = "hrandfield"; enum CmdFlags { kCmdFlagsWrite = (1 << 0), // May modify the dataset diff --git a/src/cmd_hash.cc b/src/cmd_hash.cc index 25dc40e43..8d8d702fb 100644 --- a/src/cmd_hash.cc +++ b/src/cmd_hash.cc @@ -7,6 +7,7 @@ #include "cmd_hash.h" +#include "pstd/pstd_string.h" #include "store.h" namespace pikiwidb { @@ -285,4 +286,67 @@ void HStrLenCmd::DoCmd(PClient* client) { client->AppendStringRaw(reply.ReadAddr()); } +HRandFieldCmd::HRandFieldCmd(const std::string& name, int16_t arity) + : BaseCmd(name, arity, kCmdFlagsReadonly, kAclCategoryRead | kAclCategoryHash) {} + +bool HRandFieldCmd::DoInitial(PClient* client) { + if (client->argv_.size() > 4) { + client->SetRes(CmdRes::kSyntaxErr); + return false; + } + client->SetKey(client->argv_[1]); + return true; +} + +void HRandFieldCmd::DoCmd(PClient* client) { + // parse arguments + std::optional count{std::nullopt}; + bool with_values{false}; + bool allow_repeat{false}; + if (client->argv_.size() > 2) { + int32_t cnt{}; + if (pstd::String2int(client->argv_[2], &cnt) == 0) { + client->SetRes(CmdRes::kInvalidInt); + return; + } + if (cnt < 0) { + allow_repeat = true; + cnt = -cnt; + } + count = std::make_optional(cnt); + + if (client->argv_.size() > 3) { + if (kWithValueString != pstd::StringToLower(client->argv_[3])) { + client->SetRes(CmdRes::kSyntaxErr); + return; + } + with_values = true; + } + } + + // get hash + PObject* value = nullptr; + PError err = PSTORE.GetValueByType(client->Key(), value, kPTypeHash); + if (err != kPErrorOK) { + if (err == kPErrorNotExist) { + client->AppendString(""); + } else { + client->SetRes(CmdRes::kSyntaxErr, "hrandfield cmd error"); + } + return; + } + auto hash = value->CastHash(); + if (hash->empty()) { + client->AppendString(""); + return; + } + + // fetch field(s) and reply + if (count) { + } else { + auto it = std::next(hash->begin(), rand() % hash->size()); + client->AppendString(it->first); + } +} + } // namespace pikiwidb diff --git a/src/cmd_hash.h b/src/cmd_hash.h index 16a820ff6..df6d02e14 100644 --- a/src/cmd_hash.h +++ b/src/cmd_hash.h @@ -99,4 +99,18 @@ class HStrLenCmd : public BaseCmd { void DoCmd(PClient *client) override; }; +class HRandFieldCmd : public BaseCmd { + public: + HRandFieldCmd(const std::string &name, int16_t arity); + + protected: + bool DoInitial(PClient *client) override; + + private: + void DoCmd(PClient *client) override; + auto ParseOptions(PClient *client, int32_t *count, bool *with_value) -> bool; + + static const inline std::string kWithValueString{"withvalues"}; +}; + } // namespace pikiwidb diff --git a/src/cmd_table_manager.cc b/src/cmd_table_manager.cc index 606e8b08a..675a218c1 100644 --- a/src/cmd_table_manager.cc +++ b/src/cmd_table_manager.cc @@ -71,6 +71,7 @@ void CmdTableManager::InitCmdTable() { ADD_COMMAND(HKeys, 2); ADD_COMMAND(HLen, 2); ADD_COMMAND(HStrLen, 3); + ADD_COMMAND(HRandField, -2); } std::pair CmdTableManager::GetCommand(const std::string& cmdName, PClient* client) { From 55f1e87949ccedbf7d9cbd2d6525126d113c6213 Mon Sep 17 00:00:00 2001 From: longfar Date: Thu, 21 Dec 2023 23:43:04 +0800 Subject: [PATCH 02/13] feat: hrandfield -- support positive count --- src/cmd_hash.cc | 42 ++++++++++++++++++++++++++++++++---------- src/cmd_hash.h | 3 ++- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/cmd_hash.cc b/src/cmd_hash.cc index 8d8d702fb..3740109dc 100644 --- a/src/cmd_hash.cc +++ b/src/cmd_hash.cc @@ -300,20 +300,13 @@ bool HRandFieldCmd::DoInitial(PClient* client) { void HRandFieldCmd::DoCmd(PClient* client) { // parse arguments - std::optional count{std::nullopt}; + int64_t count{}; bool with_values{false}; - bool allow_repeat{false}; if (client->argv_.size() > 2) { - int32_t cnt{}; - if (pstd::String2int(client->argv_[2], &cnt) == 0) { + if (pstd::String2int(client->argv_[2], &count) == 0) { client->SetRes(CmdRes::kInvalidInt); return; } - if (cnt < 0) { - allow_repeat = true; - cnt = -cnt; - } - count = std::make_optional(cnt); if (client->argv_.size() > 3) { if (kWithValueString != pstd::StringToLower(client->argv_[3])) { @@ -342,11 +335,40 @@ void HRandFieldCmd::DoCmd(PClient* client) { } // fetch field(s) and reply - if (count) { + if (client->argv_.size() > 2) { + DoWithCount(client, hash, count, with_values); } else { auto it = std::next(hash->begin(), rand() % hash->size()); client->AppendString(it->first); } } +void HRandFieldCmd::DoWithCount(PClient* client, const PHash* hash, int64_t count, bool with_value) { + if (count >= 0) { + if (hash->size() <= count) { // reply all fields + client->AppendArrayLen(with_value ? hash->size() * 2 : hash->size()); + for (auto&& kv : *hash) { + client->AppendString(kv.first); + if (with_value) { + client->AppendString(kv.second); + } + } + } else { // reply [count] fields + std::vector> kvs; + for (auto&& kv : *hash) { + kvs.push_back(kv); + } + std::random_shuffle(kvs.begin(), kvs.end()); + + client->AppendArrayLen(with_value ? count * 2 : count); + for (size_t i = 0; i < count; i++) { + client->AppendString(kvs[i].first); + if (with_value) { + client->AppendString(kvs[i].second); + } + } + } + } +} + } // namespace pikiwidb diff --git a/src/cmd_hash.h b/src/cmd_hash.h index df6d02e14..1769e55ec 100644 --- a/src/cmd_hash.h +++ b/src/cmd_hash.h @@ -8,6 +8,7 @@ #pragma once #include "base_cmd.h" +#include "hash.h" namespace pikiwidb { @@ -108,7 +109,7 @@ class HRandFieldCmd : public BaseCmd { private: void DoCmd(PClient *client) override; - auto ParseOptions(PClient *client, int32_t *count, bool *with_value) -> bool; + void DoWithCount(PClient *client, const PHash *hash, int64_t count, bool with_value); static const inline std::string kWithValueString{"withvalues"}; }; From c3bccc34725edf8d7b68ee0c910c71817bb8f781 Mon Sep 17 00:00:00 2001 From: longfar Date: Fri, 22 Dec 2023 00:11:47 +0800 Subject: [PATCH 03/13] feat: hrandfield -- support negative count --- src/cmd_hash.cc | 77 ++++++++++++++++++++++++++++++++----------------- src/cmd_hash.h | 3 +- 2 files changed, 52 insertions(+), 28 deletions(-) diff --git a/src/cmd_hash.cc b/src/cmd_hash.cc index 3740109dc..1ae2d630e 100644 --- a/src/cmd_hash.cc +++ b/src/cmd_hash.cc @@ -290,10 +290,14 @@ HRandFieldCmd::HRandFieldCmd(const std::string& name, int16_t arity) : BaseCmd(name, arity, kCmdFlagsReadonly, kAclCategoryRead | kAclCategoryHash) {} bool HRandFieldCmd::DoInitial(PClient* client) { - if (client->argv_.size() > 4) { - client->SetRes(CmdRes::kSyntaxErr); - return false; - } + /* + * There should not be quantity detection here, + * because the quantity detection of redis is after the COUNT integer detection. + */ + // if (client->argv_.size() > 4) { + // client->SetRes(CmdRes::kSyntaxErr); + // return false; + // } client->SetKey(client->argv_[1]); return true; } @@ -308,6 +312,11 @@ void HRandFieldCmd::DoCmd(PClient* client) { return; } + if (client->argv_.size() > 4) { + client->SetRes(CmdRes::kSyntaxErr); + return false; + } + if (client->argv_.size() > 3) { if (kWithValueString != pstd::StringToLower(client->argv_[3])) { client->SetRes(CmdRes::kSyntaxErr); @@ -336,39 +345,53 @@ void HRandFieldCmd::DoCmd(PClient* client) { // fetch field(s) and reply if (client->argv_.size() > 2) { - DoWithCount(client, hash, count, with_values); + if (count >= 0) { + DoWithPositiveCount(client, hash, count, with_values); + } else { + DoWithNegativeCount(client, hash, count, with_values); + } } else { auto it = std::next(hash->begin(), rand() % hash->size()); client->AppendString(it->first); } } -void HRandFieldCmd::DoWithCount(PClient* client, const PHash* hash, int64_t count, bool with_value) { - if (count >= 0) { - if (hash->size() <= count) { // reply all fields - client->AppendArrayLen(with_value ? hash->size() * 2 : hash->size()); - for (auto&& kv : *hash) { - client->AppendString(kv.first); - if (with_value) { - client->AppendString(kv.second); - } +void HRandFieldCmd::DoWithPositiveCount(PClient* client, const PHash* hash, int64_t count, bool with_value) { + if (hash->size() <= count) { // reply all fields + client->AppendArrayLen(with_value ? hash->size() * 2 : hash->size()); + for (auto&& kv : *hash) { + client->AppendString(kv.first); + if (with_value) { + client->AppendString(kv.second); } - } else { // reply [count] fields - std::vector> kvs; - for (auto&& kv : *hash) { - kvs.push_back(kv); - } - std::random_shuffle(kvs.begin(), kvs.end()); - - client->AppendArrayLen(with_value ? count * 2 : count); - for (size_t i = 0; i < count; i++) { - client->AppendString(kvs[i].first); - if (with_value) { - client->AppendString(kvs[i].second); - } + } + } else { // reply [count] fields + std::vector> kvs; + for (auto&& kv : *hash) { + kvs.push_back(kv); + } + std::random_shuffle(kvs.begin(), kvs.end()); + + client->AppendArrayLen(with_value ? count * 2 : count); + for (size_t i = 0; i < count; i++) { + client->AppendString(kvs[i].first); + if (with_value) { + client->AppendString(kvs[i].second); } } } } +void HRandFieldCmd::DoWithNegativeCount(PClient* client, const PHash* hash, int64_t count, bool with_value) { + count = -count; + client->AppendArrayLen(with_value ? count * 2 : count); + while (count--) { + auto it = std::next(hash->begin(), rand() % hash->size()); + client->AppendString(it->first); + if (with_value) { + client->AppendString(it->second); + } + } +} + } // namespace pikiwidb diff --git a/src/cmd_hash.h b/src/cmd_hash.h index 1769e55ec..7bc193990 100644 --- a/src/cmd_hash.h +++ b/src/cmd_hash.h @@ -109,7 +109,8 @@ class HRandFieldCmd : public BaseCmd { private: void DoCmd(PClient *client) override; - void DoWithCount(PClient *client, const PHash *hash, int64_t count, bool with_value); + void DoWithPositiveCount(PClient *client, const PHash *hash, int64_t count, bool with_value); + void DoWithNegativeCount(PClient *client, const PHash *hash, int64_t count, bool with_value); static const inline std::string kWithValueString{"withvalues"}; }; From e81497837fa84be39f3f9f14a81cd207d5436edc Mon Sep 17 00:00:00 2001 From: longfar Date: Fri, 22 Dec 2023 09:22:44 +0800 Subject: [PATCH 04/13] fix: compile error --- src/cmd_hash.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd_hash.cc b/src/cmd_hash.cc index 1ae2d630e..35d765533 100644 --- a/src/cmd_hash.cc +++ b/src/cmd_hash.cc @@ -314,7 +314,7 @@ void HRandFieldCmd::DoCmd(PClient* client) { if (client->argv_.size() > 4) { client->SetRes(CmdRes::kSyntaxErr); - return false; + return; } if (client->argv_.size() > 3) { From 518db20d6c59757020327286d2bf5fbea7706e47 Mon Sep 17 00:00:00 2001 From: longfar Date: Fri, 22 Dec 2023 17:39:23 +0800 Subject: [PATCH 05/13] fix: use std::shuffle instead of std::random_shuffle --- src/cmd_hash.cc | 2 +- src/cmd_hash.h | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cmd_hash.cc b/src/cmd_hash.cc index 35d765533..9271e7c98 100644 --- a/src/cmd_hash.cc +++ b/src/cmd_hash.cc @@ -370,7 +370,7 @@ void HRandFieldCmd::DoWithPositiveCount(PClient* client, const PHash* hash, int6 for (auto&& kv : *hash) { kvs.push_back(kv); } - std::random_shuffle(kvs.begin(), kvs.end()); + std::shuffle(kvs.begin(), kvs.end(), std::mt19937(rd_())); client->AppendArrayLen(with_value ? count * 2 : count); for (size_t i = 0; i < count; i++) { diff --git a/src/cmd_hash.h b/src/cmd_hash.h index 7bc193990..505ae9997 100644 --- a/src/cmd_hash.h +++ b/src/cmd_hash.h @@ -7,6 +7,8 @@ #pragma once +#include + #include "base_cmd.h" #include "hash.h" @@ -112,6 +114,8 @@ class HRandFieldCmd : public BaseCmd { void DoWithPositiveCount(PClient *client, const PHash *hash, int64_t count, bool with_value); void DoWithNegativeCount(PClient *client, const PHash *hash, int64_t count, bool with_value); + std::random_device rd_; + static const inline std::string kWithValueString{"withvalues"}; }; From ed4256b78e7f6827496301e1c3bdc4694cb4f242 Mon Sep 17 00:00:00 2001 From: longfar Date: Sat, 6 Jan 2024 16:58:44 +0800 Subject: [PATCH 06/13] feat: implement HRandField API in blackwidow and apply it in cmd structure --- src/cmd_hash.cc | 90 +++++++-------------------- src/cmd_hash.h | 6 +- src/storage/include/storage/storage.h | 3 + src/storage/src/redis_hashes.cc | 53 ++++++++++++++++ src/storage/src/redis_hashes.h | 1 + src/storage/src/storage.cc | 4 ++ 6 files changed, 85 insertions(+), 72 deletions(-) diff --git a/src/cmd_hash.cc b/src/cmd_hash.cc index 14c33782c..a9c033a15 100644 --- a/src/cmd_hash.cc +++ b/src/cmd_hash.cc @@ -9,8 +9,8 @@ #include -#include "store.h" #include "pstd/pstd_string.h" +#include "store.h" namespace pikiwidb { @@ -246,21 +246,21 @@ bool HRandFieldCmd::DoInitial(PClient* client) { void HRandFieldCmd::DoCmd(PClient* client) { // parse arguments - int64_t count{}; + const auto& argv = client->argv_; + int64_t count{1}; bool with_values{false}; - if (client->argv_.size() > 2) { - if (pstd::String2int(client->argv_[2], &count) == 0) { + if (argv.size() > 2) { + // redis checks the integer argument first and then the number of parameters + if (pstd::String2int(argv[2], &count) == 0) { client->SetRes(CmdRes::kInvalidInt); return; } - - if (client->argv_.size() > 4) { + if (argv.size() > 4) { client->SetRes(CmdRes::kSyntaxErr); return; } - - if (client->argv_.size() > 3) { - if (kWithValueString != pstd::StringToLower(client->argv_[3])) { + if (argv.size() > 3) { + if (kWithValueString != pstd::StringToLower(argv[3])) { client->SetRes(CmdRes::kSyntaxErr); return; } @@ -268,70 +268,26 @@ void HRandFieldCmd::DoCmd(PClient* client) { } } - // get hash - PObject* value = nullptr; - PError err = PSTORE.GetValueByType(client->Key(), value, kPTypeHash); - if (err != kPErrorOK) { - if (err == kPErrorNotExist) { - client->AppendString(""); - } else { - client->SetRes(CmdRes::kSyntaxErr, "hrandfield cmd error"); - } - return; - } - auto hash = value->CastHash(); - if (hash->empty()) { + // execute command + std::vector fvs; + auto s = PSTORE.GetBackend()->HRandField(client->Key(), count, &fvs); + if (s.IsNotFound()) { client->AppendString(""); return; } - - // fetch field(s) and reply - if (client->argv_.size() > 2) { - if (count >= 0) { - DoWithPositiveCount(client, hash, count, with_values); - } else { - DoWithNegativeCount(client, hash, count, with_values); - } - } else { - auto it = std::next(hash->begin(), rand() % hash->size()); - client->AppendString(it->first); + if (!s.ok()) { + client->SetRes(CmdRes::kErrOther, s.ToString()); + return; } -} - -void HRandFieldCmd::DoWithPositiveCount(PClient* client, const PHash* hash, int64_t count, bool with_value) { - if (hash->size() <= count) { // reply all fields - client->AppendArrayLen(with_value ? hash->size() * 2 : hash->size()); - for (auto&& kv : *hash) { - client->AppendString(kv.first); - if (with_value) { - client->AppendString(kv.second); - } - } - } else { // reply [count] fields - std::vector> kvs; - for (auto&& kv : *hash) { - kvs.push_back(kv); - } - std::shuffle(kvs.begin(), kvs.end(), std::mt19937(rd_())); - client->AppendArrayLen(with_value ? count * 2 : count); - for (size_t i = 0; i < count; i++) { - client->AppendString(kvs[i].first); - if (with_value) { - client->AppendString(kvs[i].second); - } - } + // reply to client + if (argv.size() > 2) { + client->AppendArrayLenUint64(with_values ? fvs.size() * 2 : fvs.size()); } -} - -void HRandFieldCmd::DoWithNegativeCount(PClient* client, const PHash* hash, int64_t count, bool with_value) { - count = -count; - client->AppendArrayLen(with_value ? count * 2 : count); - while (count--) { - auto it = std::next(hash->begin(), rand() % hash->size()); - client->AppendString(it->first); - if (with_value) { - client->AppendString(it->second); + for (const auto& [field, value] : fvs) { + client->AppendString(field); + if (with_values) { + client->AppendString(value); } } } diff --git a/src/cmd_hash.h b/src/cmd_hash.h index 505ae9997..60ad5f5d3 100644 --- a/src/cmd_hash.h +++ b/src/cmd_hash.h @@ -111,12 +111,8 @@ class HRandFieldCmd : public BaseCmd { private: void DoCmd(PClient *client) override; - void DoWithPositiveCount(PClient *client, const PHash *hash, int64_t count, bool with_value); - void DoWithNegativeCount(PClient *client, const PHash *hash, int64_t count, bool with_value); - std::random_device rd_; - - static const inline std::string kWithValueString{"withvalues"}; + static constexpr const char *kWithValueString = "withvalues"; }; } // namespace pikiwidb diff --git a/src/storage/include/storage/storage.h b/src/storage/include/storage/storage.h index 8c2517bab..246c08ad4 100644 --- a/src/storage/include/storage/storage.h +++ b/src/storage/include/storage/storage.h @@ -352,6 +352,9 @@ class Storage { Status HScanx(const Slice& key, const std::string& start_field, const std::string& pattern, int64_t count, std::vector* field_values, std::string* next_field); + // Return random field(s) and value(s) from the hash value stored at key. + Status HRandField(const Slice& key, int64_t count, std::vector* fvs); + // Iterate over a Hash table of fields by specified range // return next_field that the user need to use as the start_field argument // in the next call diff --git a/src/storage/src/redis_hashes.cc b/src/storage/src/redis_hashes.cc index 3a4a2dc08..25ad3ab98 100644 --- a/src/storage/src/redis_hashes.cc +++ b/src/storage/src/redis_hashes.cc @@ -6,6 +6,8 @@ #include "src/redis_hashes.h" #include +#include +#include #include #include @@ -923,6 +925,57 @@ Status RedisHashes::HScanx(const Slice& key, const std::string& start_field, con return Status::OK(); } +Status RedisHashes::HRandField(const Slice& key, int64_t count, std::vector* fvs) { + std::string meta_value; + Status s = db_->Get(default_read_options_, handles_[0], key, &meta_value); + if (!s.ok()) { + return s; + } + ParsedHashesMetaValue parsed_hashes_meta_value(&meta_value); + auto hlen = parsed_hashes_meta_value.count(); + if (parsed_hashes_meta_value.IsStale() || hlen == 0) { + return Status::NotFound(); + } + + if (count >= hlen) { + // case 1: count > 0 and >= hlen, return all fv + return HGetall(key, fvs); + } + + std::vector idxs; + if (count < 0) { + // case 2: count < 0, allow duplication + while (idxs.size() < -count) { + idxs.push_back(rand() % hlen); + } + } else { + // case 3: count > 0 and < hlen, no duplication + std::vector range(hlen); + std::iota(range.begin(), range.end(), 0); + std::random_device rd; + std::mt19937 g(rd()); + std::shuffle(range.begin(), range.end(), g); + idxs.insert(idxs.cend(), range.begin(), range.begin() + count); + } + std::sort(idxs.begin(), idxs.end()); + + HashesDataKey hashes_data_key(key, parsed_hashes_meta_value.version(), ""); + Slice prefix = hashes_data_key.Encode(); + auto iter = db_->NewIterator(default_read_options_, handles_[1]); + iter->Seek(prefix); + uint32_t save_idx{}; + for (auto idx : idxs) { + while (save_idx < idx) { + iter->Next(); + save_idx++; + } + assert(iter->Valid()); + ParsedHashesDataKey datakey(iter->key()); + fvs->emplace_back(datakey.field().ToString(), iter->value().ToString()); + } + return Status::OK(); +} + Status RedisHashes::PKHScanRange(const Slice& key, const Slice& field_start, const std::string& field_end, const Slice& pattern, int32_t limit, std::vector* field_values, std::string* next_field) { diff --git a/src/storage/src/redis_hashes.h b/src/storage/src/redis_hashes.h index 935a73843..00a676732 100644 --- a/src/storage/src/redis_hashes.h +++ b/src/storage/src/redis_hashes.h @@ -47,6 +47,7 @@ class RedisHashes : public Redis { std::vector* field_values, int64_t* next_cursor); Status HScanx(const Slice& key, const std::string& start_field, const std::string& pattern, int64_t count, std::vector* field_values, std::string* next_field); + Status HRandField(const Slice& key, int64_t count, std::vector* fvs); Status PKHScanRange(const Slice& key, const Slice& field_start, const std::string& field_end, const Slice& pattern, int32_t limit, std::vector* field_values, std::string* next_field); Status PKHRScanRange(const Slice& key, const Slice& field_start, const std::string& field_end, const Slice& pattern, diff --git a/src/storage/src/storage.cc b/src/storage/src/storage.cc index 0022834f0..b17f6a6bd 100644 --- a/src/storage/src/storage.cc +++ b/src/storage/src/storage.cc @@ -285,6 +285,10 @@ Status Storage::HScanx(const Slice& key, const std::string& start_field, const s return hashes_db_->HScanx(key, start_field, pattern, count, field_values, next_field); } +Status Storage::HRandField(const Slice& key, int64_t count, std::vector* fvs) { + return hashes_db_->HRandField(key, count, fvs); +} + Status Storage::PKHScanRange(const Slice& key, const Slice& field_start, const std::string& field_end, const Slice& pattern, int32_t limit, std::vector* field_values, std::string* next_field) { From 04bf9efce69129ffef3c539f52e7020e662250b5 Mon Sep 17 00:00:00 2001 From: longfar Date: Sun, 7 Jan 2024 21:28:50 +0800 Subject: [PATCH 07/13] fix: compile error --- src/storage/src/redis_hashes.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/src/redis_hashes.cc b/src/storage/src/redis_hashes.cc index 25ad3ab98..60040b1e6 100644 --- a/src/storage/src/redis_hashes.cc +++ b/src/storage/src/redis_hashes.cc @@ -971,7 +971,7 @@ Status RedisHashes::HRandField(const Slice& key, int64_t count, std::vectorValid()); ParsedHashesDataKey datakey(iter->key()); - fvs->emplace_back(datakey.field().ToString(), iter->value().ToString()); + fvs->push_back({datakey.field().ToString(), iter->value().ToString()}); } return Status::OK(); } From da04df9f812a1357961ca5cc80b3cc2774bbd791 Mon Sep 17 00:00:00 2001 From: longfar Date: Sun, 7 Jan 2024 23:01:44 +0800 Subject: [PATCH 08/13] fix: some details --- src/cmd_hash.h | 3 --- src/storage/src/redis_hashes.cc | 8 ++++++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/cmd_hash.h b/src/cmd_hash.h index 06f293f10..84a66f720 100644 --- a/src/cmd_hash.h +++ b/src/cmd_hash.h @@ -7,10 +7,7 @@ #pragma once -#include - #include "base_cmd.h" -#include "hash.h" namespace pikiwidb { diff --git a/src/storage/src/redis_hashes.cc b/src/storage/src/redis_hashes.cc index 60040b1e6..b741ddd94 100644 --- a/src/storage/src/redis_hashes.cc +++ b/src/storage/src/redis_hashes.cc @@ -943,11 +943,15 @@ Status RedisHashes::HRandField(const Slice& key, int64_t count, std::vector idxs; - if (count < 0) { + if (count == 1) { + // special case of case 3 + idxs.push_back(rand() % hlen); + } else if (count < 0) { // case 2: count < 0, allow duplication while (idxs.size() < -count) { idxs.push_back(rand() % hlen); } + std::sort(idxs.begin(), idxs.end()); } else { // case 3: count > 0 and < hlen, no duplication std::vector range(hlen); @@ -956,8 +960,8 @@ Status RedisHashes::HRandField(const Slice& key, int64_t count, std::vector Date: Thu, 11 Jan 2024 21:45:42 +0800 Subject: [PATCH 09/13] refactor: bw do not return value when with_values is false --- src/cmd_hash.cc | 13 +++++-------- src/storage/include/storage/storage.h | 2 +- src/storage/src/redis_hashes.cc | 18 +++++++++++++++--- src/storage/src/redis_hashes.h | 2 +- src/storage/src/storage.cc | 4 ++-- 5 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/cmd_hash.cc b/src/cmd_hash.cc index 7d8331333..952ab7351 100644 --- a/src/cmd_hash.cc +++ b/src/cmd_hash.cc @@ -324,8 +324,8 @@ void HRandFieldCmd::DoCmd(PClient* client) { } // execute command - std::vector fvs; - auto s = PSTORE.GetBackend()->HRandField(client->Key(), count, &fvs); + std::vector res; + auto s = PSTORE.GetBackend()->HRandField(client->Key(), count, with_values, &res); if (s.IsNotFound()) { client->AppendString(""); return; @@ -337,13 +337,10 @@ void HRandFieldCmd::DoCmd(PClient* client) { // reply to client if (argv.size() > 2) { - client->AppendArrayLenUint64(with_values ? fvs.size() * 2 : fvs.size()); + client->AppendArrayLenUint64(res.size()); } - for (const auto& [field, value] : fvs) { - client->AppendString(field); - if (with_values) { - client->AppendString(value); - } + for (const auto& item : res) { + client->AppendString(item); } } diff --git a/src/storage/include/storage/storage.h b/src/storage/include/storage/storage.h index 246c08ad4..c52b104fd 100644 --- a/src/storage/include/storage/storage.h +++ b/src/storage/include/storage/storage.h @@ -353,7 +353,7 @@ class Storage { std::vector* field_values, std::string* next_field); // Return random field(s) and value(s) from the hash value stored at key. - Status HRandField(const Slice& key, int64_t count, std::vector* fvs); + Status HRandField(const Slice& key, int64_t count, bool with_values, std::vector* res); // Iterate over a Hash table of fields by specified range // return next_field that the user need to use as the start_field argument diff --git a/src/storage/src/redis_hashes.cc b/src/storage/src/redis_hashes.cc index b741ddd94..4c392ebba 100644 --- a/src/storage/src/redis_hashes.cc +++ b/src/storage/src/redis_hashes.cc @@ -925,7 +925,7 @@ Status RedisHashes::HScanx(const Slice& key, const std::string& start_field, con return Status::OK(); } -Status RedisHashes::HRandField(const Slice& key, int64_t count, std::vector* fvs) { +Status RedisHashes::HRandField(const Slice& key, int64_t count, bool with_values, std::vector* res) { std::string meta_value; Status s = db_->Get(default_read_options_, handles_[0], key, &meta_value); if (!s.ok()) { @@ -939,7 +939,16 @@ Status RedisHashes::HRandField(const Slice& key, int64_t count, std::vector= hlen) { // case 1: count > 0 and >= hlen, return all fv - return HGetall(key, fvs); + if (!with_values) { + return HKeys(key, res); + } + std::vector fvs; + s = HGetall(key, &fvs); + for (const auto& [field, value] : fvs) { + res->push_back(field); + res->push_back(value); + } + return s; } std::vector idxs; @@ -975,7 +984,10 @@ Status RedisHashes::HRandField(const Slice& key, int64_t count, std::vectorValid()); ParsedHashesDataKey datakey(iter->key()); - fvs->push_back({datakey.field().ToString(), iter->value().ToString()}); + res->push_back(datakey.field().ToString()); + if (with_values) { + res->push_back(iter->value().ToString()); + } } return Status::OK(); } diff --git a/src/storage/src/redis_hashes.h b/src/storage/src/redis_hashes.h index 00a676732..89b769aaa 100644 --- a/src/storage/src/redis_hashes.h +++ b/src/storage/src/redis_hashes.h @@ -47,7 +47,7 @@ class RedisHashes : public Redis { std::vector* field_values, int64_t* next_cursor); Status HScanx(const Slice& key, const std::string& start_field, const std::string& pattern, int64_t count, std::vector* field_values, std::string* next_field); - Status HRandField(const Slice& key, int64_t count, std::vector* fvs); + Status HRandField(const Slice& key, int64_t count, bool with_values, std::vector* res); Status PKHScanRange(const Slice& key, const Slice& field_start, const std::string& field_end, const Slice& pattern, int32_t limit, std::vector* field_values, std::string* next_field); Status PKHRScanRange(const Slice& key, const Slice& field_start, const std::string& field_end, const Slice& pattern, diff --git a/src/storage/src/storage.cc b/src/storage/src/storage.cc index b17f6a6bd..36b2b1b6d 100644 --- a/src/storage/src/storage.cc +++ b/src/storage/src/storage.cc @@ -285,8 +285,8 @@ Status Storage::HScanx(const Slice& key, const std::string& start_field, const s return hashes_db_->HScanx(key, start_field, pattern, count, field_values, next_field); } -Status Storage::HRandField(const Slice& key, int64_t count, std::vector* fvs) { - return hashes_db_->HRandField(key, count, fvs); +Status Storage::HRandField(const Slice& key, int64_t count, bool with_values, std::vector* res) { + return hashes_db_->HRandField(key, count, with_values, res); } Status Storage::PKHScanRange(const Slice& key, const Slice& field_start, const std::string& field_end, From d7a04d27dd62e6fe911302e603f1253d956cb0cd Mon Sep 17 00:00:00 2001 From: longfar Date: Thu, 11 Jan 2024 21:47:06 +0800 Subject: [PATCH 10/13] fix: release iterator of rocksdb --- src/storage/src/redis_hashes.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/storage/src/redis_hashes.cc b/src/storage/src/redis_hashes.cc index 4c392ebba..fb0f28a7e 100644 --- a/src/storage/src/redis_hashes.cc +++ b/src/storage/src/redis_hashes.cc @@ -974,7 +974,8 @@ Status RedisHashes::HRandField(const Slice& key, int64_t count, bool with_values HashesDataKey hashes_data_key(key, parsed_hashes_meta_value.version(), ""); Slice prefix = hashes_data_key.Encode(); - auto iter = db_->NewIterator(default_read_options_, handles_[1]); + auto tmp_iter = db_->NewIterator(default_read_options_, handles_[1]); + std::unique_ptr iter{tmp_iter}; iter->Seek(prefix); uint32_t save_idx{}; for (auto idx : idxs) { From 540a11d33551e6975d7a1a2a515ffb7052da3905 Mon Sep 17 00:00:00 2001 From: longfar Date: Thu, 11 Jan 2024 21:48:54 +0800 Subject: [PATCH 11/13] fix: replace assert with return IOError --- src/storage/src/redis_hashes.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/storage/src/redis_hashes.cc b/src/storage/src/redis_hashes.cc index fb0f28a7e..b79c1139c 100644 --- a/src/storage/src/redis_hashes.cc +++ b/src/storage/src/redis_hashes.cc @@ -983,7 +983,9 @@ Status RedisHashes::HRandField(const Slice& key, int64_t count, bool with_values iter->Next(); save_idx++; } - assert(iter->Valid()); + if (!iter->Valid()) { + return Status::IOError(fmt::format("Should search for the data starting with {}", prefix.ToString())); + } ParsedHashesDataKey datakey(iter->key()); res->push_back(datakey.field().ToString()); if (with_values) { From dab7de8cde6cd4ede997aba48d8bf3bc2d99d0cb Mon Sep 17 00:00:00 2001 From: longfar Date: Thu, 11 Jan 2024 22:10:26 +0800 Subject: [PATCH 12/13] fix: clear result vector when return error --- src/storage/src/redis_hashes.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/storage/src/redis_hashes.cc b/src/storage/src/redis_hashes.cc index b79c1139c..38d83f6f7 100644 --- a/src/storage/src/redis_hashes.cc +++ b/src/storage/src/redis_hashes.cc @@ -984,6 +984,7 @@ Status RedisHashes::HRandField(const Slice& key, int64_t count, bool with_values save_idx++; } if (!iter->Valid()) { + res->clear(); return Status::IOError(fmt::format("Should search for the data starting with {}", prefix.ToString())); } ParsedHashesDataKey datakey(iter->key()); From 2b55a03f819a965fa0a840e6a2a79d07ae6a56e6 Mon Sep 17 00:00:00 2001 From: longfar Date: Fri, 12 Jan 2024 16:04:24 +0800 Subject: [PATCH 13/13] test: add test for hrandfield --- tests/hash_test.go | 84 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/tests/hash_test.go b/tests/hash_test.go index 60a65a248..37caa8e2d 100644 --- a/tests/hash_test.go +++ b/tests/hash_test.go @@ -67,4 +67,88 @@ var _ = Describe("Hash", Ordered, func() { log.Println("Cmd HSET Begin") Expect(client.HSet(ctx, "myhash", "one").Val()).NotTo(Equal("FooBar")) }) + + It("Cmd HRandField Test", func() { + // set test data + key := "hrandfield" + kvs := map[string]string{ + "field0": "value0", + "field1": "value1", + "field2": "value2", + } + for f, v := range kvs { + client.HSet(ctx, key, f, v) + } + + num_test := 10 + for i := 0; i < num_test; i++ { + // count < hlen + { + // without values + fields := client.HRandField(ctx, key, 2).Val() + Expect(len(fields)).To(Equal(2)) + Expect(fields[0]).ToNot(Equal(fields[1])) + for _, field := range fields { + _, ok := kvs[field] + Expect(ok).To(BeTrue()) + } + + // with values + fvs := client.HRandFieldWithValues(ctx, key, 2).Val() + Expect(len(fvs)).To(Equal(2)) + Expect(fvs[0].Key).ToNot(Equal(fvs[1].Key)) + for _, fv := range fvs { + val, ok := kvs[fv.Key] + Expect(ok).To(BeTrue()) + Expect(val).To(Equal(fv.Value)) + } + } + + // count > hlen + { + fields := client.HRandField(ctx, key, 10).Val() + Expect(len(fields)).To(Equal(3)) + Expect(fields[0]).ToNot(Equal(fields[1])) + Expect(fields[2]).ToNot(Equal(fields[1])) + Expect(fields[0]).ToNot(Equal(fields[2])) + for _, field := range fields { + _, ok := kvs[field] + Expect(ok).To(BeTrue()) + } + + fvs := client.HRandFieldWithValues(ctx, key, 10).Val() + Expect(len(fvs)).To(Equal(3)) + Expect(fvs[0].Key).ToNot(Equal(fvs[1].Key)) + Expect(fvs[2].Key).ToNot(Equal(fvs[1].Key)) + Expect(fvs[0].Key).ToNot(Equal(fvs[2].Key)) + for _, fv := range fvs { + val, ok := kvs[fv.Key] + Expect(ok).To(BeTrue()) + Expect(val).To(Equal(fv.Value)) + } + } + + // count < 0 + { + fields := client.HRandField(ctx, key, -10).Val() + Expect(len(fields)).To(Equal(10)) + for _, field := range fields { + _, ok := kvs[field] + Expect(ok).To(BeTrue()) + } + + fvs := client.HRandFieldWithValues(ctx, key, -10).Val() + Expect(len(fvs)).To(Equal(10)) + for _, fv := range fvs { + val, ok := kvs[fv.Key] + Expect(ok).To(BeTrue()) + Expect(val).To(Equal(fv.Value)) + } + } + } + + // the key not exist + res1 := client.HRandField(ctx, "not_exist_key", 1).Val() + Expect(len(res1)).To(Equal(0)) + }) })