Skip to content

Conversation

@Valay17
Copy link

@Valay17 Valay17 commented Dec 30, 2025

Fixes #3310

Implement DELEX Command

Digest function commented out

@Valay17 Valay17 changed the title Implement DELEX Command feat(delex): Implement DELEX Command Dec 30, 2025
Comment on lines +119 to +133
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};
Copy link
Member

Choose a reason for hiding this comment

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

Please use an enum class instead of meaningless chars like 1, 2, 3, 4.

Comment on lines +185 to +186
rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, std::optional<char> &opt_,
std::optional<std::string> &hash_or_val_, std::optional<bool> &res_) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, std::optional<char> &opt_,
std::optional<std::string> &hash_or_val_, std::optional<bool> &res_) {
rocksdb::Status String::DelEX(engine::Context &ctx, const std::string &user_key, const DelExOption &option, bool &deleted) {

Comment on lines +198 to +204
if (hash_or_val_.value() == "12345") { // StringDigest(value)
res_ = true;
}
break;
case '2':
if (hash_or_val_.value() != "12345") { // StringDigest(value)
res_ = true;
Copy link
Member

Choose a reason for hiding this comment

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

You can just add the StringDigest function, otherwise this PR cannot be merged.

4},
std::get<StringLCSIdxResult>(rst));
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}
}

Copy link
Member

@PragmaTwice PragmaTwice left a comment

Choose a reason for hiding this comment

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

Hey thank you for your contribution, could you please add some golang test cases?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support of the DELEX command

2 participants