Re: [PR] fix(set): return scalar for SRANDMEMBER without count [kvrocks]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #3601:
URL: https://github.com/apache/kvrocks/pull/3601#discussion_r3899673129


##########
src/commands/cmd_set.cc:
##########
@@ -219,12 +220,18 @@ class CommandSRandMember : public Commander {
     if (!s.ok()) {
       return {Status::RedisExecErr, s.ToString()};
     }
-    *output = conn->SetOfBulkStrings(members);
+
+    if (with_count_) {
+      *output = conn->SetOfBulkStrings(members);
+    } else {
+      *output = members.empty() ? conn->NilString() : redis::BulkString(members.front());
+    }

Review Comment:
   For `SRANDMEMBER key count`, the reply should be an array (even under RESP3). Using `conn->SetOfBulkStrings` emits a RESP3 Set (`~`), which is not Redis-compatible for SRANDMEMBER, and is also semantically wrong when `count` is negative (duplicates are allowed but a set reply implies uniqueness). Prefer emitting an array of bulk strings for the counted form.



-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org