From 39feee8e3a6fee885aee60672a9dde0dfd08896c Mon Sep 17 00:00:00 2001 From: Binbin Date: Tue, 11 Jan 2022 20:26:13 +0800 Subject: [PATCH] LPOP/RPOP with count against non existing list return null array (#10095) It used to return `$-1` in RESP2, now we will return `*-1`. This is a bug in redis 6.2 when COUNT was added, the `COUNT` option was introduced in #8179. Fix #10089. the documentation of [LPOP](https://redis.io/commands/lpop) says ``` When called without the count argument: Bulk string reply: the value of the first element, or nil when key does not exist. When called with the count argument: Array reply: list of popped elements, or nil when key does not exist. ``` --- src/t_list.c | 2 +- tests/unit/type/list.tcl | 44 +++++++++++++++++++++++++++++++++------- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/t_list.c b/src/t_list.c index 460437853..4d74a1665 100644 --- a/src/t_list.c +++ b/src/t_list.c @@ -501,7 +501,7 @@ void popGenericCommand(client *c, int where) { return; } - robj *o = lookupKeyWriteOrReply(c, c->argv[1], shared.null[c->resp]); + robj *o = lookupKeyWriteOrReply(c, c->argv[1], hascount ? shared.nullarray[c->resp]: shared.null[c->resp]); if (o == NULL || checkType(c, o, OBJ_LIST)) return; diff --git a/tests/unit/type/list.tcl b/tests/unit/type/list.tcl index 31c8c56bb..3981d3f87 100644 --- a/tests/unit/type/list.tcl +++ b/tests/unit/type/list.tcl @@ -496,15 +496,45 @@ 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 + proc verify_resp_response {resp response resp2_response resp3_response} { + if {$resp == 2} { + assert_equal $response $resp2_response + } elseif {$resp == 3} { + assert_equal $response $resp3_response + } + } - test {RPOP/LPOP with the count 0 returns an empty array} { - r lpush listcount zero - r lpop listcount 0 - } {*0} + foreach resp {3 2} { + r hello $resp - r readraw 0 + # Make sure we can distinguish between an empty array and a null response + r readraw 1 + + test "LPOP/RPOP with the count 0 returns an empty array in RESP$resp" { + r lpush listcount zero + assert_equal {*0} [r lpop listcount 0] + assert_equal {*0} [r rpop listcount 0] + } + + test "LPOP/RPOP against non existing key in RESP$resp" { + r del non_existing_key + + verify_resp_response $resp [r lpop non_existing_key] {$-1} {_} + verify_resp_response $resp [r rpop non_existing_key] {$-1} {_} + } + + test "LPOP/RPOP with against non existing key in RESP$resp" { + r del non_existing_key + + verify_resp_response $resp [r lpop non_existing_key 0] {*-1} {_} + verify_resp_response $resp [r lpop non_existing_key 1] {*-1} {_} + + verify_resp_response $resp [r rpop non_existing_key 0] {*-1} {_} + verify_resp_response $resp [r rpop non_existing_key 1] {*-1} {_} + } + + r readraw 0 + } test {Variadic RPUSH/LPUSH} { r del mylist