Describe the bug
The LPOP documentation says that when a count is given the result is "Array reply: list of popped elements, or nil when key does not exist." However, this fast path writes a nil reply when count is 0, rather than an empty list.
It's also exiting before even checking the type of the key, so instead of getting a WRONGTYPE error, it can "successfully" do an LPOP on non-lists. This differs from SPOP key 0 which return a WRONGTYPE error.
To reproduce
With redis-cli (note, I'm using server version 6.2.6, but only have redis-cli 5.0.7 handy, so this is presumably all RESP2):
127.0.0.1:6379> rpush foo a
(integer) 1
127.0.0.1:6379> lpop foo 0
(nil)
Expected behavior
Expected to get (empty list or set).
Additional information
It seems a bit odd that SPOP returns an empty array if the key does not exist while LPOP/RPOP returns nil.
Comment From: enjoy-binbin
i don't know the nil history, we may not be able to change it very well for compatibility.
but the WRONGTYPE error indeed is really weird...
127.0.0.1:6379> set foo a
OK
127.0.0.1:6379> lpop foo 0
(nil)
127.0.0.1:6379> zpopmax foo 0
(empty array)
127.0.0.1:6379> spop foo 0
(error) WRONGTYPE Operation against a key holding the wrong kind of value
Comment From: itamarhaber
Hello @bmerry
Thanks for noticing and reporting the issue. These issues were introduced with #8179 and prove (once again) that software development should be left to responsible professionals.
Comment From: zuiderkwast
zpopmax foo 0 should also give a WRONGTYPE error in @enjoy-binbin's example above. That's another bug?
Comment From: madolson
I think it's more of an inconsistency than a bug, but I would still be in favor of updating zpopmax so that it returns WRONGTYPE in the case you outlined. I did scan through the code and didn't find any other inconsistencies.
Comment From: itamarhaber
What @madolson said because https://i.kym-cdn.com/photos/images/original/000/909/991/48c.jpg and also iirc @guybe7 said there's another just like that that was recently fixed/PRed.
Comment From: enjoy-binbin
zpopmax foo 0 should also give a WRONGTYPE error
@zuiderkwast I think it is another bug or inconsistency. I will take a look since we will do the same in LPOP in haber's PR
About the zpop, I might modify it like this ``` before: 127.0.0.1:6379> set zset str OK 127.0.0.1:6379> zpopmin zset (error) WRONGTYPE Operation against a key holding the wrong kind of value 127.0.0.1:6379> zpopmin zset 1 (error) WRONGTYPE Operation against a key holding the wrong kind of value 127.0.0.1:6379> zpopmin zset 0 (empty array) 127.0.0.1:6379> zpopmin zset -1 (empty array)
after: 27.0.0.1:6379> set zset str OK 127.0.0.1:6379> zpopmin zset (error) WRONGTYPE Operation against a key holding the wrong kind of value 127.0.0.1:6379> zpopmin zset 1 (error) WRONGTYPE Operation against a key holding the wrong kind of value 127.0.0.1:6379> zpopmin zset 0 (error) WRONGTYPE Operation against a key holding the wrong kind of value 127.0.0.1:6379> zpopmin zset -1 (error) ERR value is out of range, must be positive
Comment From: guybe7
@itamarhaber found it: https://github.com/redis/redis/pull/9271