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]

Reply via email to