ZRANDMEMBER WITHSCORES with negative COUNT may return bad score (#9162)

Return a bad score when used with negative count (or count of 1), and non-ziplist encoded zset.
Also add test to validate the return value and cover the issue.
This commit is contained in:
Binbin 2021-06-29 15:14:28 +08:00 committed by GitHub
parent 4fa3e23092
commit 4bc5a8324d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 33 additions and 14 deletions

View File

@ -4031,7 +4031,7 @@ void zrandmemberWithCountCommand(client *c, long l, int withscores) {
addReplyArrayLen(c,2); addReplyArrayLen(c,2);
addReplyBulkCBuffer(c, key, sdslen(key)); addReplyBulkCBuffer(c, key, sdslen(key));
if (withscores) if (withscores)
addReplyDouble(c, dictGetDoubleVal(de)); addReplyDouble(c, *(double*)dictGetVal(de));
} }
} else if (zsetobj->encoding == OBJ_ENCODING_ZIPLIST) { } else if (zsetobj->encoding == OBJ_ENCODING_ZIPLIST) {
ziplistEntry *keys, *vals = NULL; ziplistEntry *keys, *vals = NULL;

View File

@ -1652,6 +1652,20 @@ start_server {tags {"zset"}} {
return $res return $res
} }
# Check whether the zset members belong to the zset
proc check_member {mydict res} {
foreach ele $res {
assert {[dict exists $mydict $ele]}
}
}
# Check whether the zset members and score belong to the zset
proc check_member_and_score {mydict res} {
foreach {key val} $res {
assert_equal $val [dict get $mydict $key]
}
}
foreach {type contents} "ziplist {1 a 2 b 3 c} skiplist {1 a 2 b 3 [randstring 70 90 alpha]}" { foreach {type contents} "ziplist {1 a 2 b 3 c} skiplist {1 a 2 b 3 [randstring 70 90 alpha]}" {
set original_max_value [lindex [r config get zset-max-ziplist-value] 1] set original_max_value [lindex [r config get zset-max-ziplist-value] 1]
r config set zset-max-ziplist-value 10 r config set zset-max-ziplist-value 10
@ -1712,25 +1726,29 @@ start_server {tags {"zset"}} {
# PATH 1: Use negative count. # PATH 1: Use negative count.
# 1) Check that it returns repeated elements with and without values. # 1) Check that it returns repeated elements with and without values.
# 2) Check that all the elements actually belong to the original zset.
set res [r zrandmember myzset -20] set res [r zrandmember myzset -20]
assert_equal [llength $res] 20 assert_equal [llength $res] 20
check_member $mydict $res
set res [r zrandmember myzset -1001] set res [r zrandmember myzset -1001]
assert_equal [llength $res] 1001 assert_equal [llength $res] 1001
check_member $mydict $res
# again with WITHSCORES # again with WITHSCORES
set res [r zrandmember myzset -20 withscores] set res [r zrandmember myzset -20 withscores]
assert_equal [llength $res] 40 assert_equal [llength $res] 40
check_member_and_score $mydict $res
set res [r zrandmember myzset -1001 withscores] set res [r zrandmember myzset -1001 withscores]
assert_equal [llength $res] 2002 assert_equal [llength $res] 2002
check_member_and_score $mydict $res
# Test random uniform distribution # Test random uniform distribution
# df = 9, 40 means 0.00001 probability # df = 9, 40 means 0.00001 probability
set res [r zrandmember myzset -1000] set res [r zrandmember myzset -1000]
assert_lessthan [chi_square_value $res] 40 assert_lessthan [chi_square_value $res] 40
check_member $mydict $res
# 2) Check that all the elements actually belong to the original zset.
foreach {key val} $res {
assert {[dict exists $mydict $key]}
}
# 3) Check that eventually all the elements are returned. # 3) Check that eventually all the elements are returned.
# Use both WITHSCORES and without # Use both WITHSCORES and without
@ -1746,7 +1764,7 @@ start_server {tags {"zset"}} {
} else { } else {
set res [r zrandmember myzset -3] set res [r zrandmember myzset -3]
foreach key $res { foreach key $res {
dict append auxset $key $val dict append auxset $key
} }
} }
if {[lsort [dict keys $mydict]] eq if {[lsort [dict keys $mydict]] eq
@ -1762,11 +1780,13 @@ start_server {tags {"zset"}} {
set res [r zrandmember myzset $size] set res [r zrandmember myzset $size]
assert_equal [llength $res] 10 assert_equal [llength $res] 10
assert_equal [lsort $res] [lsort [dict keys $mydict]] assert_equal [lsort $res] [lsort [dict keys $mydict]]
check_member $mydict $res
# again with WITHSCORES # again with WITHSCORES
set res [r zrandmember myzset $size withscores] set res [r zrandmember myzset $size withscores]
assert_equal [llength $res] 20 assert_equal [llength $res] 20
assert_equal [lsort $res] [lsort $mydict] assert_equal [lsort $res] [lsort $mydict]
check_member_and_score $mydict $res
} }
# PATH 3: Ask almost as elements as there are in the set. # PATH 3: Ask almost as elements as there are in the set.
@ -1778,18 +1798,17 @@ start_server {tags {"zset"}} {
# #
# We can test both the code paths just changing the size but # We can test both the code paths just changing the size but
# using the same code. # using the same code.
foreach size {8 2} { foreach size {1 2 8} {
# 1) Check that all the elements actually belong to the
# original set.
set res [r zrandmember myzset $size] set res [r zrandmember myzset $size]
assert_equal [llength $res] $size assert_equal [llength $res] $size
check_member $mydict $res
# again with WITHSCORES # again with WITHSCORES
set res [r zrandmember myzset $size withscores] set res [r zrandmember myzset $size withscores]
assert_equal [llength $res] [expr {$size * 2}] assert_equal [llength $res] [expr {$size * 2}]
check_member_and_score $mydict $res
# 1) Check that all the elements actually belong to the
# original set.
foreach ele [dict keys $res] {
assert {[dict exists $mydict $ele]}
}
# 2) Check that eventually all the elements are returned. # 2) Check that eventually all the elements are returned.
# Use both WITHSCORES and without # Use both WITHSCORES and without