From 30a98c4fac924765a03ef19ce23f7d68fedf420d Mon Sep 17 00:00:00 2001
From: Abhijat Malviya <abhijat@dragonflydb.io>
Date: Wed, 23 Apr 2025 18:22:02 +0530
Subject: [PATCH] chore(hset_family): Support resp3 format for hrandfield
 (#4978)

* chore(hset_family): Support resp3 format for hrandfield

Return nested arrays if hrandfield is used with values.

Signed-off-by: Abhijat Malviya <abhijat@dragonflydb.io>
---
 src/server/hset_family.cc      | 19 ++++++++++++++++---
 src/server/hset_family_test.cc | 27 +++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/src/server/hset_family.cc b/src/server/hset_family.cc
index 77871054d..f50fc11be 100644
--- a/src/server/hset_family.cc
+++ b/src/server/hset_family.cc
@@ -1271,10 +1271,23 @@ void HSetFamily::HRandField(CmdArgList args, const CommandContext& cmd_cntx) {
   auto* rb = static_cast<RedisReplyBuilder*>(cmd_cntx.rb);
   OpResult<StringVec> result = cmd_cntx.tx->ScheduleSingleHopT(std::move(cb));
   if (result) {
-    if ((result->size() == 1) && (args.size() == 1))
+    if (result->size() == 1 && args.size() == 1)
       rb->SendBulkString(result->front());
-    else
-      rb->SendBulkStrArr(*result, facade::RedisReplyBuilder::ARRAY);
+    else if (with_values) {
+      const auto result_size = result->size();
+      DCHECK(result_size % 2 == 0)
+          << "unexpected size of strings " << result_size << ", expected pairs";
+      SinkReplyBuilder::ReplyScope scope{rb};
+      const bool is_resp3 = rb->IsResp3();
+      rb->StartArray(is_resp3 ? result_size / 2 : result_size);
+      for (size_t i = 0; i < result_size; i += 2) {
+        if (is_resp3)
+          rb->StartArray(2);
+        rb->SendBulkString((*result)[i]);
+        rb->SendBulkString((*result)[i + 1]);
+      }
+    } else
+      rb->SendBulkStrArr(*result, RedisReplyBuilder::ARRAY);
   } else if (result.status() == OpStatus::KEY_NOTFOUND) {
     if (args.size() == 1)
       rb->SendNull();
diff --git a/src/server/hset_family_test.cc b/src/server/hset_family_test.cc
index 12497dba2..65fad97d8 100644
--- a/src/server/hset_family_test.cc
+++ b/src/server/hset_family_test.cc
@@ -541,4 +541,31 @@ TEST_F(HSetFamilyTest, KeyRemovedWhenEmpty) {
   test_cmd([&] { EXPECT_THAT(Run({"HSTRLEN", "a", "afield"}), IntArg(0)); }, "HSTRLEN");
 }
 
+TEST_F(HSetFamilyTest, HRandFieldRespFormat) {
+  absl::flat_hash_map<std::string, std::string> expected{
+      {"a", "1"},
+      {"b", "2"},
+      {"c", "3"},
+  };
+  Run({"HELLO", "3"});
+  EXPECT_THAT(Run({"HSET", "key", "a", "1", "b", "2", "c", "3"}), IntArg(3));
+  auto resp = Run({"HRANDFIELD", "key", "3", "WITHVALUES"});
+  EXPECT_THAT(resp, ArrLen(3));
+  for (const auto& v : resp.GetVec()) {
+    EXPECT_THAT(v, ArrLen(2));
+    const auto& kv = v.GetVec();
+    EXPECT_THAT(kv[0], AnyOf("a", "b", "c"));
+    EXPECT_THAT(kv[1], expected[kv[0].GetView()]);
+  }
+
+  Run({"HELLO", "2"});
+  resp = Run({"HRANDFIELD", "key", "3", "WITHVALUES"});
+  EXPECT_THAT(resp, ArrLen(6));
+  const auto& vec = resp.GetVec();
+  for (size_t i = 0; i < vec.size(); i += 2) {
+    EXPECT_THAT(vec[i], AnyOf("a", "b", "c"));
+    EXPECT_THAT(vec[i + 1], expected[vec[i].GetView()]);
+  }
+}
+
 }  // namespace dfly