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>
This commit is contained in:
parent
0fd1fb59f2
commit
75ca6c2ef6
@ -1078,6 +1078,7 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) {
|
|||||||
* used into CASE 4 is highly inefficient. */
|
* used into CASE 4 is highly inefficient. */
|
||||||
if (count*HRANDFIELD_SUB_STRATEGY_MUL > size) {
|
if (count*HRANDFIELD_SUB_STRATEGY_MUL > size) {
|
||||||
dict *d = dictCreate(&sdsReplyDictType, NULL);
|
dict *d = dictCreate(&sdsReplyDictType, NULL);
|
||||||
|
dictExpand(d, size);
|
||||||
hashTypeIterator *hi = hashTypeInitIterator(hash);
|
hashTypeIterator *hi = hashTypeInitIterator(hash);
|
||||||
|
|
||||||
/* Add all the elements into the temporary dictionary. */
|
/* Add all the elements into the temporary dictionary. */
|
||||||
@ -1147,6 +1148,7 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) {
|
|||||||
unsigned long added = 0;
|
unsigned long added = 0;
|
||||||
ziplistEntry key, value;
|
ziplistEntry key, value;
|
||||||
dict *d = dictCreate(&hashDictType, NULL);
|
dict *d = dictCreate(&hashDictType, NULL);
|
||||||
|
dictExpand(d, count);
|
||||||
while(added < count) {
|
while(added < count) {
|
||||||
hashTypeRandomElement(hash, size, &key, withvalues? &value : NULL);
|
hashTypeRandomElement(hash, size, &key, withvalues? &value : NULL);
|
||||||
|
|
||||||
|
24
src/t_set.c
24
src/t_set.c
@ -674,13 +674,13 @@ void srandmemberWithCountCommand(client *c) {
|
|||||||
uniq = 0;
|
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;
|
== NULL || checkType(c,set,OBJ_SET)) return;
|
||||||
size = setTypeSize(set);
|
size = setTypeSize(set);
|
||||||
|
|
||||||
/* If count is zero, serve it ASAP to avoid special cases later. */
|
/* If count is zero, serve it ASAP to avoid special cases later. */
|
||||||
if (count == 0) {
|
if (count == 0) {
|
||||||
addReply(c,shared.emptyset[c->resp]);
|
addReply(c,shared.emptyarray);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -690,7 +690,7 @@ void srandmemberWithCountCommand(client *c) {
|
|||||||
* structures. This case is the only one that also needs to return the
|
* structures. This case is the only one that also needs to return the
|
||||||
* elements in random order. */
|
* elements in random order. */
|
||||||
if (!uniq || count == 1) {
|
if (!uniq || count == 1) {
|
||||||
addReplySetLen(c,count);
|
addReplyArrayLen(c,count);
|
||||||
while(count--) {
|
while(count--) {
|
||||||
encoding = setTypeRandomElement(set,&ele,&llele);
|
encoding = setTypeRandomElement(set,&ele,&llele);
|
||||||
if (encoding == OBJ_ENCODING_INTSET) {
|
if (encoding == OBJ_ENCODING_INTSET) {
|
||||||
@ -706,7 +706,19 @@ void srandmemberWithCountCommand(client *c) {
|
|||||||
* The number of requested elements is greater than the number of
|
* The number of requested elements is greater than the number of
|
||||||
* elements inside the set: simply return the whole set. */
|
* elements inside the set: simply return the whole set. */
|
||||||
if (count >= size) {
|
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;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -727,6 +739,7 @@ void srandmemberWithCountCommand(client *c) {
|
|||||||
|
|
||||||
/* Add all the elements into the temporary dictionary. */
|
/* Add all the elements into the temporary dictionary. */
|
||||||
si = setTypeInitIterator(set);
|
si = setTypeInitIterator(set);
|
||||||
|
dictExpand(d, size);
|
||||||
while ((encoding = setTypeNext(si,&ele,&llele)) != -1) {
|
while ((encoding = setTypeNext(si,&ele,&llele)) != -1) {
|
||||||
int retval = DICT_ERR;
|
int retval = DICT_ERR;
|
||||||
|
|
||||||
@ -759,6 +772,7 @@ void srandmemberWithCountCommand(client *c) {
|
|||||||
unsigned long added = 0;
|
unsigned long added = 0;
|
||||||
sds sdsele;
|
sds sdsele;
|
||||||
|
|
||||||
|
dictExpand(d, count);
|
||||||
while (added < count) {
|
while (added < count) {
|
||||||
encoding = setTypeRandomElement(set,&ele,&llele);
|
encoding = setTypeRandomElement(set,&ele,&llele);
|
||||||
if (encoding == OBJ_ENCODING_INTSET) {
|
if (encoding == OBJ_ENCODING_INTSET) {
|
||||||
@ -781,7 +795,7 @@ void srandmemberWithCountCommand(client *c) {
|
|||||||
dictIterator *di;
|
dictIterator *di;
|
||||||
dictEntry *de;
|
dictEntry *de;
|
||||||
|
|
||||||
addReplySetLen(c,count);
|
addReplyArrayLen(c,count);
|
||||||
di = dictGetIterator(d);
|
di = dictGetIterator(d);
|
||||||
while((de = dictNext(di)) != NULL)
|
while((de = dictNext(di)) != NULL)
|
||||||
addReplyBulkSds(c,dictGetKey(de));
|
addReplyBulkSds(c,dictGetKey(de));
|
||||||
|
@ -4085,6 +4085,7 @@ void zrandmemberWithCountCommand(client *c, long l, int withscores) {
|
|||||||
* used into CASE 4 is highly inefficient. */
|
* used into CASE 4 is highly inefficient. */
|
||||||
if (count*ZRANDMEMBER_SUB_STRATEGY_MUL > size) {
|
if (count*ZRANDMEMBER_SUB_STRATEGY_MUL > size) {
|
||||||
dict *d = dictCreate(&sdsReplyDictType, NULL);
|
dict *d = dictCreate(&sdsReplyDictType, NULL);
|
||||||
|
dictExpand(d, size);
|
||||||
/* Add all the elements into the temporary dictionary. */
|
/* Add all the elements into the temporary dictionary. */
|
||||||
while (zuiNext(&src, &zval)) {
|
while (zuiNext(&src, &zval)) {
|
||||||
sds key = zuiNewSdsFromValue(&zval);
|
sds key = zuiNewSdsFromValue(&zval);
|
||||||
@ -4143,6 +4144,7 @@ void zrandmemberWithCountCommand(client *c, long l, int withscores) {
|
|||||||
/* Hashtable encoding (generic implementation) */
|
/* Hashtable encoding (generic implementation) */
|
||||||
unsigned long added = 0;
|
unsigned long added = 0;
|
||||||
dict *d = dictCreate(&hashDictType, NULL);
|
dict *d = dictCreate(&hashDictType, NULL);
|
||||||
|
dictExpand(d, count);
|
||||||
|
|
||||||
while (added < count) {
|
while (added < count) {
|
||||||
ziplistEntry key;
|
ziplistEntry key;
|
||||||
|
Loading…
x
Reference in New Issue
Block a user