From f5235b2d7660b163c739325ec9284f97d5b963be Mon Sep 17 00:00:00 2001
From: Wen Hui <wen.hui.ware@gmail.com>
Date: Mon, 22 Feb 2021 08:00:59 -0500
Subject: [PATCH] SRANDMEMBER RESP3 return should be Array, not Set (#8504)

SRANDMEMBER with negative count (non unique) can return the same member
multiple times, and the order of elements in the returned collection matters.
For these reasons returning a RESP3 Set type is not valid for the negative
count, but also not really valid for the positive (unique) variant either (the
command returns an array of random picks, not a set)

This PR also contains a minor optimization for SRANDMEMBER, HRANDFIELD,
and ZRANDMEMBER, to avoid the temporary dict from being rehashed while it grows.

Co-authored-by: Oran Agra <oran@redislabs.com>
---
 src/t_hash.c |  2 ++
 src/t_set.c  | 24 +++++++++++++++++++-----
 src/t_zset.c |  2 ++
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/src/t_hash.c b/src/t_hash.c
index 15b57c1d7..eb980578c 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -1078,6 +1078,7 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) {
      * used into CASE 4 is highly inefficient. */
     if (count*HRANDFIELD_SUB_STRATEGY_MUL > size) {
         dict *d = dictCreate(&sdsReplyDictType, NULL);
+        dictExpand(d, size);
         hashTypeIterator *hi = hashTypeInitIterator(hash);
 
         /* Add all the elements into the temporary dictionary. */
@@ -1147,6 +1148,7 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) {
         unsigned long added = 0;
         ziplistEntry key, value;
         dict *d = dictCreate(&hashDictType, NULL);
+        dictExpand(d, count);
         while(added < count) {
             hashTypeRandomElement(hash, size, &key, withvalues? &value : NULL);
 
diff --git a/src/t_set.c b/src/t_set.c
index f7a05206d..b655b716d 100644
--- a/src/t_set.c
+++ b/src/t_set.c
@@ -674,13 +674,13 @@ void srandmemberWithCountCommand(client *c) {
         uniq = 0;
     }
 
-    if ((set = lookupKeyReadOrReply(c,c->argv[1],shared.emptyset[c->resp]))
+    if ((set = lookupKeyReadOrReply(c,c->argv[1],shared.emptyarray))
         == NULL || checkType(c,set,OBJ_SET)) return;
     size = setTypeSize(set);
 
     /* If count is zero, serve it ASAP to avoid special cases later. */
     if (count == 0) {
-        addReply(c,shared.emptyset[c->resp]);
+        addReply(c,shared.emptyarray);
         return;
     }
 
@@ -690,7 +690,7 @@ void srandmemberWithCountCommand(client *c) {
      * structures. This case is the only one that also needs to return the
      * elements in random order. */
     if (!uniq || count == 1) {
-        addReplySetLen(c,count);
+        addReplyArrayLen(c,count);
         while(count--) {
             encoding = setTypeRandomElement(set,&ele,&llele);
             if (encoding == OBJ_ENCODING_INTSET) {
@@ -706,7 +706,19 @@ void srandmemberWithCountCommand(client *c) {
      * The number of requested elements is greater than the number of
      * elements inside the set: simply return the whole set. */
     if (count >= size) {
-        sunionDiffGenericCommand(c,c->argv+1,1,NULL,SET_OP_UNION);
+        setTypeIterator *si;
+        addReplyArrayLen(c,size);
+        si = setTypeInitIterator(set);
+        while ((encoding = setTypeNext(si,&ele,&llele)) != -1) {
+            if (encoding == OBJ_ENCODING_INTSET) {
+                addReplyBulkLongLong(c,llele);
+            } else {
+                addReplyBulkCBuffer(c,ele,sdslen(ele));
+            }
+            size--;
+        }
+        setTypeReleaseIterator(si);
+        serverAssert(size==0);
         return;
     }
 
@@ -727,6 +739,7 @@ void srandmemberWithCountCommand(client *c) {
 
         /* Add all the elements into the temporary dictionary. */
         si = setTypeInitIterator(set);
+        dictExpand(d, size);
         while ((encoding = setTypeNext(si,&ele,&llele)) != -1) {
             int retval = DICT_ERR;
 
@@ -759,6 +772,7 @@ void srandmemberWithCountCommand(client *c) {
         unsigned long added = 0;
         sds sdsele;
 
+        dictExpand(d, count);
         while (added < count) {
             encoding = setTypeRandomElement(set,&ele,&llele);
             if (encoding == OBJ_ENCODING_INTSET) {
@@ -781,7 +795,7 @@ void srandmemberWithCountCommand(client *c) {
         dictIterator *di;
         dictEntry *de;
 
-        addReplySetLen(c,count);
+        addReplyArrayLen(c,count);
         di = dictGetIterator(d);
         while((de = dictNext(di)) != NULL)
             addReplyBulkSds(c,dictGetKey(de));
diff --git a/src/t_zset.c b/src/t_zset.c
index 869cf6c9a..6df21e300 100644
--- a/src/t_zset.c
+++ b/src/t_zset.c
@@ -4085,6 +4085,7 @@ void zrandmemberWithCountCommand(client *c, long l, int withscores) {
      * used into CASE 4 is highly inefficient. */
     if (count*ZRANDMEMBER_SUB_STRATEGY_MUL > size) {
         dict *d = dictCreate(&sdsReplyDictType, NULL);
+        dictExpand(d, size);
         /* Add all the elements into the temporary dictionary. */
         while (zuiNext(&src, &zval)) {
             sds key = zuiNewSdsFromValue(&zval);
@@ -4143,6 +4144,7 @@ void zrandmemberWithCountCommand(client *c, long l, int withscores) {
         /* Hashtable encoding (generic implementation) */
         unsigned long added = 0;
         dict *d = dictCreate(&hashDictType, NULL);
+        dictExpand(d, count);
 
         while (added < count) {
             ziplistEntry key;