Commit fad44611 authored by Jason Elbaum's avatar Jason Elbaum Committed by Oran Agra
Browse files

Change return value type for ZPOPMAX/MIN in RESP3 (#8981)

When using RESP3, ZPOPMAX/ZPOPMIN should return nested arrays for consistency
with other commands (e.g. ZRANGE).

We do that only when COUNT argument is present (similarly to how LPOP behaves).
for reasoning see https://github.com/redis/redis/issues/8824#issuecomment-855427955

This is a breaking change only when RESP3 is used, and COUNT argument is present!

(cherry picked from commit 7f342020)
parent 88849712
...@@ -3825,11 +3825,16 @@ void genericZpopCommand(client *c, robj **keyv, int keyc, int where, int emitkey ...@@ -3825,11 +3825,16 @@ void genericZpopCommand(client *c, robj **keyv, int keyc, int where, int emitkey
} }
void *arraylen_ptr = addReplyDeferredLen(c); void *arraylen_ptr = addReplyDeferredLen(c);
long arraylen = 0; long result_count = 0;
/* We emit the key only for the blocking variant. */ /* We emit the key only for the blocking variant. */
if (emitkey) addReplyBulk(c,key); if (emitkey) addReplyBulk(c,key);
/* Respond with a single (flat) array in RESP2 or if countarg is not
* provided (returning a single element). In RESP3, when countarg is
* provided, use nested array. */
int use_nested_array = c->resp > 2 && countarg != NULL;
/* Remove the element. */ /* Remove the element. */
do { do {
if (zobj->encoding == OBJ_ENCODING_ZIPLIST) { if (zobj->encoding == OBJ_ENCODING_ZIPLIST) {
...@@ -3872,16 +3877,19 @@ void genericZpopCommand(client *c, robj **keyv, int keyc, int where, int emitkey ...@@ -3872,16 +3877,19 @@ void genericZpopCommand(client *c, robj **keyv, int keyc, int where, int emitkey
serverAssertWithInfo(c,zobj,zsetDel(zobj,ele)); serverAssertWithInfo(c,zobj,zsetDel(zobj,ele));
server.dirty++; server.dirty++;
if (arraylen == 0) { /* Do this only for the first iteration. */ if (result_count == 0) { /* Do this only for the first iteration. */
char *events[2] = {"zpopmin","zpopmax"}; char *events[2] = {"zpopmin","zpopmax"};
notifyKeyspaceEvent(NOTIFY_ZSET,events[where],key,c->db->id); notifyKeyspaceEvent(NOTIFY_ZSET,events[where],key,c->db->id);
signalModifiedKey(c,c->db,key); signalModifiedKey(c,c->db,key);
} }
if (use_nested_array) {
addReplyArrayLen(c,2);
}
addReplyBulkCBuffer(c,ele,sdslen(ele)); addReplyBulkCBuffer(c,ele,sdslen(ele));
addReplyDouble(c,score); addReplyDouble(c,score);
sdsfree(ele); sdsfree(ele);
arraylen += 2; ++result_count;
/* Remove the key, if indeed needed. */ /* Remove the key, if indeed needed. */
if (zsetLength(zobj) == 0) { if (zsetLength(zobj) == 0) {
...@@ -3891,7 +3899,10 @@ void genericZpopCommand(client *c, robj **keyv, int keyc, int where, int emitkey ...@@ -3891,7 +3899,10 @@ void genericZpopCommand(client *c, robj **keyv, int keyc, int where, int emitkey
} }
} while(--count); } while(--count);
setDeferredArrayLen(c,arraylen_ptr,arraylen + (emitkey != 0)); if (!use_nested_array) {
result_count *= 2;
}
setDeferredArrayLen(c,arraylen_ptr,result_count + (emitkey != 0));
} }
/* ZPOPMIN key [<count>] */ /* ZPOPMIN key [<count>] */
......
...@@ -960,6 +960,39 @@ start_server {tags {"zset"}} { ...@@ -960,6 +960,39 @@ start_server {tags {"zset"}} {
assert_equal 1 [r zcard z2] assert_equal 1 [r zcard z2]
} }
test "Basic ZPOP - $encoding RESP3" {
r hello 3
r del z1
create_zset z1 {0 a 1 b 2 c 3 d}
assert_equal {a 0.0} [r zpopmin z1]
assert_equal {d 3.0} [r zpopmax z1]
r hello 2
}
test "ZPOP with count - $encoding RESP3" {
r hello 3
r del z1
create_zset z1 {0 a 1 b 2 c 3 d}
assert_equal {{a 0.0} {b 1.0}} [r zpopmin z1 2]
assert_equal {{d 3.0} {c 2.0}} [r zpopmax z1 2]
r hello 2
}
test "BZPOP - $encoding RESP3" {
r hello 3
set rd [redis_deferring_client]
create_zset zset {0 a 1 b 2 c}
$rd bzpopmin zset 5
assert_equal {zset a 0} [$rd read]
$rd bzpopmin zset 5
assert_equal {zset b 1} [$rd read]
$rd bzpopmax zset 5
assert_equal {zset c 2} [$rd read]
assert_equal 0 [r exists zset]
r hello 2
}
r config set zset-max-ziplist-entries $original_max_entries r config set zset-max-ziplist-entries $original_max_entries
r config set zset-max-ziplist-value $original_max_value r config set zset-max-ziplist-value $original_max_value
} }
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment