Valay17 commented on code in PR #3317:
URL: https://github.com/apache/kvrocks/pull/3317#discussion_r2656984564
##########
src/types/redis_string.cc:
##########
@@ -182,6 +183,52 @@ 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, const DelExOption &option,
+ bool &deleted) {
+ deleted = false;
+ std::string ns_key = AppendNamespacePrefix(user_key);
Review Comment:
Agreed. At the moment, the Execute function in cmd_string is already written
with this behavior in mind. Please let me know if you’d prefer that I keep it
as it is, or if you’d like me to update it based on your suggestion.
My main reason for keeping it this way is that the Exists command checks for
the key a bit more efficiently. Since the “no arguments” case is really just
about verifying whether the key exists (and then deleting it), this approach
felt slightly faster than calling getValue.
That said, I wasn’t completely sure whether the Delete function already
handles this internally, as I couldn’t fully confirm it by tracing the parent
function calls. I’m happy to adjust this if there’s a clearer or more preferred
approach. Thank You!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]