From ade014cf851e18501950e70a17b3965c79e4f19c Mon Sep 17 00:00:00 2001
From: Abhijat Malviya <abhijat@dragonflydb.io>
Date: Tue, 22 Apr 2025 17:20:29 +0530
Subject: [PATCH] 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/facade/reply_builder.cc    | 14 ++++++++++++++
 src/facade/reply_builder.h     |  2 ++
 src/server/hset_family.cc      |  6 ++++--
 src/server/hset_family_test.cc | 26 ++++++++++++++++++++++++++
 4 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/src/facade/reply_builder.cc b/src/facade/reply_builder.cc
index d910a72cc..daa11a882 100644
--- a/src/facade/reply_builder.cc
+++ b/src/facade/reply_builder.cc
@@ -458,4 +458,18 @@ void RedisReplyBuilder::SendEmptyArray() {
   StartArray(0);
 }
 
+void RedisReplyBuilder::SendBulkStrArrAsPairs(const ArgRange& strs) {
+  const size_t strs_size = strs.Size();
+  DCHECK(strs_size % 2 == 0) << "unexpected size of strings " << strs_size << ", expected pairs";
+  ReplyScope scope{this};
+  const bool is_resp3 = IsResp3();
+  StartArray(is_resp3 ? strs_size / 2 : strs_size);
+  for (size_t i = 0; i < strs_size; i += 2) {
+    if (is_resp3)
+      StartArray(2);
+    SendBulkString(strs[i]);
+    SendBulkString(strs[i + 1]);
+  }
+}
+
 }  // namespace facade
diff --git a/src/facade/reply_builder.h b/src/facade/reply_builder.h
index 9bfc6b7c4..693a40379 100644
--- a/src/facade/reply_builder.h
+++ b/src/facade/reply_builder.h
@@ -296,6 +296,8 @@ class RedisReplyBuilder : public RedisReplyBuilderBase {
 
   void StartArray(unsigned len);
   void SendEmptyArray();
+
+  void SendBulkStrArrAsPairs(const ArgRange& strs);
 };
 
 }  // namespace facade
diff --git a/src/server/hset_family.cc b/src/server/hset_family.cc
index 77871054d..42ab64c5d 100644
--- a/src/server/hset_family.cc
+++ b/src/server/hset_family.cc
@@ -1271,10 +1271,12 @@ 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 if (with_values)
+      rb->SendBulkStrArrAsPairs(*result);
     else
-      rb->SendBulkStrArr(*result, facade::RedisReplyBuilder::ARRAY);
+      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..0cc5cebd3 100644
--- a/src/server/hset_family_test.cc
+++ b/src/server/hset_family_test.cc
@@ -541,4 +541,30 @@ 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));
+    EXPECT_THAT(v.GetVec()[0], AnyOf("a", "b", "c"));
+    EXPECT_THAT(v.GetVec()[1], expected[v.GetVec()[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