From 32b52b6cfb3d4f2e6fa3bd0b6dbab66645e2ae37 Mon Sep 17 00:00:00 2001 From: Valay Date: Mon, 29 Dec 2025 22:14:18 -0800 Subject: [PATCH 01/21] Implement DELEX Command --- src/commands/cmd_string.cc | 52 +++++++++ src/types/redis_string.cc | 43 ++++++++ src/types/redis_string.h | 2 + tests/cppunit/types/string_test.cc | 163 ++++++++++++++++++++++++++++- 4 files changed, 259 insertions(+), 1 deletion(-) diff --git a/src/commands/cmd_string.cc b/src/commands/cmd_string.cc index a13ca38b213..687de303111 100644 --- a/src/commands/cmd_string.cc +++ b/src/commands/cmd_string.cc @@ -108,6 +108,56 @@ class CommandGetEx : public Commander { std::optional expire_; }; +class CommandDelEX : public Commander { + public: + Status Parse(const std::vector &args) override { + if (args.size() > 4) { + return {Status::RedisParseErr, errWrongNumOfArguments}; + } + + CommandParser parser(args, 2); + while (parser.Good()) { + if (parser.EatEqICase("ifdeq")) { + opt_ = '1'; + hash_or_val_ = GET_OR_RET(parser.TakeStr()); + } else if (parser.EatEqICase("ifdne")) { + opt_ = '2'; + hash_or_val_ = GET_OR_RET(parser.TakeStr()); + } else if (parser.EatEqICase("ifeq")) { + opt_ = '3'; + hash_or_val_ = GET_OR_RET(parser.TakeStr()); + } else if (parser.EatEqICase("ifne")) { + opt_ = '4'; + hash_or_val_ = GET_OR_RET(parser.TakeStr()); + } else { + return {Status::RedisParseErr, errInvalidSyntax}; + } + } + return Status::OK(); + } + + Status Execute(engine::Context &ctx, Server *srv, Connection *conn, std::string *output) override { + redis::String string_db(srv->storage, conn->GetNamespace()); + auto s = string_db.DelEX(ctx, args_[1], opt_, hash_or_val_, res_); + + if (!s.ok() && !s.IsNotFound()) { + return {Status::RedisExecErr, s.ToString()}; + } + + if (s.IsNotFound() || (opt_ && !res_)) { + *output = redis::Integer(0); + } else { + *output = redis::Integer(1); + } + return Status::OK(); + } + + private: + std::optional hash_or_val_ = std::nullopt; + std::optional opt_ = std::nullopt; + std::optional res_ = false; +}; + class CommandStrlen : public Commander { public: Status Execute(engine::Context &ctx, Server *srv, Connection *conn, std::string *output) override { @@ -721,6 +771,7 @@ class CommandLCS : public Commander { int64_t min_match_len_ = 0; }; + REDIS_REGISTER_COMMANDS( String, MakeCmdAttr("get", 2, "read-only", 1, 1, 1), MakeCmdAttr("getex", -2, "write", 1, 1, 1), @@ -729,6 +780,7 @@ REDIS_REGISTER_COMMANDS( MakeCmdAttr("getrange", 4, "read-only", 1, 1, 1), MakeCmdAttr("substr", 4, "read-only", 1, 1, 1), MakeCmdAttr("getdel", 2, "write no-dbsize-check", 1, 1, 1), + MakeCmdAttr("delex", -2, "write", 1, 1, 1), MakeCmdAttr("setrange", 4, "write", 1, 1, 1), MakeCmdAttr("mget", -2, "read-only", 1, -1, 1), MakeCmdAttr("append", 3, "write", 1, 1, 1), MakeCmdAttr("set", -3, "write", 1, 1, 1), diff --git a/src/types/redis_string.cc b/src/types/redis_string.cc index a3d27492e0f..a1ce70219ec 100644 --- a/src/types/redis_string.cc +++ b/src/types/redis_string.cc @@ -29,6 +29,7 @@ #include "parse_util.h" #include "storage/redis_metadata.h" #include "time_util.h" +#include "string_util.h" namespace redis { @@ -181,6 +182,48 @@ rocksdb::Status String::GetEx(engine::Context &ctx, const std::string &user_key, return rocksdb::Status::OK(); } +rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, std::optional &opt_, + std::optional &hash_or_val_, std::optional &res_) { + std::string ns_key = AppendNamespacePrefix(user_key); + std::string value; + rocksdb::Status s = getValue(ctx, ns_key, &value); + if (!s.ok() || s.IsNotFound()) { + return s; + } + if (!opt_.has_value() && !hash_or_val_.has_value()) { + return storage_->Delete(ctx, storage_->DefaultWriteOptions(), metadata_cf_handle_, ns_key); + } + switch (opt_.value()) { + case '1': + if (hash_or_val_.value() == "12345") { // StringDigest(value) + res_ = true; + } + break; + case '2': + if (hash_or_val_.value() != "12345") { // StringDigest(value) + res_ = true; + } + break; + case '3': + if (hash_or_val_.value() == value) { + res_ = true; + } + break; + case '4': + if (hash_or_val_.value() != value) { + res_ = true; + } + break; + default: + return rocksdb::Status::InvalidArgument(); + break; + } + if (res_.value()) { + return storage_->Delete(ctx, storage_->DefaultWriteOptions(), metadata_cf_handle_, ns_key); + } + return rocksdb::Status::OK(); +} + rocksdb::Status String::GetSet(engine::Context &ctx, const std::string &user_key, const std::string &new_value, std::optional &old_value) { auto s = diff --git a/src/types/redis_string.h b/src/types/redis_string.h index e5025d64fc6..a82a48e4bcf 100644 --- a/src/types/redis_string.h +++ b/src/types/redis_string.h @@ -82,6 +82,8 @@ class String : public Database { rocksdb::Status Get(engine::Context &ctx, const std::string &user_key, std::string *value); rocksdb::Status GetEx(engine::Context &ctx, const std::string &user_key, std::string *value, std::optional expire); + rocksdb::Status DelEX(engine::Context &ctx, const std::string &user_key, std::optional &opt_, + std::optional &hash_or_val_, std::optional &res_); rocksdb::Status GetSet(engine::Context &ctx, const std::string &user_key, const std::string &new_value, std::optional &old_value); rocksdb::Status GetDel(engine::Context &ctx, const std::string &user_key, std::string *value); diff --git a/tests/cppunit/types/string_test.cc b/tests/cppunit/types/string_test.cc index 1b5c1a27c2e..abe684f12ae 100644 --- a/tests/cppunit/types/string_test.cc +++ b/tests/cppunit/types/string_test.cc @@ -24,6 +24,7 @@ #include "test_base.h" #include "time_util.h" +#include "string_util.h" #include "types/redis_string.h" class RedisStringTest : public TestBase { @@ -156,6 +157,166 @@ TEST_F(RedisStringTest, GetSet) { } auto s = string_->Del(*ctx_, key_); } + +TEST_F(RedisStringTest, DelEX) { + std::optional hash_or_val_ = std::nullopt; + std::optional opt_ = std::nullopt; + std::optional res_ = 0; + + std::string key = "test-string-key10"; + std::string value = "test-strings-value10"; + auto status = string_->Set(*ctx_, key, value); + ASSERT_TRUE(status.ok()); + status = string_->Get(*ctx_, key, &value); + ASSERT_TRUE(status.ok() && !status.IsNotFound()); + EXPECT_EQ("test-strings-value10", value); + + // Check no args delete works + auto s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + EXPECT_TRUE(s.ok()); + EXPECT_FALSE(s.IsNotFound()); + EXPECT_FALSE(res_.value()); + EXPECT_FALSE(opt_.has_value()); + status = string_->Get(*ctx_, key, &value); + EXPECT_TRUE(!status.ok() && status.IsNotFound()); + EXPECT_NE("test-strings-value10", value); + + // Check no args delete on same key + s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + EXPECT_FALSE(s.ok()); + EXPECT_TRUE(s.IsNotFound()); + + // Check no args delete on invalid/notfound key + key = "random"; + s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + EXPECT_FALSE(s.ok()); + EXPECT_TRUE(s.IsNotFound()); + status = string_->Get(*ctx_, key, &value); + EXPECT_TRUE(!status.ok() && status.IsNotFound()); + + // Correct value but incorrect opt_ + key = "test-string-key10"; + value = "test-strings-value10"; + status = string_->Set(*ctx_, key, value); + EXPECT_TRUE(status.ok()); + opt_ = '0'; + hash_or_val_ = "test-strings-value10"; + s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + EXPECT_TRUE(s.IsInvalidArgument()); + status = string_->Get(*ctx_, key, &value); + EXPECT_TRUE(status.ok() && !status.IsNotFound()); + EXPECT_EQ("test-strings-value10", value); + + // Checking true false cases for all args + key = "test-string-key10"; + value = "test-strings-value10"; + status = string_->Set(*ctx_, key, value); + EXPECT_TRUE(status.ok()); + opt_ = '1'; + hash_or_val_ = "xxxxxxxxxxxxxxxx"; + res_ = 0; + s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + EXPECT_TRUE(s.ok()); + EXPECT_FALSE(s.IsNotFound()); + EXPECT_FALSE(res_.value()); + status = string_->Get(*ctx_, key, &value); + EXPECT_TRUE(status.ok() && !status.IsNotFound()); + EXPECT_EQ("test-strings-value10", value); + + opt_ = '1'; + hash_or_val_ = "12345";//StringDigest(value); + res_ = 0; + s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + EXPECT_TRUE(s.ok()); + EXPECT_FALSE(s.IsNotFound()); + EXPECT_TRUE(res_.value()); + status = string_->Get(*ctx_, key, &value); + EXPECT_TRUE(!status.ok()); + EXPECT_TRUE(status.IsNotFound()); + EXPECT_NE("test-strings-value10", value); + + key = "test-string-key10"; + value = "test-strings-value10"; + status = string_->Set(*ctx_, key, value); + EXPECT_TRUE(status.ok()); + opt_ = '2'; + hash_or_val_ = "12345"; // StringDigest(value); + res_ = 0; + s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + EXPECT_TRUE(s.ok()); + EXPECT_FALSE(s.IsNotFound()); + EXPECT_FALSE(res_.value()); + status = string_->Get(*ctx_, key, &value); + EXPECT_TRUE(status.ok() && !status.IsNotFound()); + EXPECT_EQ("test-strings-value10", value); + + opt_ = '2'; + hash_or_val_ = "xxxxxxxxxxxxxxxx"; + res_ = 0; + s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + EXPECT_TRUE(s.ok()); + EXPECT_FALSE(s.IsNotFound()); + EXPECT_TRUE(res_.value()); + status = string_->Get(*ctx_, key, &value); + EXPECT_TRUE(!status.ok()); + EXPECT_TRUE(status.IsNotFound()); + EXPECT_NE("test-strings-value10", value); + + key = "test-string-key10"; + value = "test-strings-value10"; + status = string_->Set(*ctx_, key, value); + EXPECT_TRUE(status.ok()); + opt_ = '3'; + hash_or_val_ = "random"; + res_ = 0; + s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + EXPECT_TRUE(s.ok()); + EXPECT_FALSE(s.IsNotFound()); + EXPECT_FALSE(res_.value()); + status = string_->Get(*ctx_, key, &value); + EXPECT_TRUE(status.ok() && !status.IsNotFound()); + EXPECT_EQ("test-strings-value10", value); + + opt_ = '3'; + hash_or_val_ = "test-strings-value10"; + res_ = 0; + s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + EXPECT_TRUE(s.ok()); + EXPECT_FALSE(s.IsNotFound()); + EXPECT_TRUE(res_.value()); + status = string_->Get(*ctx_, key, &value); + EXPECT_TRUE(!status.ok()); + EXPECT_TRUE(status.IsNotFound()); + EXPECT_NE("test-strings-value10", value); + + key = "test-string-key10"; + value = "test-strings-value10"; + status = string_->Set(*ctx_, key, value); + EXPECT_TRUE(status.ok()); + opt_ = '4'; + hash_or_val_ = "test-strings-value10"; + res_ = 0; + s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + EXPECT_TRUE(s.ok()); + EXPECT_FALSE(s.IsNotFound()); + EXPECT_FALSE(res_.value()); + status = string_->Get(*ctx_, key, &value); + EXPECT_TRUE(status.ok() && !status.IsNotFound()); + EXPECT_EQ("test-strings-value10", value); + + opt_ = '4'; + hash_or_val_ = "random"; + res_ = 0; + s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + EXPECT_TRUE(s.ok()); + EXPECT_FALSE(s.IsNotFound()); + EXPECT_TRUE(res_.value()); + status = string_->Get(*ctx_, key, &value); + EXPECT_TRUE(!status.ok()); + EXPECT_TRUE(status.IsNotFound()); + EXPECT_NE("test-strings-value10", value); +} + TEST_F(RedisStringTest, GetDel) { for (auto &pair : pairs_) { string_->Set(*ctx_, pair.key.ToString(), pair.value.ToString()); @@ -367,4 +528,4 @@ TEST_F(RedisStringTest, LCS) { }, 4}, std::get(rst)); -} +} \ No newline at end of file From ecfcbd63e1f93faae27fc78642d63920aa86e76b Mon Sep 17 00:00:00 2001 From: Valay Date: Mon, 29 Dec 2025 23:55:37 -0800 Subject: [PATCH 02/21] Fix clang formatting --- src/commands/cmd_string.cc | 1 - src/types/redis_string.cc | 4 ++-- tests/cppunit/types/string_test.cc | 6 +++--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/commands/cmd_string.cc b/src/commands/cmd_string.cc index 687de303111..1253e901d0d 100644 --- a/src/commands/cmd_string.cc +++ b/src/commands/cmd_string.cc @@ -771,7 +771,6 @@ class CommandLCS : public Commander { int64_t min_match_len_ = 0; }; - REDIS_REGISTER_COMMANDS( String, MakeCmdAttr("get", 2, "read-only", 1, 1, 1), MakeCmdAttr("getex", -2, "write", 1, 1, 1), diff --git a/src/types/redis_string.cc b/src/types/redis_string.cc index a1ce70219ec..76de306828b 100644 --- a/src/types/redis_string.cc +++ b/src/types/redis_string.cc @@ -28,8 +28,8 @@ #include "parse_util.h" #include "storage/redis_metadata.h" -#include "time_util.h" #include "string_util.h" +#include "time_util.h" namespace redis { @@ -195,7 +195,7 @@ rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, } switch (opt_.value()) { case '1': - if (hash_or_val_.value() == "12345") { // StringDigest(value) + if (hash_or_val_.value() == "12345") { // StringDigest(value) res_ = true; } break; diff --git a/tests/cppunit/types/string_test.cc b/tests/cppunit/types/string_test.cc index abe684f12ae..2b7dfe1bb7f 100644 --- a/tests/cppunit/types/string_test.cc +++ b/tests/cppunit/types/string_test.cc @@ -22,9 +22,9 @@ #include +#include "string_util.h" #include "test_base.h" #include "time_util.h" -#include "string_util.h" #include "types/redis_string.h" class RedisStringTest : public TestBase { @@ -224,7 +224,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_EQ("test-strings-value10", value); opt_ = '1'; - hash_or_val_ = "12345";//StringDigest(value); + hash_or_val_ = "12345"; // StringDigest(value); res_ = 0; s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); EXPECT_TRUE(s.ok()); @@ -315,7 +315,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); EXPECT_NE("test-strings-value10", value); -} +} TEST_F(RedisStringTest, GetDel) { for (auto &pair : pairs_) { From 3c2b8f068b54c40a71b28978d7da6cd0564f85ba Mon Sep 17 00:00:00 2001 From: Valay Date: Tue, 30 Dec 2025 15:53:38 -0800 Subject: [PATCH 03/21] Improved Code --- src/commands/cmd_string.cc | 26 +++---- src/types/redis_string.cc | 34 ++++----- src/types/redis_string.h | 6 +- tests/cppunit/types/string_test.cc | 106 ++++++++++++++--------------- 4 files changed, 87 insertions(+), 85 deletions(-) diff --git a/src/commands/cmd_string.cc b/src/commands/cmd_string.cc index 1253e901d0d..0505a83178d 100644 --- a/src/commands/cmd_string.cc +++ b/src/commands/cmd_string.cc @@ -118,17 +118,17 @@ class CommandDelEX : public Commander { CommandParser parser(args, 2); while (parser.Good()) { if (parser.EatEqICase("ifdeq")) { - opt_ = '1'; - hash_or_val_ = GET_OR_RET(parser.TakeStr()); + option = DelExOption::IFDEQ; + hash_or_value = GET_OR_RET(parser.TakeStr()); } else if (parser.EatEqICase("ifdne")) { - opt_ = '2'; - hash_or_val_ = GET_OR_RET(parser.TakeStr()); + option = DelExOption::IFDNE; + hash_or_value = GET_OR_RET(parser.TakeStr()); } else if (parser.EatEqICase("ifeq")) { - opt_ = '3'; - hash_or_val_ = GET_OR_RET(parser.TakeStr()); + option = DelExOption::IFEQ; + hash_or_value = GET_OR_RET(parser.TakeStr()); } else if (parser.EatEqICase("ifne")) { - opt_ = '4'; - hash_or_val_ = GET_OR_RET(parser.TakeStr()); + option = DelExOption::IFNE; + hash_or_value = GET_OR_RET(parser.TakeStr()); } else { return {Status::RedisParseErr, errInvalidSyntax}; } @@ -138,13 +138,13 @@ class CommandDelEX : public Commander { Status Execute(engine::Context &ctx, Server *srv, Connection *conn, std::string *output) override { redis::String string_db(srv->storage, conn->GetNamespace()); - auto s = string_db.DelEX(ctx, args_[1], opt_, hash_or_val_, res_); + auto s = string_db.DelEX(ctx, args_[1], option, hash_or_value, deleted); if (!s.ok() && !s.IsNotFound()) { return {Status::RedisExecErr, s.ToString()}; } - if (s.IsNotFound() || (opt_ && !res_)) { + if (s.IsNotFound() || (option == DelExOption::NONE && !deleted)) { *output = redis::Integer(0); } else { *output = redis::Integer(1); @@ -153,9 +153,9 @@ class CommandDelEX : public Commander { } private: - std::optional hash_or_val_ = std::nullopt; - std::optional opt_ = std::nullopt; - std::optional res_ = false; + std::optional hash_or_value = std::nullopt; + DelExOption option = DelExOption::NONE; + bool deleted = false; }; class CommandStrlen : public Commander { diff --git a/src/types/redis_string.cc b/src/types/redis_string.cc index 76de306828b..f9f2aa923bb 100644 --- a/src/types/redis_string.cc +++ b/src/types/redis_string.cc @@ -182,43 +182,43 @@ rocksdb::Status String::GetEx(engine::Context &ctx, const std::string &user_key, return rocksdb::Status::OK(); } -rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, std::optional &opt_, - std::optional &hash_or_val_, std::optional &res_) { +rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, DelExOption &option, + std::optional &hash_or_value, bool &deleted) { std::string ns_key = AppendNamespacePrefix(user_key); std::string value; rocksdb::Status s = getValue(ctx, ns_key, &value); if (!s.ok() || s.IsNotFound()) { return s; } - if (!opt_.has_value() && !hash_or_val_.has_value()) { + if (option == DelExOption::NONE && !hash_or_value.has_value()) { return storage_->Delete(ctx, storage_->DefaultWriteOptions(), metadata_cf_handle_, ns_key); } - switch (opt_.value()) { - case '1': - if (hash_or_val_.value() == "12345") { // StringDigest(value) - res_ = true; + switch (option) { + case DelExOption::IFDEQ: + if (hash_or_value.value() == "12345") { // StringDigest(value) + deleted = true; } break; - case '2': - if (hash_or_val_.value() != "12345") { // StringDigest(value) - res_ = true; + case DelExOption::IFDNE: + if (hash_or_value.value() != "12345") { // StringDigest(value) + deleted = true; } break; - case '3': - if (hash_or_val_.value() == value) { - res_ = true; + case DelExOption::IFEQ: + if (hash_or_value.value() == value) { + deleted = true; } break; - case '4': - if (hash_or_val_.value() != value) { - res_ = true; + case DelExOption::IFNE: + if (hash_or_value.value() != value) { + deleted = true; } break; default: return rocksdb::Status::InvalidArgument(); break; } - if (res_.value()) { + if (deleted) { return storage_->Delete(ctx, storage_->DefaultWriteOptions(), metadata_cf_handle_, ns_key); } return rocksdb::Status::OK(); diff --git a/src/types/redis_string.h b/src/types/redis_string.h index a82a48e4bcf..bcbc374a943 100644 --- a/src/types/redis_string.h +++ b/src/types/redis_string.h @@ -34,6 +34,8 @@ struct StringPair { Slice value; }; +enum class DelExOption { NONE, IFDEQ, IFDNE, IFEQ, IFNE }; + enum class StringSetType { NONE, NX, XX }; struct StringSetArgs { @@ -82,8 +84,8 @@ class String : public Database { rocksdb::Status Get(engine::Context &ctx, const std::string &user_key, std::string *value); rocksdb::Status GetEx(engine::Context &ctx, const std::string &user_key, std::string *value, std::optional expire); - rocksdb::Status DelEX(engine::Context &ctx, const std::string &user_key, std::optional &opt_, - std::optional &hash_or_val_, std::optional &res_); + rocksdb::Status DelEX(engine::Context &ctx, const std::string &user_key, DelExOption &option, + std::optional &hash_or_value, bool &deleted); rocksdb::Status GetSet(engine::Context &ctx, const std::string &user_key, const std::string &new_value, std::optional &old_value); rocksdb::Status GetDel(engine::Context &ctx, const std::string &user_key, std::string *value); diff --git a/tests/cppunit/types/string_test.cc b/tests/cppunit/types/string_test.cc index 2b7dfe1bb7f..7b51b32ab4f 100644 --- a/tests/cppunit/types/string_test.cc +++ b/tests/cppunit/types/string_test.cc @@ -159,9 +159,9 @@ TEST_F(RedisStringTest, GetSet) { } TEST_F(RedisStringTest, DelEX) { - std::optional hash_or_val_ = std::nullopt; - std::optional opt_ = std::nullopt; - std::optional res_ = 0; + std::optional hash_or_value = std::nullopt; + DelExOption option = DelExOption::NONE; + bool deleted = 0; std::string key = "test-string-key10"; std::string value = "test-strings-value10"; @@ -172,36 +172,36 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_EQ("test-strings-value10", value); // Check no args delete works - auto s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + auto s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); - EXPECT_FALSE(res_.value()); - EXPECT_FALSE(opt_.has_value()); + EXPECT_FALSE(deleted); + EXPECT_EQ(option, DelExOption::NONE); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok() && status.IsNotFound()); EXPECT_NE("test-strings-value10", value); // Check no args delete on same key - s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_FALSE(s.ok()); EXPECT_TRUE(s.IsNotFound()); // Check no args delete on invalid/notfound key key = "random"; - s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_FALSE(s.ok()); EXPECT_TRUE(s.IsNotFound()); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok() && status.IsNotFound()); - // Correct value but incorrect opt_ + // Correct value but incorrect option key = "test-string-key10"; value = "test-strings-value10"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); - opt_ = '0'; - hash_or_val_ = "test-strings-value10"; - s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + option = DelExOption::NONE; + hash_or_value = "test-strings-value10"; + s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.IsInvalidArgument()); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); @@ -212,24 +212,24 @@ TEST_F(RedisStringTest, DelEX) { value = "test-strings-value10"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); - opt_ = '1'; - hash_or_val_ = "xxxxxxxxxxxxxxxx"; - res_ = 0; - s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + option = DelExOption::IFDEQ; + hash_or_value = "xxxxxxxxxxxxxxxx"; + deleted = 0; + s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); - EXPECT_FALSE(res_.value()); + EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); EXPECT_EQ("test-strings-value10", value); - opt_ = '1'; - hash_or_val_ = "12345"; // StringDigest(value); - res_ = 0; - s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + option = DelExOption::IFDEQ; + hash_or_value = "12345"; // StringDigest(value); + deleted = 0; + s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); - EXPECT_TRUE(res_.value()); + EXPECT_TRUE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); @@ -239,24 +239,24 @@ TEST_F(RedisStringTest, DelEX) { value = "test-strings-value10"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); - opt_ = '2'; - hash_or_val_ = "12345"; // StringDigest(value); - res_ = 0; - s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + option = DelExOption::IFDNE; + hash_or_value = "12345"; // StringDigest(value); + deleted = 0; + s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); - EXPECT_FALSE(res_.value()); + EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); EXPECT_EQ("test-strings-value10", value); - opt_ = '2'; - hash_or_val_ = "xxxxxxxxxxxxxxxx"; - res_ = 0; - s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + option = DelExOption::IFDNE; + hash_or_value = "xxxxxxxxxxxxxxxx"; + deleted = 0; + s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); - EXPECT_TRUE(res_.value()); + EXPECT_TRUE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); @@ -266,24 +266,24 @@ TEST_F(RedisStringTest, DelEX) { value = "test-strings-value10"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); - opt_ = '3'; - hash_or_val_ = "random"; - res_ = 0; - s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + option = DelExOption::IFEQ; + hash_or_value = "random"; + deleted = 0; + s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); - EXPECT_FALSE(res_.value()); + EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); EXPECT_EQ("test-strings-value10", value); - opt_ = '3'; - hash_or_val_ = "test-strings-value10"; - res_ = 0; - s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + option = DelExOption::IFEQ; + hash_or_value = "test-strings-value10"; + deleted = 0; + s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); - EXPECT_TRUE(res_.value()); + EXPECT_TRUE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); @@ -293,24 +293,24 @@ TEST_F(RedisStringTest, DelEX) { value = "test-strings-value10"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); - opt_ = '4'; - hash_or_val_ = "test-strings-value10"; - res_ = 0; - s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + option = DelExOption::IFNE; + hash_or_value = "test-strings-value10"; + deleted = 0; + s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); - EXPECT_FALSE(res_.value()); + EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); EXPECT_EQ("test-strings-value10", value); - opt_ = '4'; - hash_or_val_ = "random"; - res_ = 0; - s = string_->DelEX(*ctx_, key, opt_, hash_or_val_, res_); + option = DelExOption::IFNE; + hash_or_value = "random"; + deleted = 0; + s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); - EXPECT_TRUE(res_.value()); + EXPECT_TRUE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); @@ -528,4 +528,4 @@ TEST_F(RedisStringTest, LCS) { }, 4}, std::get(rst)); -} \ No newline at end of file +} From c4d7a8f8520097fe4fda7f157afe34a17af12e05 Mon Sep 17 00:00:00 2001 From: Valay Date: Tue, 30 Dec 2025 17:47:23 -0800 Subject: [PATCH 04/21] Optimize CPP Tests and Add Go Tests --- tests/cppunit/types/string_test.cc | 46 +++++------- .../gocase/unit/type/strings/strings_test.go | 73 +++++++++++++++++++ 2 files changed, 91 insertions(+), 28 deletions(-) diff --git a/tests/cppunit/types/string_test.cc b/tests/cppunit/types/string_test.cc index 7b51b32ab4f..430865d3c03 100644 --- a/tests/cppunit/types/string_test.cc +++ b/tests/cppunit/types/string_test.cc @@ -163,13 +163,13 @@ TEST_F(RedisStringTest, DelEX) { DelExOption option = DelExOption::NONE; bool deleted = 0; - std::string key = "test-string-key10"; - std::string value = "test-strings-value10"; + std::string key = "test-string-key69"; + std::string value = "test-strings-value69"; auto status = string_->Set(*ctx_, key, value); ASSERT_TRUE(status.ok()); status = string_->Get(*ctx_, key, &value); ASSERT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value10", value); + EXPECT_EQ("test-strings-value69", value); // Check no args delete works auto s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); @@ -179,7 +179,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_EQ(option, DelExOption::NONE); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok() && status.IsNotFound()); - EXPECT_NE("test-strings-value10", value); + EXPECT_NE("test-strings-value69", value); // Check no args delete on same key s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); @@ -195,21 +195,19 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(!status.ok() && status.IsNotFound()); // Correct value but incorrect option - key = "test-string-key10"; - value = "test-strings-value10"; + key = "test-string-key69"; + value = "test-strings-value69"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); option = DelExOption::NONE; - hash_or_value = "test-strings-value10"; + hash_or_value = "test-strings-value69"; s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.IsInvalidArgument()); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value10", value); + EXPECT_EQ("test-strings-value69", value); // Checking true false cases for all args - key = "test-string-key10"; - value = "test-strings-value10"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); option = DelExOption::IFDEQ; @@ -221,9 +219,8 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value10", value); + EXPECT_EQ("test-strings-value69", value); - option = DelExOption::IFDEQ; hash_or_value = "12345"; // StringDigest(value); deleted = 0; s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); @@ -233,10 +230,8 @@ TEST_F(RedisStringTest, DelEX) { status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); - EXPECT_NE("test-strings-value10", value); + EXPECT_NE("test-strings-value69", value); - key = "test-string-key10"; - value = "test-strings-value10"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); option = DelExOption::IFDNE; @@ -248,9 +243,8 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value10", value); + EXPECT_EQ("test-strings-value69", value); - option = DelExOption::IFDNE; hash_or_value = "xxxxxxxxxxxxxxxx"; deleted = 0; s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); @@ -260,10 +254,8 @@ TEST_F(RedisStringTest, DelEX) { status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); - EXPECT_NE("test-strings-value10", value); + EXPECT_NE("test-strings-value69", value); - key = "test-string-key10"; - value = "test-strings-value10"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); option = DelExOption::IFEQ; @@ -275,10 +267,10 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value10", value); + EXPECT_EQ("test-strings-value69", value); option = DelExOption::IFEQ; - hash_or_value = "test-strings-value10"; + hash_or_value = "test-strings-value69"; deleted = 0; s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.ok()); @@ -287,14 +279,12 @@ TEST_F(RedisStringTest, DelEX) { status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); - EXPECT_NE("test-strings-value10", value); + EXPECT_NE("test-strings-value69", value); - key = "test-string-key10"; - value = "test-strings-value10"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); option = DelExOption::IFNE; - hash_or_value = "test-strings-value10"; + hash_or_value = "test-strings-value69"; deleted = 0; s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.ok()); @@ -302,7 +292,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value10", value); + EXPECT_EQ("test-strings-value69", value); option = DelExOption::IFNE; hash_or_value = "random"; @@ -314,7 +304,7 @@ TEST_F(RedisStringTest, DelEX) { status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); - EXPECT_NE("test-strings-value10", value); + EXPECT_NE("test-strings-value69", value); } TEST_F(RedisStringTest, GetDel) { diff --git a/tests/gocase/unit/type/strings/strings_test.go b/tests/gocase/unit/type/strings/strings_test.go index 56b67cb002c..17270f4d1e7 100644 --- a/tests/gocase/unit/type/strings/strings_test.go +++ b/tests/gocase/unit/type/strings/strings_test.go @@ -277,6 +277,79 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { require.Equal(t, "", rdb.GetDel(ctx, "foo").Val()) }) + t.Run("DELEX command no args", func(t *testing.T) { + key := "test-string-key69" + value := "test-strings-value69" + require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) + require.Equal(t, value, rdb.Get(ctx, key).Val()) + + require.Equal(t, "1", rdb.DelEX(ctx, key).Val()) + require.Equal(t, "", rdb.Get(ctx, key).Val()) + //on same key + require.Error(t, rdb.DelEX(ctx,key).Err()) + require.Equal(t, "0", rdb.Do(ctx, "DELEX", "test-string-key69").Val()) + //on non existent key + key := "random" + require.Equal(t, "", rdb.Get(ctx, key).Val()) + require.Error(t, rdb.DelEX(ctx,key).Err()) + require.Equal(t, "0", rdb.Do(ctx, "DELEX", "random").Val()) + }) + + t.Run("DELEX command with args", func(t *testing.T) { + key := "test-string-key69" + value := "test-strings-value69" + require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) + require.Equal(t, value, rdb.Get(ctx, key).Val()) + + // More than 4 args + require.Error(t, rdb.Do(ctx, "DELEX", "test-string-key69", "random", "random", "random").Err()) + //Incorrect option + require.Error(t, rdb.Do(ctx, "DELEX", "test-string-key69", "random", "random").Err()) + //True cases for all options + //digest1 := rdb.Do(ctx, "DIGEST", value).Val().(string) + require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Err()) + require.Equal(t, "0", rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Val()) + require.Equal(t, value, rdb.Get(ctx, key).Val()) + require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdeq", "12345").Err()) //digest1 + require.Equal(t, "", rdb.Get(ctx, value).Val()) + require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) + require.Equal(t, "1", rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", "12345").Val()) //digest1 + require.Equal(t, "", rdb.Get(ctx, value).Val()) + + require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) + require.Equal(t, value, rdb.Get(ctx, key).Val()) + require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdne", "12345").Err()) //digest1 + require.Equal(t, "0", rdb.Do(ctx, "DELEX","test-string-key69", "ifdne", "12345").Val()) //digest1 + require.Equal(t, value, rdb.Get(ctx, key).Val()) + require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdne", "xxxxxxxxxxxxxxxx").Err()) + require.Equal(t, "", rdb.Get(ctx, value).Val()) + require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) + require.Equal(t, "1", rdb.Do(ctx, "DELEX","test-string-key69", "ifdne", "xxxxxxxxxxxxxxxx").Val()) + require.Equal(t, "", rdb.Get(ctx, value).Val()) + + require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) + require.Equal(t, value, rdb.Get(ctx, key).Val()) + require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifeq", "random").Err()) + require.Equal(t, "0", rdb.Do(ctx, "DELEX","test-string-key69", "ifeq", "random").Val()) + require.Equal(t, value, rdb.Get(ctx, key).Val()) + require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifeq", "test-strings-value69").Err()) + require.Equal(t, "", rdb.Get(ctx, value).Val()) + require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) + require.Equal(t, "1", rdb.Do(ctx, "DELEX","test-string-key69", "ifeq", "test-strings-value69").Val()) + require.Equal(t, "", rdb.Get(ctx, value).Val()) + + require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) + require.Equal(t, value, rdb.Get(ctx, key).Val()) + require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifne", "test-strings-value69").Err()) + require.Equal(t, "0", rdb.Do(ctx, "DELEX","test-string-key69", "ifne", "test-strings-value69").Val()) + require.Equal(t, value, rdb.Get(ctx, key).Val()) + require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifne", "random").Err()) + require.Equal(t, "", rdb.Get(ctx, value).Val()) + require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) + require.Equal(t, "1", rdb.Do(ctx, "DELEX","test-string-key69", "ifne", "random").Val()) + require.Equal(t, "", rdb.Get(ctx, value).Val()) + }) + t.Run("MGET command", func(t *testing.T) { require.NoError(t, rdb.FlushDB(ctx).Err()) require.NoError(t, rdb.Set(ctx, "foo", "BAR", 0).Err()) From 1d89a0428a7e0887cd7c2d1ca423fb911298a4b7 Mon Sep 17 00:00:00 2001 From: Valay Saitwadekar <51906989+Valay17@users.noreply.github.com> Date: Tue, 30 Dec 2025 17:51:09 -0800 Subject: [PATCH 05/21] Update strings_test.go --- tests/gocase/unit/type/strings/strings_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/gocase/unit/type/strings/strings_test.go b/tests/gocase/unit/type/strings/strings_test.go index 17270f4d1e7..91c31c52024 100644 --- a/tests/gocase/unit/type/strings/strings_test.go +++ b/tests/gocase/unit/type/strings/strings_test.go @@ -306,20 +306,20 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { //Incorrect option require.Error(t, rdb.Do(ctx, "DELEX", "test-string-key69", "random", "random").Err()) //True cases for all options - //digest1 := rdb.Do(ctx, "DIGEST", value).Val().(string) + //digest := rdb.Do(ctx, "DIGEST", value).Val().(string) require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Err()) require.Equal(t, "0", rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdeq", "12345").Err()) //digest1 + require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdeq", "12345").Err()) //digest require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, "1", rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", "12345").Val()) //digest1 + require.Equal(t, "1", rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", "12345").Val()) //digest require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdne", "12345").Err()) //digest1 - require.Equal(t, "0", rdb.Do(ctx, "DELEX","test-string-key69", "ifdne", "12345").Val()) //digest1 + require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdne", "12345").Err()) //digest + require.Equal(t, "0", rdb.Do(ctx, "DELEX","test-string-key69", "ifdne", "12345").Val()) //digest require.Equal(t, value, rdb.Get(ctx, key).Val()) require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdne", "xxxxxxxxxxxxxxxx").Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) From de13ac20ab72f0b21125d4cf8b98535a4fa94c14 Mon Sep 17 00:00:00 2001 From: Valay Saitwadekar <51906989+Valay17@users.noreply.github.com> Date: Tue, 30 Dec 2025 18:14:17 -0800 Subject: [PATCH 06/21] Update src/types/redis_string.cc Co-authored-by: hulk --- src/types/redis_string.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/types/redis_string.cc b/src/types/redis_string.cc index f9f2aa923bb..9d9b81af05b 100644 --- a/src/types/redis_string.cc +++ b/src/types/redis_string.cc @@ -187,9 +187,7 @@ rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, std::string ns_key = AppendNamespacePrefix(user_key); std::string value; rocksdb::Status s = getValue(ctx, ns_key, &value); - if (!s.ok() || s.IsNotFound()) { - return s; - } + if (!s.ok()) return s; if (option == DelExOption::NONE && !hash_or_value.has_value()) { return storage_->Delete(ctx, storage_->DefaultWriteOptions(), metadata_cf_handle_, ns_key); } From b722332f81ca8d954c39f092a0e7becf10e160e6 Mon Sep 17 00:00:00 2001 From: Valay Saitwadekar <51906989+Valay17@users.noreply.github.com> Date: Tue, 30 Dec 2025 18:23:23 -0800 Subject: [PATCH 07/21] Update string_test.cc --- tests/cppunit/types/string_test.cc | 46 ++++++++++++++++++------------ 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/tests/cppunit/types/string_test.cc b/tests/cppunit/types/string_test.cc index 430865d3c03..7b51b32ab4f 100644 --- a/tests/cppunit/types/string_test.cc +++ b/tests/cppunit/types/string_test.cc @@ -163,13 +163,13 @@ TEST_F(RedisStringTest, DelEX) { DelExOption option = DelExOption::NONE; bool deleted = 0; - std::string key = "test-string-key69"; - std::string value = "test-strings-value69"; + std::string key = "test-string-key10"; + std::string value = "test-strings-value10"; auto status = string_->Set(*ctx_, key, value); ASSERT_TRUE(status.ok()); status = string_->Get(*ctx_, key, &value); ASSERT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value69", value); + EXPECT_EQ("test-strings-value10", value); // Check no args delete works auto s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); @@ -179,7 +179,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_EQ(option, DelExOption::NONE); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok() && status.IsNotFound()); - EXPECT_NE("test-strings-value69", value); + EXPECT_NE("test-strings-value10", value); // Check no args delete on same key s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); @@ -195,19 +195,21 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(!status.ok() && status.IsNotFound()); // Correct value but incorrect option - key = "test-string-key69"; - value = "test-strings-value69"; + key = "test-string-key10"; + value = "test-strings-value10"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); option = DelExOption::NONE; - hash_or_value = "test-strings-value69"; + hash_or_value = "test-strings-value10"; s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.IsInvalidArgument()); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value69", value); + EXPECT_EQ("test-strings-value10", value); // Checking true false cases for all args + key = "test-string-key10"; + value = "test-strings-value10"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); option = DelExOption::IFDEQ; @@ -219,8 +221,9 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value69", value); + EXPECT_EQ("test-strings-value10", value); + option = DelExOption::IFDEQ; hash_or_value = "12345"; // StringDigest(value); deleted = 0; s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); @@ -230,8 +233,10 @@ TEST_F(RedisStringTest, DelEX) { status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); - EXPECT_NE("test-strings-value69", value); + EXPECT_NE("test-strings-value10", value); + key = "test-string-key10"; + value = "test-strings-value10"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); option = DelExOption::IFDNE; @@ -243,8 +248,9 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value69", value); + EXPECT_EQ("test-strings-value10", value); + option = DelExOption::IFDNE; hash_or_value = "xxxxxxxxxxxxxxxx"; deleted = 0; s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); @@ -254,8 +260,10 @@ TEST_F(RedisStringTest, DelEX) { status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); - EXPECT_NE("test-strings-value69", value); + EXPECT_NE("test-strings-value10", value); + key = "test-string-key10"; + value = "test-strings-value10"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); option = DelExOption::IFEQ; @@ -267,10 +275,10 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value69", value); + EXPECT_EQ("test-strings-value10", value); option = DelExOption::IFEQ; - hash_or_value = "test-strings-value69"; + hash_or_value = "test-strings-value10"; deleted = 0; s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.ok()); @@ -279,12 +287,14 @@ TEST_F(RedisStringTest, DelEX) { status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); - EXPECT_NE("test-strings-value69", value); + EXPECT_NE("test-strings-value10", value); + key = "test-string-key10"; + value = "test-strings-value10"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); option = DelExOption::IFNE; - hash_or_value = "test-strings-value69"; + hash_or_value = "test-strings-value10"; deleted = 0; s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); EXPECT_TRUE(s.ok()); @@ -292,7 +302,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value69", value); + EXPECT_EQ("test-strings-value10", value); option = DelExOption::IFNE; hash_or_value = "random"; @@ -304,7 +314,7 @@ TEST_F(RedisStringTest, DelEX) { status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); - EXPECT_NE("test-strings-value69", value); + EXPECT_NE("test-strings-value10", value); } TEST_F(RedisStringTest, GetDel) { From 56030ea827b917fbd5fd017b5a8f7dfb8472a6ad Mon Sep 17 00:00:00 2001 From: Valay Date: Tue, 30 Dec 2025 22:25:46 -0800 Subject: [PATCH 08/21] Added XXH3 Digest and Updated Code --- src/commands/cmd_string.cc | 19 ++--- src/common/string_util.cc | 6 ++ src/common/string_util.h | 4 +- src/types/redis_string.cc | 29 ++++---- src/types/redis_string.h | 9 ++- tests/cppunit/types/string_test.cc | 71 +++++++++++-------- .../gocase/unit/type/strings/strings_test.go | 54 +++++++------- 7 files changed, 103 insertions(+), 89 deletions(-) diff --git a/src/commands/cmd_string.cc b/src/commands/cmd_string.cc index 0505a83178d..54f8e696d65 100644 --- a/src/commands/cmd_string.cc +++ b/src/commands/cmd_string.cc @@ -118,17 +118,13 @@ class CommandDelEX : public Commander { CommandParser parser(args, 2); while (parser.Good()) { if (parser.EatEqICase("ifdeq")) { - option = DelExOption::IFDEQ; - hash_or_value = GET_OR_RET(parser.TakeStr()); + option = {DelExOption::Type::IFDEQ, GET_OR_RET(parser.TakeStr())}; } else if (parser.EatEqICase("ifdne")) { - option = DelExOption::IFDNE; - hash_or_value = GET_OR_RET(parser.TakeStr()); + option = {DelExOption::Type::IFDNE, GET_OR_RET(parser.TakeStr())}; } else if (parser.EatEqICase("ifeq")) { - option = DelExOption::IFEQ; - hash_or_value = GET_OR_RET(parser.TakeStr()); + option = {DelExOption::Type::IFEQ, GET_OR_RET(parser.TakeStr())}; } else if (parser.EatEqICase("ifne")) { - option = DelExOption::IFNE; - hash_or_value = GET_OR_RET(parser.TakeStr()); + option = {DelExOption::Type::IFNE, GET_OR_RET(parser.TakeStr())}; } else { return {Status::RedisParseErr, errInvalidSyntax}; } @@ -138,13 +134,13 @@ class CommandDelEX : public Commander { Status Execute(engine::Context &ctx, Server *srv, Connection *conn, std::string *output) override { redis::String string_db(srv->storage, conn->GetNamespace()); - auto s = string_db.DelEX(ctx, args_[1], option, hash_or_value, deleted); + auto s = string_db.DelEX(ctx, args_[1], option, deleted); if (!s.ok() && !s.IsNotFound()) { return {Status::RedisExecErr, s.ToString()}; } - if (s.IsNotFound() || (option == DelExOption::NONE && !deleted)) { + if (s.IsNotFound() || (option.type == DelExOption::Type::NONE && !deleted)) { *output = redis::Integer(0); } else { *output = redis::Integer(1); @@ -153,8 +149,7 @@ class CommandDelEX : public Commander { } private: - std::optional hash_or_value = std::nullopt; - DelExOption option = DelExOption::NONE; + DelExOption option = {DelExOption::Type::NONE, ""}; bool deleted = false; }; diff --git a/src/common/string_util.cc b/src/common/string_util.cc index 2342db323fc..c3cbc511e31 100644 --- a/src/common/string_util.cc +++ b/src/common/string_util.cc @@ -26,6 +26,7 @@ #include #include "parse_util.h" +#include "xxh3.h" namespace util { @@ -556,4 +557,9 @@ std::string StringNext(std::string s) { return s; } +std::string StringDigest(std::string_view data) { + XXH64_hash_t hash = XXH3_64bits(data.data(), data.size()); + return fmt::format("{:016x}", hash); +} + } // namespace util diff --git a/src/common/string_util.h b/src/common/string_util.h index 88232873ad5..1bea3a5813b 100644 --- a/src/common/string_util.h +++ b/src/common/string_util.h @@ -62,6 +62,7 @@ std::string StringToHex(std::string_view input); std::vector TokenizeRedisProtocol(const std::string &value); std::string EscapeString(std::string_view s); std::string StringNext(std::string s); +std::string StringDigest(std::string_view data); template , int> = 0> std::string StringJoin(const T &con, F &&f, std::string_view sep = ", ") { @@ -80,7 +81,8 @@ std::string StringJoin(const T &con, F &&f, std::string_view sep = ", ") { template std::string StringJoin(const T &con, std::string_view sep = ", ") { - return StringJoin(con, [](const auto &v) -> decltype(auto) { return v; }, sep); + return StringJoin( + con, [](const auto &v) -> decltype(auto) { return v; }, sep); } } // namespace util diff --git a/src/types/redis_string.cc b/src/types/redis_string.cc index f9f2aa923bb..2419e322191 100644 --- a/src/types/redis_string.cc +++ b/src/types/redis_string.cc @@ -182,35 +182,34 @@ rocksdb::Status String::GetEx(engine::Context &ctx, const std::string &user_key, return rocksdb::Status::OK(); } -rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, DelExOption &option, - std::optional &hash_or_value, bool &deleted) { +rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, DelExOption &option, bool &deleted) { std::string ns_key = AppendNamespacePrefix(user_key); std::string value; rocksdb::Status s = getValue(ctx, ns_key, &value); - if (!s.ok() || s.IsNotFound()) { - return s; - } - if (option == DelExOption::NONE && !hash_or_value.has_value()) { + if (!s.ok()) return s; + + if (option.type == DelExOption::Type::NONE && option.value == "") { return storage_->Delete(ctx, storage_->DefaultWriteOptions(), metadata_cf_handle_, ns_key); } - switch (option) { - case DelExOption::IFDEQ: - if (hash_or_value.value() == "12345") { // StringDigest(value) + + switch (option.type) { + case DelExOption::Type::IFDEQ: + if (option.value == util::StringDigest(value)) { deleted = true; } break; - case DelExOption::IFDNE: - if (hash_or_value.value() != "12345") { // StringDigest(value) + case DelExOption::Type::IFDNE: + if (option.value != util::StringDigest(value)) { deleted = true; } break; - case DelExOption::IFEQ: - if (hash_or_value.value() == value) { + case DelExOption::Type::IFEQ: + if (option.value == value) { deleted = true; } break; - case DelExOption::IFNE: - if (hash_or_value.value() != value) { + case DelExOption::Type::IFNE: + if (option.value != value) { deleted = true; } break; diff --git a/src/types/redis_string.h b/src/types/redis_string.h index bcbc374a943..027c16e298a 100644 --- a/src/types/redis_string.h +++ b/src/types/redis_string.h @@ -34,7 +34,11 @@ struct StringPair { Slice value; }; -enum class DelExOption { NONE, IFDEQ, IFDNE, IFEQ, IFNE }; +struct DelExOption { + enum class Type { NONE, IFDEQ, IFDNE, IFEQ, IFNE }; + Type type; + std::string value; +}; enum class StringSetType { NONE, NX, XX }; @@ -84,8 +88,7 @@ class String : public Database { rocksdb::Status Get(engine::Context &ctx, const std::string &user_key, std::string *value); rocksdb::Status GetEx(engine::Context &ctx, const std::string &user_key, std::string *value, std::optional expire); - rocksdb::Status DelEX(engine::Context &ctx, const std::string &user_key, DelExOption &option, - std::optional &hash_or_value, bool &deleted); + rocksdb::Status DelEX(engine::Context &ctx, const std::string &user_key, DelExOption &option, bool &deleted); rocksdb::Status GetSet(engine::Context &ctx, const std::string &user_key, const std::string &new_value, std::optional &old_value); rocksdb::Status GetDel(engine::Context &ctx, const std::string &user_key, std::string *value); diff --git a/tests/cppunit/types/string_test.cc b/tests/cppunit/types/string_test.cc index 430865d3c03..995b1af4057 100644 --- a/tests/cppunit/types/string_test.cc +++ b/tests/cppunit/types/string_test.cc @@ -159,8 +159,7 @@ TEST_F(RedisStringTest, GetSet) { } TEST_F(RedisStringTest, DelEX) { - std::optional hash_or_value = std::nullopt; - DelExOption option = DelExOption::NONE; + DelExOption option = {DelExOption::Type::NONE, ""}; bool deleted = 0; std::string key = "test-string-key69"; @@ -172,23 +171,23 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_EQ("test-strings-value69", value); // Check no args delete works - auto s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); + auto s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); EXPECT_FALSE(deleted); - EXPECT_EQ(option, DelExOption::NONE); + EXPECT_EQ(option.type, DelExOption::Type::NONE); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok() && status.IsNotFound()); EXPECT_NE("test-strings-value69", value); // Check no args delete on same key - s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); + s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_FALSE(s.ok()); EXPECT_TRUE(s.IsNotFound()); // Check no args delete on invalid/notfound key key = "random"; - s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); + s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_FALSE(s.ok()); EXPECT_TRUE(s.IsNotFound()); status = string_->Get(*ctx_, key, &value); @@ -199,21 +198,23 @@ TEST_F(RedisStringTest, DelEX) { value = "test-strings-value69"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); - option = DelExOption::NONE; - hash_or_value = "test-strings-value69"; - s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); + option.type = DelExOption::Type::NONE; + option.value = "test-strings-value69"; + s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.IsInvalidArgument()); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); EXPECT_EQ("test-strings-value69", value); // Checking true false cases for all args + key = "test-string-key69"; + value = "test-strings-value69"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); - option = DelExOption::IFDEQ; - hash_or_value = "xxxxxxxxxxxxxxxx"; + option.type = DelExOption::Type::IFDEQ; + option.value = "xxxxxxxxxxxxxxxx"; deleted = 0; - s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); + s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); EXPECT_FALSE(deleted); @@ -221,9 +222,10 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(status.ok() && !status.IsNotFound()); EXPECT_EQ("test-strings-value69", value); - hash_or_value = "12345"; // StringDigest(value); + option.type = DelExOption::Type::IFDEQ; + option.value = util::StringDigest(value); deleted = 0; - s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); + s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); EXPECT_TRUE(deleted); @@ -232,12 +234,14 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(status.IsNotFound()); EXPECT_NE("test-strings-value69", value); + key = "test-string-key69"; + value = "test-strings-value69"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); - option = DelExOption::IFDNE; - hash_or_value = "12345"; // StringDigest(value); + option.type = DelExOption::Type::IFDNE; + option.value = util::StringDigest(value); deleted = 0; - s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); + s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); EXPECT_FALSE(deleted); @@ -245,9 +249,10 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(status.ok() && !status.IsNotFound()); EXPECT_EQ("test-strings-value69", value); - hash_or_value = "xxxxxxxxxxxxxxxx"; + option.type = DelExOption::Type::IFDNE; + option.value = "xxxxxxxxxxxxxxxx"; deleted = 0; - s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); + s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); EXPECT_TRUE(deleted); @@ -256,12 +261,14 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(status.IsNotFound()); EXPECT_NE("test-strings-value69", value); + key = "test-string-key69"; + value = "test-strings-value69"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); - option = DelExOption::IFEQ; - hash_or_value = "random"; + option.type = DelExOption::Type::IFEQ; + option.value = "random"; deleted = 0; - s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); + s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); EXPECT_FALSE(deleted); @@ -269,10 +276,10 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(status.ok() && !status.IsNotFound()); EXPECT_EQ("test-strings-value69", value); - option = DelExOption::IFEQ; - hash_or_value = "test-strings-value69"; + option.type = DelExOption::Type::IFEQ; + option.value = "test-strings-value69"; deleted = 0; - s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); + s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); EXPECT_TRUE(deleted); @@ -281,12 +288,14 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(status.IsNotFound()); EXPECT_NE("test-strings-value69", value); + key = "test-string-key69"; + value = "test-strings-value69"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); - option = DelExOption::IFNE; - hash_or_value = "test-strings-value69"; + option.type = DelExOption::Type::IFNE; + option.value = "test-strings-value69"; deleted = 0; - s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); + s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); EXPECT_FALSE(deleted); @@ -294,10 +303,10 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(status.ok() && !status.IsNotFound()); EXPECT_EQ("test-strings-value69", value); - option = DelExOption::IFNE; - hash_or_value = "random"; + option.type = DelExOption::Type::IFNE; + option.value = "random"; deleted = 0; - s = string_->DelEX(*ctx_, key, option, hash_or_value, deleted); + s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); EXPECT_TRUE(deleted); diff --git a/tests/gocase/unit/type/strings/strings_test.go b/tests/gocase/unit/type/strings/strings_test.go index 17270f4d1e7..3b209f30e29 100644 --- a/tests/gocase/unit/type/strings/strings_test.go +++ b/tests/gocase/unit/type/strings/strings_test.go @@ -277,76 +277,76 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { require.Equal(t, "", rdb.GetDel(ctx, "foo").Val()) }) - t.Run("DELEX command no args", func(t *testing.T) { + t.Run("DelEX command no args", func(t *testing.T) { key := "test-string-key69" value := "test-strings-value69" require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.Equal(t, "1", rdb.DelEX(ctx, key).Val()) + require.Equal(t, "1", rdb.Do(ctx, "DelEX", key).Val()) require.Equal(t, "", rdb.Get(ctx, key).Val()) //on same key - require.Error(t, rdb.DelEX(ctx,key).Err()) - require.Equal(t, "0", rdb.Do(ctx, "DELEX", "test-string-key69").Val()) + require.Error(t, rdb.Do(ctx, "DelEX", key).Err()) + require.Equal(t, "0", rdb.Do(ctx, "DelEX", "test-string-key69").Val()) //on non existent key key := "random" require.Equal(t, "", rdb.Get(ctx, key).Val()) - require.Error(t, rdb.DelEX(ctx,key).Err()) - require.Equal(t, "0", rdb.Do(ctx, "DELEX", "random").Val()) + require.Error(t, rdb.Do(ctx, "DelEX", key).Err()) + require.Equal(t, "0", rdb.Do(ctx, "DelEX", "random").Val()) }) - t.Run("DELEX command with args", func(t *testing.T) { + t.Run("DelEX command with args", func(t *testing.T) { key := "test-string-key69" value := "test-strings-value69" require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) // More than 4 args - require.Error(t, rdb.Do(ctx, "DELEX", "test-string-key69", "random", "random", "random").Err()) + require.Error(t, rdb.Do(ctx, "DelEX", "test-string-key69", "random", "random", "random").Err()) //Incorrect option - require.Error(t, rdb.Do(ctx, "DELEX", "test-string-key69", "random", "random").Err()) - //True cases for all options - //digest1 := rdb.Do(ctx, "DIGEST", value).Val().(string) - require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Err()) - require.Equal(t, "0", rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Val()) + require.Error(t, rdb.Do(ctx, "DelEX", "test-string-key69", "random", "random").Err()) + //Cases for all options + //digest := rdb.Do(ctx, "DIGEST", value).Val().(string) + require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Err()) + require.Equal(t, "0", rdb.Do(ctx, "DelEX","test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdeq", "12345").Err()) //digest1 + require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifdeq", "12345").Err()) //digest require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, "1", rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", "12345").Val()) //digest1 + require.Equal(t, "1", rdb.Do(ctx, "DelEX","test-string-key69", "ifdeq", "12345").Val()) //digest require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdne", "12345").Err()) //digest1 - require.Equal(t, "0", rdb.Do(ctx, "DELEX","test-string-key69", "ifdne", "12345").Val()) //digest1 + require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifdne", "12345").Err()) //digest + require.Equal(t, "0", rdb.Do(ctx, "DelEX","test-string-key69", "ifdne", "12345").Val()) //digest require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdne", "xxxxxxxxxxxxxxxx").Err()) + require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifdne", "xxxxxxxxxxxxxxxx").Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, "1", rdb.Do(ctx, "DELEX","test-string-key69", "ifdne", "xxxxxxxxxxxxxxxx").Val()) + require.Equal(t, "1", rdb.Do(ctx, "DelEX","test-string-key69", "ifdne", "xxxxxxxxxxxxxxxx").Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifeq", "random").Err()) - require.Equal(t, "0", rdb.Do(ctx, "DELEX","test-string-key69", "ifeq", "random").Val()) + require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifeq", "random").Err()) + require.Equal(t, "0", rdb.Do(ctx, "DelEX","test-string-key69", "ifeq", "random").Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifeq", "test-strings-value69").Err()) + require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifeq", "test-strings-value69").Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, "1", rdb.Do(ctx, "DELEX","test-string-key69", "ifeq", "test-strings-value69").Val()) + require.Equal(t, "1", rdb.Do(ctx, "DelEX","test-string-key69", "ifeq", "test-strings-value69").Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifne", "test-strings-value69").Err()) - require.Equal(t, "0", rdb.Do(ctx, "DELEX","test-string-key69", "ifne", "test-strings-value69").Val()) + require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifne", "test-strings-value69").Err()) + require.Equal(t, "0", rdb.Do(ctx, "DelEX","test-string-key69", "ifne", "test-strings-value69").Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifne", "random").Err()) + require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifne", "random").Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, "1", rdb.Do(ctx, "DELEX","test-string-key69", "ifne", "random").Val()) + require.Equal(t, "1", rdb.Do(ctx, "DelEX","test-string-key69", "ifne", "random").Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) }) From bf0632eeb004158a6dc6fb0f7bcb841cf0a813da Mon Sep 17 00:00:00 2001 From: Valay Date: Wed, 31 Dec 2025 01:10:32 -0800 Subject: [PATCH 09/21] Fix Test Cases CPP --- tests/cppunit/types/string_test.cc | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/cppunit/types/string_test.cc b/tests/cppunit/types/string_test.cc index 0a38693201a..995b1af4057 100644 --- a/tests/cppunit/types/string_test.cc +++ b/tests/cppunit/types/string_test.cc @@ -162,13 +162,13 @@ TEST_F(RedisStringTest, DelEX) { DelExOption option = {DelExOption::Type::NONE, ""}; bool deleted = 0; - std::string key = "test-string-key10"; - std::string value = "test-strings-value10"; + std::string key = "test-string-key69"; + std::string value = "test-strings-value69"; auto status = string_->Set(*ctx_, key, value); ASSERT_TRUE(status.ok()); status = string_->Get(*ctx_, key, &value); ASSERT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value10", value); + EXPECT_EQ("test-strings-value69", value); // Check no args delete works auto s = string_->DelEX(*ctx_, key, option, deleted); @@ -178,7 +178,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_EQ(option.type, DelExOption::Type::NONE); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok() && status.IsNotFound()); - EXPECT_NE("test-strings-value10", value); + EXPECT_NE("test-strings-value69", value); // Check no args delete on same key s = string_->DelEX(*ctx_, key, option, deleted); @@ -194,8 +194,8 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(!status.ok() && status.IsNotFound()); // Correct value but incorrect option - key = "test-string-key10"; - value = "test-strings-value10"; + key = "test-string-key69"; + value = "test-strings-value69"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); option.type = DelExOption::Type::NONE; @@ -204,7 +204,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(s.IsInvalidArgument()); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value10", value); + EXPECT_EQ("test-strings-value69", value); // Checking true false cases for all args key = "test-string-key69"; @@ -220,7 +220,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value10", value); + EXPECT_EQ("test-strings-value69", value); option.type = DelExOption::Type::IFDEQ; option.value = util::StringDigest(value); @@ -232,7 +232,7 @@ TEST_F(RedisStringTest, DelEX) { status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); - EXPECT_NE("test-strings-value10", value); + EXPECT_NE("test-strings-value69", value); key = "test-string-key69"; value = "test-strings-value69"; @@ -247,7 +247,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value10", value); + EXPECT_EQ("test-strings-value69", value); option.type = DelExOption::Type::IFDNE; option.value = "xxxxxxxxxxxxxxxx"; @@ -259,7 +259,7 @@ TEST_F(RedisStringTest, DelEX) { status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); - EXPECT_NE("test-strings-value10", value); + EXPECT_NE("test-strings-value69", value); key = "test-string-key69"; value = "test-strings-value69"; @@ -274,7 +274,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value10", value); + EXPECT_EQ("test-strings-value69", value); option.type = DelExOption::Type::IFEQ; option.value = "test-strings-value69"; @@ -286,7 +286,7 @@ TEST_F(RedisStringTest, DelEX) { status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); - EXPECT_NE("test-strings-value10", value); + EXPECT_NE("test-strings-value69", value); key = "test-string-key69"; value = "test-strings-value69"; @@ -301,7 +301,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value10", value); + EXPECT_EQ("test-strings-value69", value); option.type = DelExOption::Type::IFNE; option.value = "random"; @@ -313,7 +313,7 @@ TEST_F(RedisStringTest, DelEX) { status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok()); EXPECT_TRUE(status.IsNotFound()); - EXPECT_NE("test-strings-value10", value); + EXPECT_NE("test-strings-value69", value); } TEST_F(RedisStringTest, GetDel) { From ac0beffc5a13eb9649e2a2ddb6a38f6403667c67 Mon Sep 17 00:00:00 2001 From: Valay Date: Wed, 31 Dec 2025 16:31:44 -0800 Subject: [PATCH 10/21] Added Suggestions and Fixed GO Cases --- src/commands/cmd_string.cc | 4 ++-- src/types/redis_string.cc | 5 +++-- src/types/redis_string.h | 5 ++++- tests/cppunit/types/string_test.cc | 2 +- tests/gocase/unit/type/strings/strings_test.go | 8 ++++---- 5 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/commands/cmd_string.cc b/src/commands/cmd_string.cc index 54f8e696d65..70edb144bda 100644 --- a/src/commands/cmd_string.cc +++ b/src/commands/cmd_string.cc @@ -140,7 +140,7 @@ class CommandDelEX : public Commander { return {Status::RedisExecErr, s.ToString()}; } - if (s.IsNotFound() || (option.type == DelExOption::Type::NONE && !deleted)) { + if (s.IsNotFound() || !deleted) { *output = redis::Integer(0); } else { *output = redis::Integer(1); @@ -149,7 +149,7 @@ class CommandDelEX : public Commander { } private: - DelExOption option = {DelExOption::Type::NONE, ""}; + DelExOption option; bool deleted = false; }; diff --git a/src/types/redis_string.cc b/src/types/redis_string.cc index 2419e322191..1c6ba8b1860 100644 --- a/src/types/redis_string.cc +++ b/src/types/redis_string.cc @@ -182,13 +182,15 @@ rocksdb::Status String::GetEx(engine::Context &ctx, const std::string &user_key, return rocksdb::Status::OK(); } -rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, DelExOption &option, bool &deleted) { +rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, const DelExOption &option, + bool &deleted) { std::string ns_key = AppendNamespacePrefix(user_key); std::string value; rocksdb::Status s = getValue(ctx, ns_key, &value); if (!s.ok()) return s; if (option.type == DelExOption::Type::NONE && option.value == "") { + deleted = true; return storage_->Delete(ctx, storage_->DefaultWriteOptions(), metadata_cf_handle_, ns_key); } @@ -215,7 +217,6 @@ rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, break; default: return rocksdb::Status::InvalidArgument(); - break; } if (deleted) { return storage_->Delete(ctx, storage_->DefaultWriteOptions(), metadata_cf_handle_, ns_key); diff --git a/src/types/redis_string.h b/src/types/redis_string.h index 027c16e298a..2e7a23f7d19 100644 --- a/src/types/redis_string.h +++ b/src/types/redis_string.h @@ -38,6 +38,9 @@ struct DelExOption { enum class Type { NONE, IFDEQ, IFDNE, IFEQ, IFNE }; Type type; std::string value; + + DelExOption() : type(Type::NONE) {} + DelExOption(Type type, std::string value) : type(type), value(std::move(value)) {} }; enum class StringSetType { NONE, NX, XX }; @@ -88,7 +91,7 @@ class String : public Database { rocksdb::Status Get(engine::Context &ctx, const std::string &user_key, std::string *value); rocksdb::Status GetEx(engine::Context &ctx, const std::string &user_key, std::string *value, std::optional expire); - rocksdb::Status DelEX(engine::Context &ctx, const std::string &user_key, DelExOption &option, bool &deleted); + rocksdb::Status DelEX(engine::Context &ctx, const std::string &user_key, const DelExOption &option, bool &deleted); rocksdb::Status GetSet(engine::Context &ctx, const std::string &user_key, const std::string &new_value, std::optional &old_value); rocksdb::Status GetDel(engine::Context &ctx, const std::string &user_key, std::string *value); diff --git a/tests/cppunit/types/string_test.cc b/tests/cppunit/types/string_test.cc index 995b1af4057..e5cd89dbaaa 100644 --- a/tests/cppunit/types/string_test.cc +++ b/tests/cppunit/types/string_test.cc @@ -174,7 +174,7 @@ TEST_F(RedisStringTest, DelEX) { auto s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); - EXPECT_FALSE(deleted); + EXPECT_TRUE(deleted); EXPECT_EQ(option.type, DelExOption::Type::NONE); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok() && status.IsNotFound()); diff --git a/tests/gocase/unit/type/strings/strings_test.go b/tests/gocase/unit/type/strings/strings_test.go index b044dd02374..ea5963867f0 100644 --- a/tests/gocase/unit/type/strings/strings_test.go +++ b/tests/gocase/unit/type/strings/strings_test.go @@ -289,9 +289,8 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { require.Error(t, rdb.Do(ctx, "DelEX", key).Err()) require.Equal(t, "0", rdb.Do(ctx, "DelEX", "test-string-key69").Val()) //on non existent key - key := "random" - require.Equal(t, "", rdb.Get(ctx, key).Val()) - require.Error(t, rdb.Do(ctx, "DelEX", key).Err()) + require.Equal(t, "", rdb.Get(ctx, "random").Val()) + require.Error(t, rdb.Do(ctx, "DelEX", "random").Err()) require.Equal(t, "0", rdb.Do(ctx, "DelEX", "random").Val()) }) @@ -306,7 +305,8 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { //Incorrect option require.Error(t, rdb.Do(ctx, "DELEX", "test-string-key69", "random", "random").Err()) //True cases for all options - //digest := rdb.Do(ctx, "DIGEST", value).Val().(string) + //digest := rdb.Do(ctx, "DIGEST", value).Val().(string) + // DIGEST command not yet implemented by issue #3309, once it is implemented it can be added require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Err()) require.Equal(t, "0", rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) From f6721b9c9428fdfe0e4d37788e4a70590609a5b8 Mon Sep 17 00:00:00 2001 From: Valay Date: Thu, 1 Jan 2026 00:44:07 -0800 Subject: [PATCH 11/21] Final Fixes --- src/commands/cmd_string.cc | 8 ++--- src/types/redis_string.cc | 10 +++---- src/types/redis_string.h | 4 +-- tests/cppunit/types/string_test.cc | 22 +++++++------- .../gocase/unit/type/strings/strings_test.go | 29 +++++++++---------- 5 files changed, 36 insertions(+), 37 deletions(-) diff --git a/src/commands/cmd_string.cc b/src/commands/cmd_string.cc index 70edb144bda..77ee5188bcb 100644 --- a/src/commands/cmd_string.cc +++ b/src/commands/cmd_string.cc @@ -118,13 +118,13 @@ class CommandDelEX : public Commander { CommandParser parser(args, 2); while (parser.Good()) { if (parser.EatEqICase("ifdeq")) { - option = {DelExOption::Type::IFDEQ, GET_OR_RET(parser.TakeStr())}; + option = {DelExOption::IFDEQ, GET_OR_RET(parser.TakeStr())}; } else if (parser.EatEqICase("ifdne")) { - option = {DelExOption::Type::IFDNE, GET_OR_RET(parser.TakeStr())}; + option = {DelExOption::IFDNE, GET_OR_RET(parser.TakeStr())}; } else if (parser.EatEqICase("ifeq")) { - option = {DelExOption::Type::IFEQ, GET_OR_RET(parser.TakeStr())}; + option = {DelExOption::IFEQ, GET_OR_RET(parser.TakeStr())}; } else if (parser.EatEqICase("ifne")) { - option = {DelExOption::Type::IFNE, GET_OR_RET(parser.TakeStr())}; + option = {DelExOption::IFNE, GET_OR_RET(parser.TakeStr())}; } else { return {Status::RedisParseErr, errInvalidSyntax}; } diff --git a/src/types/redis_string.cc b/src/types/redis_string.cc index 1c6ba8b1860..d2fd8207151 100644 --- a/src/types/redis_string.cc +++ b/src/types/redis_string.cc @@ -189,28 +189,28 @@ rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, rocksdb::Status s = getValue(ctx, ns_key, &value); if (!s.ok()) return s; - if (option.type == DelExOption::Type::NONE && option.value == "") { + if (option.type == DelExOption::NONE && option.value == "") { deleted = true; return storage_->Delete(ctx, storage_->DefaultWriteOptions(), metadata_cf_handle_, ns_key); } switch (option.type) { - case DelExOption::Type::IFDEQ: + case DelExOption::IFDEQ: if (option.value == util::StringDigest(value)) { deleted = true; } break; - case DelExOption::Type::IFDNE: + case DelExOption::IFDNE: if (option.value != util::StringDigest(value)) { deleted = true; } break; - case DelExOption::Type::IFEQ: + case DelExOption::IFEQ: if (option.value == value) { deleted = true; } break; - case DelExOption::Type::IFNE: + case DelExOption::IFNE: if (option.value != value) { deleted = true; } diff --git a/src/types/redis_string.h b/src/types/redis_string.h index 2e7a23f7d19..e78a58a6511 100644 --- a/src/types/redis_string.h +++ b/src/types/redis_string.h @@ -35,11 +35,11 @@ struct StringPair { }; struct DelExOption { - enum class Type { NONE, IFDEQ, IFDNE, IFEQ, IFNE }; + enum Type { NONE, IFDEQ, IFDNE, IFEQ, IFNE }; Type type; std::string value; - DelExOption() : type(Type::NONE) {} + DelExOption() : type(NONE) {} DelExOption(Type type, std::string value) : type(type), value(std::move(value)) {} }; diff --git a/tests/cppunit/types/string_test.cc b/tests/cppunit/types/string_test.cc index e5cd89dbaaa..6b6734397de 100644 --- a/tests/cppunit/types/string_test.cc +++ b/tests/cppunit/types/string_test.cc @@ -159,7 +159,7 @@ TEST_F(RedisStringTest, GetSet) { } TEST_F(RedisStringTest, DelEX) { - DelExOption option = {DelExOption::Type::NONE, ""}; + DelExOption option = {DelExOption::NONE, ""}; bool deleted = 0; std::string key = "test-string-key69"; @@ -175,7 +175,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); EXPECT_TRUE(deleted); - EXPECT_EQ(option.type, DelExOption::Type::NONE); + EXPECT_EQ(option.type, DelExOption::NONE); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok() && status.IsNotFound()); EXPECT_NE("test-strings-value69", value); @@ -198,7 +198,7 @@ TEST_F(RedisStringTest, DelEX) { value = "test-strings-value69"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); - option.type = DelExOption::Type::NONE; + option.type = DelExOption::NONE; option.value = "test-strings-value69"; s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.IsInvalidArgument()); @@ -211,7 +211,7 @@ TEST_F(RedisStringTest, DelEX) { value = "test-strings-value69"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); - option.type = DelExOption::Type::IFDEQ; + option.type = DelExOption::IFDEQ; option.value = "xxxxxxxxxxxxxxxx"; deleted = 0; s = string_->DelEX(*ctx_, key, option, deleted); @@ -222,7 +222,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(status.ok() && !status.IsNotFound()); EXPECT_EQ("test-strings-value69", value); - option.type = DelExOption::Type::IFDEQ; + option.type = DelExOption::IFDEQ; option.value = util::StringDigest(value); deleted = 0; s = string_->DelEX(*ctx_, key, option, deleted); @@ -238,7 +238,7 @@ TEST_F(RedisStringTest, DelEX) { value = "test-strings-value69"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); - option.type = DelExOption::Type::IFDNE; + option.type = DelExOption::IFDNE; option.value = util::StringDigest(value); deleted = 0; s = string_->DelEX(*ctx_, key, option, deleted); @@ -249,7 +249,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(status.ok() && !status.IsNotFound()); EXPECT_EQ("test-strings-value69", value); - option.type = DelExOption::Type::IFDNE; + option.type = DelExOption::IFDNE; option.value = "xxxxxxxxxxxxxxxx"; deleted = 0; s = string_->DelEX(*ctx_, key, option, deleted); @@ -265,7 +265,7 @@ TEST_F(RedisStringTest, DelEX) { value = "test-strings-value69"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); - option.type = DelExOption::Type::IFEQ; + option.type = DelExOption::IFEQ; option.value = "random"; deleted = 0; s = string_->DelEX(*ctx_, key, option, deleted); @@ -276,7 +276,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(status.ok() && !status.IsNotFound()); EXPECT_EQ("test-strings-value69", value); - option.type = DelExOption::Type::IFEQ; + option.type = DelExOption::IFEQ; option.value = "test-strings-value69"; deleted = 0; s = string_->DelEX(*ctx_, key, option, deleted); @@ -292,7 +292,7 @@ TEST_F(RedisStringTest, DelEX) { value = "test-strings-value69"; status = string_->Set(*ctx_, key, value); EXPECT_TRUE(status.ok()); - option.type = DelExOption::Type::IFNE; + option.type = DelExOption::IFNE; option.value = "test-strings-value69"; deleted = 0; s = string_->DelEX(*ctx_, key, option, deleted); @@ -303,7 +303,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(status.ok() && !status.IsNotFound()); EXPECT_EQ("test-strings-value69", value); - option.type = DelExOption::Type::IFNE; + option.type = DelExOption::IFNE; option.value = "random"; deleted = 0; s = string_->DelEX(*ctx_, key, option, deleted); diff --git a/tests/gocase/unit/type/strings/strings_test.go b/tests/gocase/unit/type/strings/strings_test.go index ea5963867f0..bfb7fd63e56 100644 --- a/tests/gocase/unit/type/strings/strings_test.go +++ b/tests/gocase/unit/type/strings/strings_test.go @@ -283,15 +283,15 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.Equal(t, "1", rdb.Do(ctx, "DelEX", key).Val()) + require.Equal(t, 1, rdb.Do(ctx, "DELEX", key).Val()) require.Equal(t, "", rdb.Get(ctx, key).Val()) //on same key require.Error(t, rdb.Do(ctx, "DelEX", key).Err()) - require.Equal(t, "0", rdb.Do(ctx, "DelEX", "test-string-key69").Val()) + require.Equal(t, 0, rdb.Do(ctx, "DELEX", "test-string-key69").Val()) //on non existent key require.Equal(t, "", rdb.Get(ctx, "random").Val()) require.Error(t, rdb.Do(ctx, "DelEX", "random").Err()) - require.Equal(t, "0", rdb.Do(ctx, "DelEX", "random").Val()) + require.Equal(t, 0, rdb.Do(ctx, "DELEX", "random").Val()) }) t.Run("DelEX command with args", func(t *testing.T) { @@ -305,48 +305,47 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { //Incorrect option require.Error(t, rdb.Do(ctx, "DELEX", "test-string-key69", "random", "random").Err()) //True cases for all options - //digest := rdb.Do(ctx, "DIGEST", value).Val().(string) - // DIGEST command not yet implemented by issue #3309, once it is implemented it can be added + digest := rdb.Do(ctx, "DIGEST", value).Val().(string) require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Err()) - require.Equal(t, "0", rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Val()) + require.Equal(t, 0, rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdeq", "12345").Err()) //digest + require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdeq", digest).Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, "1", rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", "12345").Val()) //digest + require.Equal(t, 1, rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", digest).Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdne", "12345").Err()) //digest - require.Equal(t, "0", rdb.Do(ctx, "DELEX","test-string-key69", "ifdne", "12345").Val()) //digest + require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdne", digest).Err()) + require.Equal(t, 0, rdb.Do(ctx, "DELEX","test-string-key69", "ifdne", digest).Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifdne", "xxxxxxxxxxxxxxxx").Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, "1", rdb.Do(ctx, "DelEX","test-string-key69", "ifdne", "xxxxxxxxxxxxxxxx").Val()) + require.Equal(t, 1, rdb.Do(ctx, "DELEX","test-string-key69", "ifdne", "xxxxxxxxxxxxxxxx").Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifeq", "random").Err()) - require.Equal(t, "0", rdb.Do(ctx, "DelEX","test-string-key69", "ifeq", "random").Val()) + require.Equal(t, 0, rdb.Do(ctx, "DELEX","test-string-key69", "ifeq", "random").Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifeq", "test-strings-value69").Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, "1", rdb.Do(ctx, "DelEX","test-string-key69", "ifeq", "test-strings-value69").Val()) + require.Equal(t, 1, rdb.Do(ctx, "DELEX","test-string-key69", "ifeq", "test-strings-value69").Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifne", "test-strings-value69").Err()) - require.Equal(t, "0", rdb.Do(ctx, "DelEX","test-string-key69", "ifne", "test-strings-value69").Val()) + require.Equal(t, 0, rdb.Do(ctx, "DELEX","test-string-key69", "ifne", "test-strings-value69").Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifne", "random").Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, "1", rdb.Do(ctx, "DelEX","test-string-key69", "ifne", "random").Val()) + require.Equal(t, 1, rdb.Do(ctx, "DELEX","test-string-key69", "ifne", "random").Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) }) From d918a887865b8d56906ec384b91cf65594a4e5c9 Mon Sep 17 00:00:00 2001 From: Valay Saitwadekar <51906989+Valay17@users.noreply.github.com> Date: Thu, 1 Jan 2026 01:40:48 -0800 Subject: [PATCH 12/21] Update src/types/redis_string.cc Co-authored-by: Twice --- src/types/redis_string.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types/redis_string.cc b/src/types/redis_string.cc index e4990d316a7..8f8af485667 100644 --- a/src/types/redis_string.cc +++ b/src/types/redis_string.cc @@ -190,7 +190,7 @@ rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, rocksdb::Status s = getValue(ctx, ns_key, &value); if (!s.ok()) return s; - if (option.type == DelExOption::NONE && option.value == "") { + if (option.type == DelExOption::NONE) { deleted = true; return storage_->Delete(ctx, storage_->DefaultWriteOptions(), metadata_cf_handle_, ns_key); } From dc1b264b0ad5577d2c72bbb749d5c195ac675a07 Mon Sep 17 00:00:00 2001 From: Valay Saitwadekar <51906989+Valay17@users.noreply.github.com> Date: Thu, 1 Jan 2026 01:41:47 -0800 Subject: [PATCH 13/21] Update src/types/redis_string.cc Co-authored-by: Twice --- src/types/redis_string.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/types/redis_string.cc b/src/types/redis_string.cc index 8f8af485667..9eecbce7e5c 100644 --- a/src/types/redis_string.cc +++ b/src/types/redis_string.cc @@ -185,6 +185,7 @@ rocksdb::Status String::GetEx(engine::Context &ctx, const std::string &user_key, rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, const DelExOption &option, bool &deleted) { + deleted = false; std::string ns_key = AppendNamespacePrefix(user_key); std::string value; rocksdb::Status s = getValue(ctx, ns_key, &value); From 524a4196193a4e6b19d393daad0cdf8106329b60 Mon Sep 17 00:00:00 2001 From: Valay Saitwadekar <51906989+Valay17@users.noreply.github.com> Date: Thu, 1 Jan 2026 01:47:25 -0800 Subject: [PATCH 14/21] Update string_util.h --- src/common/string_util.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/common/string_util.h b/src/common/string_util.h index 5eaca856d9a..9b2419eaf62 100644 --- a/src/common/string_util.h +++ b/src/common/string_util.h @@ -81,8 +81,7 @@ std::string StringJoin(const T &con, F &&f, std::string_view sep = ", ") { template std::string StringJoin(const T &con, std::string_view sep = ", ") { - return StringJoin( - con, [](const auto &v) -> decltype(auto) { return v; }, sep); + return StringJoin(con, [](const auto &v) -> decltype(auto) { return v; }, sep); } } // namespace util From 885c0e9568822bb6bb6e13b4c028bfe9c446a9af Mon Sep 17 00:00:00 2001 From: Valay Saitwadekar <51906989+Valay17@users.noreply.github.com> Date: Thu, 1 Jan 2026 01:58:38 -0800 Subject: [PATCH 15/21] Update strings_test.go --- .../gocase/unit/type/strings/strings_test.go | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/gocase/unit/type/strings/strings_test.go b/tests/gocase/unit/type/strings/strings_test.go index bfb7fd63e56..c5311c04d05 100644 --- a/tests/gocase/unit/type/strings/strings_test.go +++ b/tests/gocase/unit/type/strings/strings_test.go @@ -277,21 +277,21 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { require.Equal(t, "", rdb.GetDel(ctx, "foo").Val()) }) - t.Run("DelEX command no args", func(t *testing.T) { +t.Run("DelEX command no args", func(t *testing.T) { key := "test-string-key69" value := "test-strings-value69" require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.Equal(t, 1, rdb.Do(ctx, "DELEX", key).Val()) + require.Equal(t, int64(1), rdb.Do(ctx, "DELEX", key).Val()) require.Equal(t, "", rdb.Get(ctx, key).Val()) //on same key require.Error(t, rdb.Do(ctx, "DelEX", key).Err()) - require.Equal(t, 0, rdb.Do(ctx, "DELEX", "test-string-key69").Val()) + require.Equal(t, int64(0), rdb.Do(ctx, "DELEX", "test-string-key69").Val()) //on non existent key require.Equal(t, "", rdb.Get(ctx, "random").Val()) require.Error(t, rdb.Do(ctx, "DelEX", "random").Err()) - require.Equal(t, 0, rdb.Do(ctx, "DELEX", "random").Val()) + require.Equal(t, int64(0), rdb.Do(ctx, "DELEX", "random").Val()) }) t.Run("DelEX command with args", func(t *testing.T) { @@ -307,45 +307,45 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { //True cases for all options digest := rdb.Do(ctx, "DIGEST", value).Val().(string) require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Err()) - require.Equal(t, 0, rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Val()) + require.Equal(t, int64(0), rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdeq", digest).Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, 1, rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", digest).Val()) + require.Equal(t, int64(1), rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", digest).Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdne", digest).Err()) - require.Equal(t, 0, rdb.Do(ctx, "DELEX","test-string-key69", "ifdne", digest).Val()) + require.Equal(t, int64(0), rdb.Do(ctx, "DELEX","test-string-key69", "ifdne", digest).Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifdne", "xxxxxxxxxxxxxxxx").Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, 1, rdb.Do(ctx, "DELEX","test-string-key69", "ifdne", "xxxxxxxxxxxxxxxx").Val()) + require.Equal(t, int64(1), rdb.Do(ctx, "DELEX","test-string-key69", "ifdne", "xxxxxxxxxxxxxxxx").Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifeq", "random").Err()) - require.Equal(t, 0, rdb.Do(ctx, "DELEX","test-string-key69", "ifeq", "random").Val()) + require.Equal(t, int64(0), rdb.Do(ctx, "DELEX","test-string-key69", "ifeq", "random").Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifeq", "test-strings-value69").Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, 1, rdb.Do(ctx, "DELEX","test-string-key69", "ifeq", "test-strings-value69").Val()) + require.Equal(t, int64(1), rdb.Do(ctx, "DELEX","test-string-key69", "ifeq", "test-strings-value69").Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifne", "test-strings-value69").Err()) - require.Equal(t, 0, rdb.Do(ctx, "DELEX","test-string-key69", "ifne", "test-strings-value69").Val()) + require.Equal(t, int64(0), rdb.Do(ctx, "DELEX","test-string-key69", "ifne", "test-strings-value69").Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifne", "random").Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, 1, rdb.Do(ctx, "DELEX","test-string-key69", "ifne", "random").Val()) + require.Equal(t, int64(1), rdb.Do(ctx, "DELEX","test-string-key69", "ifne", "random").Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) }) From 82e8cd4dec55f900d0105d18575c7aeb567d8403 Mon Sep 17 00:00:00 2001 From: Twice Date: Thu, 1 Jan 2026 18:08:51 +0800 Subject: [PATCH 16/21] Update src/types/redis_string.cc --- src/types/redis_string.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/types/redis_string.cc b/src/types/redis_string.cc index dcf0f5221d1..45b1f27ef62 100644 --- a/src/types/redis_string.cc +++ b/src/types/redis_string.cc @@ -187,15 +187,15 @@ rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, bool &deleted) { deleted = false; std::string ns_key = AppendNamespacePrefix(user_key); - std::string value; - rocksdb::Status s = getValue(ctx, ns_key, &value); - if (!s.ok()) return s; - if (option.type == DelExOption::NONE) { deleted = true; return storage_->Delete(ctx, storage_->DefaultWriteOptions(), metadata_cf_handle_, ns_key); } + std::string value; + rocksdb::Status s = getValue(ctx, ns_key, &value); + if (!s.ok()) return s; + switch (option.type) { case DelExOption::IFDEQ: if (option.value == util::StringDigest(value)) { From 597f41f5a5959957e2ca8034e659d3b26ee8e7f4 Mon Sep 17 00:00:00 2001 From: Valay Date: Thu, 1 Jan 2026 19:53:03 -0800 Subject: [PATCH 17/21] Fix Issues and Comply with Documentation for Error --- src/types/redis_string.cc | 15 +++--- tests/cppunit/types/string_test.cc | 17 +----- .../gocase/unit/type/strings/strings_test.go | 52 ++++++++++--------- 3 files changed, 38 insertions(+), 46 deletions(-) diff --git a/src/types/redis_string.cc b/src/types/redis_string.cc index 45b1f27ef62..d4a805dbfa6 100644 --- a/src/types/redis_string.cc +++ b/src/types/redis_string.cc @@ -187,33 +187,36 @@ rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, bool &deleted) { deleted = false; std::string ns_key = AppendNamespacePrefix(user_key); + if (option.type == DelExOption::NONE) { + rocksdb::Status s = KeyExist(ctx, user_key); + if (s.IsNotFound()) return s; deleted = true; return storage_->Delete(ctx, storage_->DefaultWriteOptions(), metadata_cf_handle_, ns_key); } - std::string value; - rocksdb::Status s = getValue(ctx, ns_key, &value); + std::string val; + rocksdb::Status s = getValue(ctx, ns_key, &val); if (!s.ok()) return s; switch (option.type) { case DelExOption::IFDEQ: - if (option.value == util::StringDigest(value)) { + if (option.value == util::StringDigest(val)) { deleted = true; } break; case DelExOption::IFDNE: - if (option.value != util::StringDigest(value)) { + if (option.value != util::StringDigest(val)) { deleted = true; } break; case DelExOption::IFEQ: - if (option.value == value) { + if (option.value == val) { deleted = true; } break; case DelExOption::IFNE: - if (option.value != value) { + if (option.value != val) { deleted = true; } break; diff --git a/tests/cppunit/types/string_test.cc b/tests/cppunit/types/string_test.cc index e7251dc8bb8..93d92eb5230 100644 --- a/tests/cppunit/types/string_test.cc +++ b/tests/cppunit/types/string_test.cc @@ -182,30 +182,17 @@ TEST_F(RedisStringTest, DelEX) { // Check no args delete on same key s = string_->DelEX(*ctx_, key, option, deleted); - EXPECT_FALSE(s.ok()); EXPECT_TRUE(s.IsNotFound()); + EXPECT_FALSE(deleted); // Check no args delete on invalid/notfound key key = "random"; s = string_->DelEX(*ctx_, key, option, deleted); - EXPECT_FALSE(s.ok()); EXPECT_TRUE(s.IsNotFound()); + EXPECT_FALSE(deleted); status = string_->Get(*ctx_, key, &value); EXPECT_TRUE(!status.ok() && status.IsNotFound()); - // Correct value but incorrect option - key = "test-string-key69"; - value = "test-strings-value69"; - status = string_->Set(*ctx_, key, value); - EXPECT_TRUE(status.ok()); - option.type = DelExOption::NONE; - option.value = "test-strings-value69"; - s = string_->DelEX(*ctx_, key, option, deleted); - EXPECT_TRUE(s.IsInvalidArgument()); - status = string_->Get(*ctx_, key, &value); - EXPECT_TRUE(status.ok() && !status.IsNotFound()); - EXPECT_EQ("test-strings-value69", value); - // Checking true false cases for all args key = "test-string-key69"; value = "test-strings-value69"; diff --git a/tests/gocase/unit/type/strings/strings_test.go b/tests/gocase/unit/type/strings/strings_test.go index 1b0543c50f6..e40acdecf6d 100644 --- a/tests/gocase/unit/type/strings/strings_test.go +++ b/tests/gocase/unit/type/strings/strings_test.go @@ -277,7 +277,7 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { require.Equal(t, "", rdb.GetDel(ctx, "foo").Val()) }) -t.Run("DelEX command no args", func(t *testing.T) { + t.Run("DelEX command no args", func(t *testing.T) { key := "test-string-key69" value := "test-strings-value69" require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) @@ -286,66 +286,68 @@ t.Run("DelEX command no args", func(t *testing.T) { require.Equal(t, int64(1), rdb.Do(ctx, "DELEX", key).Val()) require.Equal(t, "", rdb.Get(ctx, key).Val()) //on same key - require.Error(t, rdb.Do(ctx, "DelEX", key).Err()) - require.Equal(t, int64(0), rdb.Do(ctx, "DELEX", "test-string-key69").Val()) + require.NoError(t, rdb.Do(ctx, "DelEX", key).Err()) + require.Equal(t, int64(0), rdb.Do(ctx, "DelEX", key).Val()) //on non existent key require.Equal(t, "", rdb.Get(ctx, "random").Val()) - require.Error(t, rdb.Do(ctx, "DelEX", "random").Err()) + require.NoError(t, rdb.Do(ctx, "DelEX", "random").Err()) require.Equal(t, int64(0), rdb.Do(ctx, "DELEX", "random").Val()) }) t.Run("DelEX command with args", func(t *testing.T) { key := "test-string-key69" - value := "test-strings-value69" + value := "Hello world" require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) // More than 4 args - require.Error(t, rdb.Do(ctx, "DelEX", "test-string-key69", "random", "random", "random").Err()) + r := rdb.Do(ctx, "DelEX", key, "random", "random", "random").Err() + require.ErrorContains(t, r, "wrong number") //Incorrect option - require.Error(t, rdb.Do(ctx, "DELEX", "test-string-key69", "random", "random").Err()) + r = rdb.Do(ctx, "DelEX", key, "random", "random").Err() + require.ErrorContains(t, r, "syntax error") //True cases for all options - digest := rdb.Do(ctx, "DIGEST", value).Val().(string) - require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Err()) - require.Equal(t, int64(0), rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", "xxxxxxxxxxxxxxxx").Val()) + digest := "b6acb9d84a38ff74" + require.NoError(t, rdb.Do(ctx, "DelEX", key, "ifdeq", "xxxxxxxxxxxxxxxx").Err()) + require.Equal(t, int64(0), rdb.Do(ctx, "DelEX", key, "ifdeq", "xxxxxxxxxxxxxxxx").Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdeq", digest).Err()) + require.NoError(t, rdb.Do(ctx, "DelEX", key, "ifdeq", digest).Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, int64(1), rdb.Do(ctx, "DELEX","test-string-key69", "ifdeq", digest).Val()) + require.Equal(t, int64(1), rdb.Do(ctx, "DELEX", key, "ifdeq", digest).Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) - + require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DELEX", "test-string-key69", "ifdne", digest).Err()) - require.Equal(t, int64(0), rdb.Do(ctx, "DELEX","test-string-key69", "ifdne", digest).Val()) + require.NoError(t, rdb.Do(ctx, "DelEX", key, "ifdne", digest).Err()) + require.Equal(t, int64(0), rdb.Do(ctx, "DelEX", key, "ifdne", digest).Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifdne", "xxxxxxxxxxxxxxxx").Err()) + require.NoError(t, rdb.Do(ctx, "DelEX", key, "ifdne", "xxxxxxxxxxxxxxxx").Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, int64(1), rdb.Do(ctx, "DELEX","test-string-key69", "ifdne", "xxxxxxxxxxxxxxxx").Val()) + require.Equal(t, int64(1), rdb.Do(ctx, "DelEX", key, "ifdne", "xxxxxxxxxxxxxxxx").Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifeq", "random").Err()) - require.Equal(t, int64(0), rdb.Do(ctx, "DELEX","test-string-key69", "ifeq", "random").Val()) + require.NoError(t, rdb.Do(ctx, "DelEX", key, "ifeq", "random").Err()) + require.Equal(t, int64(0), rdb.Do(ctx, "DelEX", key, "ifeq", "random").Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifeq", "test-strings-value69").Err()) + require.NoError(t, rdb.Do(ctx, "DelEX", key, "ifeq", value).Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, int64(1), rdb.Do(ctx, "DELEX","test-string-key69", "ifeq", "test-strings-value69").Val()) + require.Equal(t, int64(1), rdb.Do(ctx, "DelEX", key, "ifeq", value).Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifne", "test-strings-value69").Err()) - require.Equal(t, int64(0), rdb.Do(ctx, "DELEX","test-string-key69", "ifne", "test-strings-value69").Val()) + require.NoError(t, rdb.Do(ctx, "DelEX", key, "ifne", value).Err()) + require.Equal(t, int64(0), rdb.Do(ctx, "DelEX", key, "ifne", value).Val()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - require.NoError(t, rdb.Do(ctx, "DelEX", "test-string-key69", "ifne", "random").Err()) + require.NoError(t, rdb.Do(ctx, "DelEX", key, "ifne", "random").Err()) require.Equal(t, "", rdb.Get(ctx, value).Val()) require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) - require.Equal(t, int64(1), rdb.Do(ctx, "DELEX","test-string-key69", "ifne", "random").Val()) + require.Equal(t, int64(1), rdb.Do(ctx, "DelEX", key, "ifne", "random").Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) }) From 8a60c457eb9ba4dbf2f92de9f7381df87ee37577 Mon Sep 17 00:00:00 2001 From: Valay Date: Thu, 1 Jan 2026 22:48:15 -0800 Subject: [PATCH 18/21] Clang-tidy check --- src/commands/cmd_string.cc | 14 +++++++------- tests/cppunit/types/string_test.cc | 18 +++++++++--------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/commands/cmd_string.cc b/src/commands/cmd_string.cc index 645406f7602..fd43fda7263 100644 --- a/src/commands/cmd_string.cc +++ b/src/commands/cmd_string.cc @@ -118,13 +118,13 @@ class CommandDelEX : public Commander { CommandParser parser(args, 2); while (parser.Good()) { if (parser.EatEqICase("ifdeq")) { - option = {DelExOption::IFDEQ, GET_OR_RET(parser.TakeStr())}; + option_ = {DelExOption::IFDEQ, GET_OR_RET(parser.TakeStr())}; } else if (parser.EatEqICase("ifdne")) { - option = {DelExOption::IFDNE, GET_OR_RET(parser.TakeStr())}; + option_ = {DelExOption::IFDNE, GET_OR_RET(parser.TakeStr())}; } else if (parser.EatEqICase("ifeq")) { - option = {DelExOption::IFEQ, GET_OR_RET(parser.TakeStr())}; + option_ = {DelExOption::IFEQ, GET_OR_RET(parser.TakeStr())}; } else if (parser.EatEqICase("ifne")) { - option = {DelExOption::IFNE, GET_OR_RET(parser.TakeStr())}; + option_ = {DelExOption::IFNE, GET_OR_RET(parser.TakeStr())}; } else { return {Status::RedisParseErr, errInvalidSyntax}; } @@ -134,7 +134,8 @@ class CommandDelEX : public Commander { Status Execute(engine::Context &ctx, Server *srv, Connection *conn, std::string *output) override { redis::String string_db(srv->storage, conn->GetNamespace()); - auto s = string_db.DelEX(ctx, args_[1], option, deleted); + bool deleted = false; + auto s = string_db.DelEX(ctx, args_[1], option_, deleted); if (!s.ok() && !s.IsNotFound()) { return {Status::RedisExecErr, s.ToString()}; @@ -149,8 +150,7 @@ class CommandDelEX : public Commander { } private: - DelExOption option; - bool deleted = false; + DelExOption option_; }; class CommandStrlen : public Commander { diff --git a/tests/cppunit/types/string_test.cc b/tests/cppunit/types/string_test.cc index 93d92eb5230..f30ef162222 100644 --- a/tests/cppunit/types/string_test.cc +++ b/tests/cppunit/types/string_test.cc @@ -160,7 +160,7 @@ TEST_F(RedisStringTest, GetSet) { TEST_F(RedisStringTest, DelEX) { DelExOption option = {DelExOption::NONE, ""}; - bool deleted = 0; + bool deleted = false; std::string key = "test-string-key69"; std::string value = "test-strings-value69"; @@ -200,7 +200,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(status.ok()); option.type = DelExOption::IFDEQ; option.value = "xxxxxxxxxxxxxxxx"; - deleted = 0; + deleted = false; s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); @@ -211,7 +211,7 @@ TEST_F(RedisStringTest, DelEX) { option.type = DelExOption::IFDEQ; option.value = util::StringDigest(value); - deleted = 0; + deleted = false; s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); @@ -227,7 +227,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(status.ok()); option.type = DelExOption::IFDNE; option.value = util::StringDigest(value); - deleted = 0; + deleted = false; s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); @@ -238,7 +238,7 @@ TEST_F(RedisStringTest, DelEX) { option.type = DelExOption::IFDNE; option.value = "xxxxxxxxxxxxxxxx"; - deleted = 0; + deleted = false; s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); @@ -254,7 +254,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(status.ok()); option.type = DelExOption::IFEQ; option.value = "random"; - deleted = 0; + deleted = false; s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); @@ -265,7 +265,7 @@ TEST_F(RedisStringTest, DelEX) { option.type = DelExOption::IFEQ; option.value = "test-strings-value69"; - deleted = 0; + deleted = false; s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); @@ -281,7 +281,7 @@ TEST_F(RedisStringTest, DelEX) { EXPECT_TRUE(status.ok()); option.type = DelExOption::IFNE; option.value = "test-strings-value69"; - deleted = 0; + deleted = false; s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); @@ -292,7 +292,7 @@ TEST_F(RedisStringTest, DelEX) { option.type = DelExOption::IFNE; option.value = "random"; - deleted = 0; + deleted = false; s = string_->DelEX(*ctx_, key, option, deleted); EXPECT_TRUE(s.ok()); EXPECT_FALSE(s.IsNotFound()); From 0032f5b69c82670d5cf95ffa36e1d1fa21ef1dac Mon Sep 17 00:00:00 2001 From: Valay Date: Fri, 2 Jan 2026 16:12:12 -0800 Subject: [PATCH 19/21] Optimize Logic for option checking --- src/types/redis_string.cc | 37 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/src/types/redis_string.cc b/src/types/redis_string.cc index d4a805dbfa6..f7b26728803 100644 --- a/src/types/redis_string.cc +++ b/src/types/redis_string.cc @@ -186,47 +186,36 @@ rocksdb::Status String::GetEx(engine::Context &ctx, const std::string &user_key, rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, const DelExOption &option, bool &deleted) { deleted = false; - std::string ns_key = AppendNamespacePrefix(user_key); - - if (option.type == DelExOption::NONE) { - rocksdb::Status s = KeyExist(ctx, user_key); - if (s.IsNotFound()) return s; - deleted = true; - return storage_->Delete(ctx, storage_->DefaultWriteOptions(), metadata_cf_handle_, ns_key); - } - std::string val; + std::string ns_key = AppendNamespacePrefix(user_key); rocksdb::Status s = getValue(ctx, ns_key, &val); if (!s.ok()) return s; + bool matched = false; switch (option.type) { + case DelExOption::NONE: + matched = true; + break; case DelExOption::IFDEQ: - if (option.value == util::StringDigest(val)) { - deleted = true; - } + matched = option.value == util::StringDigest(val); break; case DelExOption::IFDNE: - if (option.value != util::StringDigest(val)) { - deleted = true; - } + matched = option.value != util::StringDigest(val); break; case DelExOption::IFEQ: - if (option.value == val) { - deleted = true; - } + matched = option.value == val; break; case DelExOption::IFNE: - if (option.value != val) { - deleted = true; - } + matched = option.value != val; break; default: return rocksdb::Status::InvalidArgument(); } - if (deleted) { - return storage_->Delete(ctx, storage_->DefaultWriteOptions(), metadata_cf_handle_, ns_key); + if (matched) { + s = storage_->Delete(ctx, storage_->DefaultWriteOptions(), metadata_cf_handle_, ns_key); + deleted = s.ok(); } - return rocksdb::Status::OK(); + return s; } rocksdb::Status String::GetSet(engine::Context &ctx, const std::string &user_key, const std::string &new_value, From 3e3cd1687785ae637ddb0067780ff9e1aac2698a Mon Sep 17 00:00:00 2001 From: Valay Saitwadekar <51906989+Valay17@users.noreply.github.com> Date: Fri, 2 Jan 2026 21:44:43 -0800 Subject: [PATCH 20/21] Update strings_test.go --- tests/gocase/unit/type/strings/strings_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/gocase/unit/type/strings/strings_test.go b/tests/gocase/unit/type/strings/strings_test.go index e40acdecf6d..769ff90501b 100644 --- a/tests/gocase/unit/type/strings/strings_test.go +++ b/tests/gocase/unit/type/strings/strings_test.go @@ -285,10 +285,10 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { require.Equal(t, int64(1), rdb.Do(ctx, "DELEX", key).Val()) require.Equal(t, "", rdb.Get(ctx, key).Val()) - //on same key + require.NoError(t, rdb.Do(ctx, "DelEX", key).Err()) require.Equal(t, int64(0), rdb.Do(ctx, "DelEX", key).Val()) - //on non existent key + require.Equal(t, "", rdb.Get(ctx, "random").Val()) require.NoError(t, rdb.Do(ctx, "DelEX", "random").Err()) require.Equal(t, int64(0), rdb.Do(ctx, "DELEX", "random").Val()) @@ -300,13 +300,12 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - // More than 4 args r := rdb.Do(ctx, "DelEX", key, "random", "random", "random").Err() require.ErrorContains(t, r, "wrong number") - //Incorrect option + r = rdb.Do(ctx, "DelEX", key, "random", "random").Err() require.ErrorContains(t, r, "syntax error") - //True cases for all options + digest := "b6acb9d84a38ff74" require.NoError(t, rdb.Do(ctx, "DelEX", key, "ifdeq", "xxxxxxxxxxxxxxxx").Err()) require.Equal(t, int64(0), rdb.Do(ctx, "DelEX", key, "ifdeq", "xxxxxxxxxxxxxxxx").Val()) From 9e58efc4e52f861d42038c24f0f5fb4f0f3ab126 Mon Sep 17 00:00:00 2001 From: Valay Saitwadekar <51906989+Valay17@users.noreply.github.com> Date: Fri, 2 Jan 2026 23:42:00 -0800 Subject: [PATCH 21/21] Fix Go Lint Issue --- tests/gocase/unit/type/strings/strings_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/gocase/unit/type/strings/strings_test.go b/tests/gocase/unit/type/strings/strings_test.go index 769ff90501b..c1a68484754 100644 --- a/tests/gocase/unit/type/strings/strings_test.go +++ b/tests/gocase/unit/type/strings/strings_test.go @@ -285,10 +285,10 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { require.Equal(t, int64(1), rdb.Do(ctx, "DELEX", key).Val()) require.Equal(t, "", rdb.Get(ctx, key).Val()) - + require.NoError(t, rdb.Do(ctx, "DelEX", key).Err()) require.Equal(t, int64(0), rdb.Do(ctx, "DelEX", key).Val()) - + require.Equal(t, "", rdb.Get(ctx, "random").Val()) require.NoError(t, rdb.Do(ctx, "DelEX", "random").Err()) require.Equal(t, int64(0), rdb.Do(ctx, "DELEX", "random").Val()) @@ -299,13 +299,13 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { value := "Hello world" require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) - + r := rdb.Do(ctx, "DelEX", key, "random", "random", "random").Err() require.ErrorContains(t, r, "wrong number") - + r = rdb.Do(ctx, "DelEX", key, "random", "random").Err() require.ErrorContains(t, r, "syntax error") - + digest := "b6acb9d84a38ff74" require.NoError(t, rdb.Do(ctx, "DelEX", key, "ifdeq", "xxxxxxxxxxxxxxxx").Err()) require.Equal(t, int64(0), rdb.Do(ctx, "DelEX", key, "ifdeq", "xxxxxxxxxxxxxxxx").Val()) @@ -315,7 +315,7 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, int64(1), rdb.Do(ctx, "DELEX", key, "ifdeq", digest).Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) - + require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) require.NoError(t, rdb.Do(ctx, "DelEX", key, "ifdne", digest).Err()) @@ -337,7 +337,7 @@ func testString(t *testing.T, configs util.KvrocksServerConfigs) { require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, int64(1), rdb.Do(ctx, "DelEX", key, "ifeq", value).Val()) require.Equal(t, "", rdb.Get(ctx, value).Val()) - + require.NoError(t, rdb.Set(ctx, key, value, 0).Err()) require.Equal(t, value, rdb.Get(ctx, key).Val()) require.NoError(t, rdb.Do(ctx, "DelEX", key, "ifne", value).Err())