Unverified Commit 06dd202a authored by Itamar Haber's avatar Itamar Haber Committed by GitHub
Browse files

Fixes LPOP/RPOP wrong replies when count is 0 (#9692)

Introduced in #8179, this fixes the command's replies in the 0 count edge case.
[BREAKING] changes the reply type when count is 0 to an empty array (instead of nil)
Moves LPOP ... 0 fast exit path after type check to reply with WRONGTYPE
parent ccf8a651
......@@ -503,6 +503,7 @@ void listElementsRemoved(client *c, robj *key, int where, robj *o, long count, i
* optional count may be provided as the third argument of the client's
* command. */
void popGenericCommand(client *c, int where) {
int hascount = (c->argc == 3);
long count = 0;
robj *value;
......@@ -510,21 +511,22 @@ void popGenericCommand(client *c, int where) {
addReplyErrorFormat(c,"wrong number of arguments for '%s' command",
c->cmd->name);
return;
} else if (c->argc == 3) {
} else if (hascount) {
/* Parse the optional count argument. */
if (getPositiveLongFromObjectOrReply(c,c->argv[2],&count,NULL) != C_OK)
return;
if (count == 0) {
/* Fast exit path. */
addReplyNullArray(c);
return;
}
}
robj *o = lookupKeyWriteOrReply(c, c->argv[1], shared.null[c->resp]);
if (o == NULL || checkType(c, o, OBJ_LIST))
return;
if (hascount && !count) {
/* Fast exit path. */
addReply(c,shared.emptyarray);
return;
}
if (!count) {
/* Pop a single element. This is POP's original behavior that replies
* with a bulk string. */
......
......@@ -517,10 +517,9 @@ start_server {
assert_equal $largevalue(linkedlist) [r rpop mylist2]
assert_equal c [r lpop mylist2]
}
test {R/LPOP with the optional count argument} {
test {RPOP/LPOP with the optional count argument} {
assert_equal 7 [r lpush listcount aa bb cc dd ee ff gg]
assert_equal {} [r lpop listcount 0]
assert_equal {gg} [r lpop listcount 1]
assert_equal {ff ee} [r lpop listcount 2]
assert_equal {aa bb} [r rpop listcount 2]
......@@ -529,6 +528,16 @@ start_server {
assert_error "*ERR*range*" {r lpop forbarqaz -123}
}
# Make sure we can distinguish between an empty array and a null response
r readraw 1
test {RPOP/LPOP with the count 0 returns an empty array} {
r lpush listcount zero
r lpop listcount 0
} {*0}
r readraw 0
test {Variadic RPUSH/LPUSH} {
r del mylist
assert_equal 4 [r lpush mylist a b c d]
......
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