diff options
author | Wen Hui <wen.hui.ware@gmail.com> | 2021-02-22 08:00:59 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-02-22 15:00:59 +0200 |
commit | f5235b2d7660b163c739325ec9284f97d5b963be (patch) | |
tree | eb9d3c74e84388b18f44a7b3d3ebdd28491bd34a /src/t_set.c | |
parent | 362f2a84bd67a1b65d810b064163e8432c79138e (diff) | |
download | redis-f5235b2d7660b163c739325ec9284f97d5b963be.tar.gz |
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>
Diffstat (limited to 'src/t_set.c')
-rw-r--r-- | src/t_set.c | 24 |
1 files changed, 19 insertions, 5 deletions
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)); |