Skip to content

Commit dec34bc

Browse files
kirito632kirito632
committed
feat(string): support IFEQ/IFNE/IFDEQ/IFDNE in SET command
Add IFEQ/IFNE/IFDEQ/IFDNE conditionals to SET command, extending the existing NX/XX/GET subcommand support. Move conditional SET tests into testString to inherit txn-context-enabled matrix, and add non-hex 16-char digest boundary test for IFDEQ/IFDNE.
1 parent c8a66e2 commit dec34bc

5 files changed

Lines changed: 500 additions & 6 deletions

File tree

‎src/commands/cmd_string.cc‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,24 @@ class CommandSet : public Commander {
352352
set_flag_ = StringSetType::NX;
353353
} else if (parser.EatEqICaseFlag("XX", set_flag)) {
354354
set_flag_ = StringSetType::XX;
355+
} else if (parser.EatEqICaseFlag("IFEQ", set_flag)) {
356+
set_flag_ = StringSetType::IFEQ;
357+
cmp_value_ = GET_OR_RET(parser.TakeStr());
358+
} else if (parser.EatEqICaseFlag("IFNE", set_flag)) {
359+
set_flag_ = StringSetType::IFNE;
360+
cmp_value_ = GET_OR_RET(parser.TakeStr());
361+
} else if (parser.EatEqICaseFlag("IFDEQ", set_flag)) {
362+
set_flag_ = StringSetType::IFDEQ;
363+
cmp_value_ = GET_OR_RET(parser.TakeStr());
364+
if (cmp_value_.size() != 16) {
365+
return {Status::RedisParseErr, "ERR digest must be exactly 16 hexadecimal characters"};
366+
}
367+
} else if (parser.EatEqICaseFlag("IFDNE", set_flag)) {
368+
set_flag_ = StringSetType::IFDNE;
369+
cmp_value_ = GET_OR_RET(parser.TakeStr());
370+
if (cmp_value_.size() != 16) {
371+
return {Status::RedisParseErr, "ERR digest must be exactly 16 hexadecimal characters"};
372+
}
355373
} else if (parser.EatEqICase("GET")) {
356374
get_ = true;
357375
} else {
@@ -366,7 +384,7 @@ class CommandSet : public Commander {
366384
std::optional<std::string> ret;
367385
redis::String string_db(srv->storage, conn->GetNamespace());
368386

369-
rocksdb::Status s = string_db.Set(ctx, args_[1], args_[2], {expire_, set_flag_, get_, keep_ttl_}, ret);
387+
rocksdb::Status s = string_db.Set(ctx, args_[1], args_[2], {expire_, set_flag_, get_, keep_ttl_, cmp_value_}, ret);
370388

371389
if (!s.ok()) {
372390
return {Status::RedisExecErr, s.ToString()};
@@ -393,6 +411,7 @@ class CommandSet : public Commander {
393411
bool get_ = false;
394412
bool keep_ttl_ = false;
395413
StringSetType set_flag_ = StringSetType::NONE;
414+
std::string cmp_value_;
396415
};
397416

398417
class CommandSetEX : public Commander {

‎src/types/redis_string.cc‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ rocksdb::Status String::Set(engine::Context &ctx, const std::string &user_key, c
239239
}
240240

241241
rocksdb::Status String::Set(engine::Context &ctx, const std::string &user_key, const std::string &value,
242-
StringSetArgs args, std::optional<std::string> &ret) {
242+
const StringSetArgs &args, std::optional<std::string> &ret) {
243243
uint64_t expire = 0;
244244
std::string ns_key = AppendNamespacePrefix(user_key);
245245

@@ -249,6 +249,26 @@ rocksdb::Status String::Set(engine::Context &ctx, const std::string &user_key, c
249249
uint64_t old_expire = 0;
250250
auto s = getValueAndExpire(ctx, ns_key, &old_value, &old_expire);
251251
if (!s.ok() && !s.IsNotFound() && !s.IsInvalidArgument()) return s;
252+
// If the existing key is not a string type, enforce expected behaviors:
253+
if (s.IsInvalidArgument()) {
254+
// For conditional comparisons (IFEQ/IFNE/IFDEQ/IFDNE), reading the old value is required,
255+
// so return the underlying WRONGTYPE (InvalidArgument) error.
256+
if (args.type == StringSetType::IFEQ || args.type == StringSetType::IFNE || args.type == StringSetType::IFDEQ ||
257+
args.type == StringSetType::IFDNE) {
258+
return s;
259+
}
260+
// For NX option, treat a wrong type as "key exists" so the condition is not met.
261+
if (args.type == StringSetType::NX) {
262+
// If GET is also specified, we need to return the WRONGTYPE error
263+
// because GET requires reading the old value.
264+
if (args.get) {
265+
return s;
266+
}
267+
if (!args.get) ret = std::nullopt;
268+
return rocksdb::Status::OK();
269+
}
270+
// For other options, continue (e.g., XX may still proceed since key exists).
271+
}
252272
// GET option
253273
if (args.get) {
254274
if (s.IsInvalidArgument()) {
@@ -271,6 +291,38 @@ rocksdb::Status String::Set(engine::Context &ctx, const std::string &user_key, c
271291
// if XX option given, the key didn't exist before: return nil
272292
if (!args.get) ret = std::nullopt;
273293
return rocksdb::Status::OK();
294+
} else if (args.type == StringSetType::IFEQ) {
295+
// condition met only when key exists AND value matches
296+
bool matched = s.ok() && (old_value == args.cmp_value);
297+
if (!matched) {
298+
if (!args.get) ret = std::nullopt;
299+
return rocksdb::Status::OK();
300+
}
301+
if (!args.get) ret = "";
302+
} else if (args.type == StringSetType::IFNE) {
303+
// condition not met when key exists AND value matches; key-not-found counts as met
304+
bool not_matched = s.ok() && (old_value == args.cmp_value);
305+
if (not_matched) {
306+
if (!args.get) ret = std::nullopt;
307+
return rocksdb::Status::OK();
308+
}
309+
if (!args.get) ret = "";
310+
} else if (args.type == StringSetType::IFDEQ) {
311+
// condition met only when key exists AND digest matches (case-insensitive)
312+
bool matched = s.ok() && util::EqualICase(util::StringDigest(old_value), args.cmp_value);
313+
if (!matched) {
314+
if (!args.get) ret = std::nullopt;
315+
return rocksdb::Status::OK();
316+
}
317+
if (!args.get) ret = "";
318+
} else if (args.type == StringSetType::IFDNE) {
319+
// condition not met when key exists AND digest matches (case-insensitive); key-not-found counts as met
320+
bool not_matched = s.ok() && util::EqualICase(util::StringDigest(old_value), args.cmp_value);
321+
if (not_matched) {
322+
if (!args.get) ret = std::nullopt;
323+
return rocksdb::Status::OK();
324+
}
325+
if (!args.get) ret = "";
274326
} else {
275327
// if GET option not given, make ret not nil
276328
if (!args.get) ret = "";

‎src/types/redis_string.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,15 @@ struct DelExOption {
4343
DelExOption(Type type, std::string value) : type(type), value(std::move(value)) {}
4444
};
4545

46-
enum class StringSetType { NONE, NX, XX };
46+
enum class StringSetType { NONE, NX, XX, IFEQ, IFNE, IFDEQ, IFDNE };
4747

4848
struct StringSetArgs {
4949
// Expire time in mill seconds.
5050
uint64_t expire;
5151
StringSetType type;
5252
bool get;
5353
bool keep_ttl;
54+
std::string cmp_value; // valid only when type is IFEQ/IFNE/IFDEQ/IFDNE
5455
};
5556

5657
struct StringMSetArgs {
@@ -103,8 +104,8 @@ class String : public Database {
103104
std::optional<std::string> &old_value);
104105
rocksdb::Status GetDel(engine::Context &ctx, const std::string &user_key, std::string *value);
105106
rocksdb::Status Set(engine::Context &ctx, const std::string &user_key, const std::string &value);
106-
rocksdb::Status Set(engine::Context &ctx, const std::string &user_key, const std::string &value, StringSetArgs args,
107-
std::optional<std::string> &ret);
107+
rocksdb::Status Set(engine::Context &ctx, const std::string &user_key, const std::string &value,
108+
const StringSetArgs &args, std::optional<std::string> &ret);
108109
rocksdb::Status SetEX(engine::Context &ctx, const std::string &user_key, const std::string &value,
109110
uint64_t expire_ms);
110111
rocksdb::Status SetNX(engine::Context &ctx, const std::string &user_key, const std::string &value, uint64_t expire_ms,

‎tests/cppunit/types/string_test.cc‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,3 +613,46 @@ TEST_F(RedisStringTest, LCS) {
613613
4},
614614
std::get<StringLCSIdxResult>(rst));
615615
}
616+
617+
TEST_F(RedisStringTest, SetIFDEQ) {
618+
std::string key = "ifdeq-key";
619+
std::string value = "hello";
620+
std::optional<std::string> ret;
621+
622+
// key not found → condition not met, no write
623+
auto s = string_->Set(*ctx_, key, "new", {0, StringSetType::IFDEQ, false, false, util::StringDigest(value)}, ret);
624+
EXPECT_TRUE(s.ok());
625+
EXPECT_FALSE(ret.has_value());
626+
std::string got;
627+
EXPECT_TRUE(string_->Get(*ctx_, key, &got).IsNotFound());
628+
629+
// set up the key
630+
string_->Set(*ctx_, key, value);
631+
632+
// digest matches → write succeeds
633+
ret = std::nullopt;
634+
s = string_->Set(*ctx_, key, "new", {0, StringSetType::IFDEQ, false, false, util::StringDigest(value)}, ret);
635+
EXPECT_TRUE(s.ok());
636+
EXPECT_TRUE(ret.has_value());
637+
string_->Get(*ctx_, key, &got);
638+
EXPECT_EQ("new", got);
639+
640+
// digest mismatches → no write
641+
ret = std::nullopt;
642+
s = string_->Set(*ctx_, key, "newer", {0, StringSetType::IFDEQ, false, false, "xxxxxxxxxxxxxxxx"}, ret);
643+
EXPECT_TRUE(s.ok());
644+
EXPECT_FALSE(ret.has_value());
645+
string_->Get(*ctx_, key, &got);
646+
EXPECT_EQ("new", got);
647+
648+
// empty string edge case: digest of "" is well-defined
649+
string_->Set(*ctx_, key, "");
650+
ret = std::nullopt;
651+
s = string_->Set(*ctx_, key, "nonempty", {0, StringSetType::IFDEQ, false, false, util::StringDigest("")}, ret);
652+
EXPECT_TRUE(s.ok());
653+
EXPECT_TRUE(ret.has_value());
654+
string_->Get(*ctx_, key, &got);
655+
EXPECT_EQ("nonempty", got);
656+
657+
EXPECT_TRUE(string_->Del(*ctx_, key).ok());
658+
}

0 commit comments

Comments
 (0)