zsetAdd: Fix wrong reply in case of INCR and GT/LT (#8717)
If GT/LT fails the operation we need to reply with nill (like failure due to NX). Other changes: Add the missing $encoding suffix to many zset tests Note: there's a behavior change just in case of INCR + GT/LT that fails. The old code was replying with the wrong (rejected) score, and now it'll reply with nil. Note that that's anyway a corner case so this "behavior change" shouldn't have too much affect. Using GT/LT with INCR has a predictable result even before we run the command (INCR GT will only only / always fail if the increment is negative).
This commit is contained in:
guybe7
GitHub
parent
d5935bb0a4
commit
843f769b96
33
src/t_zset.c
33
src/t_zset.c
@ -1352,16 +1352,18 @@ int zsetAdd(robj *zobj, double score, sds ele, int in_flags, int *out_flags, dou
|
||||
*out_flags |= ZADD_OUT_NAN;
|
||||
return 0;
|
||||
}
|
||||
if (newscore) *newscore = score;
|
||||
}
|
||||
|
||||
/* GT/LT? Only update if score is greater/less than current. */
|
||||
if ((lt && score >= curscore) || (gt && score <= curscore)) {
|
||||
*out_flags |= ZADD_OUT_NOP;
|
||||
return 1;
|
||||
}
|
||||
|
||||
if (newscore) *newscore = score;
|
||||
|
||||
/* Remove and re-insert when score changed. */
|
||||
if (score != curscore &&
|
||||
/* LT? Only update if score is less than current. */
|
||||
(!lt || score < curscore) &&
|
||||
/* GT? Only update if score is greater than current. */
|
||||
(!gt || score > curscore))
|
||||
{
|
||||
if (score != curscore) {
|
||||
zobj->ptr = zzlDelete(zobj->ptr,eptr);
|
||||
zobj->ptr = zzlInsert(zobj->ptr,ele,score);
|
||||
*out_flags |= ZADD_OUT_UPDATED;
|
||||
@ -1393,6 +1395,7 @@ int zsetAdd(robj *zobj, double score, sds ele, int in_flags, int *out_flags, dou
|
||||
*out_flags |= ZADD_OUT_NOP;
|
||||
return 1;
|
||||
}
|
||||
|
||||
curscore = *(double*)dictGetVal(de);
|
||||
|
||||
/* Prepare the score for the increment if needed. */
|
||||
@ -1402,16 +1405,18 @@ int zsetAdd(robj *zobj, double score, sds ele, int in_flags, int *out_flags, dou
|
||||
*out_flags |= ZADD_OUT_NAN;
|
||||
return 0;
|
||||
}
|
||||
if (newscore) *newscore = score;
|
||||
}
|
||||
|
||||
/* GT/LT? Only update if score is greater/less than current. */
|
||||
if ((lt && score >= curscore) || (gt && score <= curscore)) {
|
||||
*out_flags |= ZADD_OUT_NOP;
|
||||
return 1;
|
||||
}
|
||||
|
||||
if (newscore) *newscore = score;
|
||||
|
||||
/* Remove and re-insert when score changes. */
|
||||
if (score != curscore &&
|
||||
/* LT? Only update if score is less than current. */
|
||||
(!lt || score < curscore) &&
|
||||
/* GT? Only update if score is greater than current. */
|
||||
(!gt || score > curscore))
|
||||
{
|
||||
if (score != curscore) {
|
||||
znode = zslUpdateScore(zs->zsl,curscore,ele,score);
|
||||
/* Note that we did not removed the original element from
|
||||
* the hash table representing the sorted set, so we just
|
||||
|
||||
@ -41,11 +41,11 @@ start_server {tags {"zset"}} {
|
||||
assert_error "*not*float*" {r zadd myzset nan abc}
|
||||
}
|
||||
|
||||
test "ZSET element can't be set to NaN with ZINCRBY" {
|
||||
test "ZSET element can't be set to NaN with ZINCRBY - $encoding" {
|
||||
assert_error "*not*float*" {r zadd myzset nan abc}
|
||||
}
|
||||
|
||||
test "ZADD with options syntax error with incomplete pair" {
|
||||
test "ZADD with options syntax error with incomplete pair - $encoding" {
|
||||
r del ztmp
|
||||
catch {r zadd ztmp xx 10 x 20} err
|
||||
set err
|
||||
@ -64,14 +64,14 @@ start_server {tags {"zset"}} {
|
||||
assert {[r zcard ztmp] == 1}
|
||||
}
|
||||
|
||||
test "ZADD XX returns the number of elements actually added" {
|
||||
test "ZADD XX returns the number of elements actually added - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp 10 x
|
||||
set retval [r zadd ztmp 10 x 20 y 30 z]
|
||||
assert {$retval == 2}
|
||||
}
|
||||
|
||||
test "ZADD XX updates existing elements score" {
|
||||
test "ZADD XX updates existing elements score - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp 10 x 20 y 30 z
|
||||
r zadd ztmp xx 5 foo 11 x 21 y 40 zap
|
||||
@ -80,7 +80,7 @@ start_server {tags {"zset"}} {
|
||||
assert {[r zscore ztmp y] == 21}
|
||||
}
|
||||
|
||||
test "ZADD GT updates existing elements when new scores are greater" {
|
||||
test "ZADD GT updates existing elements when new scores are greater - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp 10 x 20 y 30 z
|
||||
assert {[r zadd ztmp gt ch 5 foo 11 x 21 y 29 z] == 3}
|
||||
@ -90,7 +90,7 @@ start_server {tags {"zset"}} {
|
||||
assert {[r zscore ztmp z] == 30}
|
||||
}
|
||||
|
||||
test "ZADD LT updates existing elements when new scores are lower" {
|
||||
test "ZADD LT updates existing elements when new scores are lower - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp 10 x 20 y 30 z
|
||||
assert {[r zadd ztmp lt ch 5 foo 11 x 21 y 29 z] == 2}
|
||||
@ -100,7 +100,7 @@ start_server {tags {"zset"}} {
|
||||
assert {[r zscore ztmp z] == 29}
|
||||
}
|
||||
|
||||
test "ZADD GT XX updates existing elements when new scores are greater and skips new elements" {
|
||||
test "ZADD GT XX updates existing elements when new scores are greater and skips new elements - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp 10 x 20 y 30 z
|
||||
assert {[r zadd ztmp gt xx ch 5 foo 11 x 21 y 29 z] == 2}
|
||||
@ -110,7 +110,7 @@ start_server {tags {"zset"}} {
|
||||
assert {[r zscore ztmp z] == 30}
|
||||
}
|
||||
|
||||
test "ZADD LT XX updates existing elements when new scores are lower and skips new elements" {
|
||||
test "ZADD LT XX updates existing elements when new scores are lower and skips new elements - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp 10 x 20 y 30 z
|
||||
assert {[r zadd ztmp lt xx ch 5 foo 11 x 21 y 29 z] == 1}
|
||||
@ -120,19 +120,19 @@ start_server {tags {"zset"}} {
|
||||
assert {[r zscore ztmp z] == 29}
|
||||
}
|
||||
|
||||
test "ZADD XX and NX are not compatible" {
|
||||
test "ZADD XX and NX are not compatible - $encoding" {
|
||||
r del ztmp
|
||||
catch {r zadd ztmp xx nx 10 x} err
|
||||
set err
|
||||
} {ERR*}
|
||||
|
||||
test "ZADD NX with non existing key" {
|
||||
test "ZADD NX with non existing key - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp nx 10 x 20 y 30 z
|
||||
assert {[r zcard ztmp] == 3}
|
||||
}
|
||||
|
||||
test "ZADD NX only add new elements without updating old ones" {
|
||||
test "ZADD NX only add new elements without updating old ones - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp 10 x 20 y 30 z
|
||||
assert {[r zadd ztmp nx 11 x 21 y 100 a 200 b] == 2}
|
||||
@ -142,73 +142,105 @@ start_server {tags {"zset"}} {
|
||||
assert {[r zscore ztmp b] == 200}
|
||||
}
|
||||
|
||||
test "ZADD GT and NX are not compatible" {
|
||||
test "ZADD GT and NX are not compatible - $encoding" {
|
||||
r del ztmp
|
||||
catch {r zadd ztmp gt nx 10 x} err
|
||||
set err
|
||||
} {ERR*}
|
||||
|
||||
test "ZADD LT and NX are not compatible" {
|
||||
test "ZADD LT and NX are not compatible - $encoding" {
|
||||
r del ztmp
|
||||
catch {r zadd ztmp lt nx 10 x} err
|
||||
set err
|
||||
} {ERR*}
|
||||
|
||||
test "ZADD LT and GT are not compatible" {
|
||||
test "ZADD LT and GT are not compatible - $encoding" {
|
||||
r del ztmp
|
||||
catch {r zadd ztmp lt gt 10 x} err
|
||||
set err
|
||||
} {ERR*}
|
||||
|
||||
test "ZADD INCR works like ZINCRBY" {
|
||||
test "ZADD INCR LT/GT replies with nill if score not updated - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp 28 x
|
||||
assert {[r zadd ztmp lt incr 1 x] eq {}}
|
||||
assert {[r zscore ztmp x] == 28}
|
||||
assert {[r zadd ztmp gt incr -1 x] eq {}}
|
||||
assert {[r zscore ztmp x] == 28}
|
||||
}
|
||||
|
||||
test "ZADD INCR LT/GT with inf - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp +inf x -inf y
|
||||
|
||||
assert {[r zadd ztmp lt incr 1 x] eq {}}
|
||||
assert {[r zscore ztmp x] == inf}
|
||||
assert {[r zadd ztmp gt incr -1 x] eq {}}
|
||||
assert {[r zscore ztmp x] == inf}
|
||||
assert {[r zadd ztmp lt incr -1 x] eq {}}
|
||||
assert {[r zscore ztmp x] == inf}
|
||||
assert {[r zadd ztmp gt incr 1 x] eq {}}
|
||||
assert {[r zscore ztmp x] == inf}
|
||||
|
||||
assert {[r zadd ztmp lt incr 1 y] eq {}}
|
||||
assert {[r zscore ztmp y] == -inf}
|
||||
assert {[r zadd ztmp gt incr -1 y] eq {}}
|
||||
assert {[r zscore ztmp y] == -inf}
|
||||
assert {[r zadd ztmp lt incr -1 y] eq {}}
|
||||
assert {[r zscore ztmp y] == -inf}
|
||||
assert {[r zadd ztmp gt incr 1 y] eq {}}
|
||||
assert {[r zscore ztmp y] == -inf}
|
||||
}
|
||||
|
||||
test "ZADD INCR works like ZINCRBY - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp 10 x 20 y 30 z
|
||||
r zadd ztmp INCR 15 x
|
||||
assert {[r zscore ztmp x] == 25}
|
||||
}
|
||||
|
||||
test "ZADD INCR works with a single score-elemenet pair" {
|
||||
test "ZADD INCR works with a single score-elemenet pair - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp 10 x 20 y 30 z
|
||||
catch {r zadd ztmp INCR 15 x 10 y} err
|
||||
set err
|
||||
} {ERR*}
|
||||
|
||||
test "ZADD CH option changes return value to all changed elements" {
|
||||
test "ZADD CH option changes return value to all changed elements - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp 10 x 20 y 30 z
|
||||
assert {[r zadd ztmp 11 x 21 y 30 z] == 0}
|
||||
assert {[r zadd ztmp ch 12 x 22 y 30 z] == 2}
|
||||
}
|
||||
|
||||
test "ZINCRBY calls leading to NaN result in error" {
|
||||
test "ZINCRBY calls leading to NaN result in error - $encoding" {
|
||||
r zincrby myzset +inf abc
|
||||
assert_error "*NaN*" {r zincrby myzset -inf abc}
|
||||
}
|
||||
|
||||
test {ZADD - Variadic version base case} {
|
||||
test {ZADD - Variadic version base case - $encoding} {
|
||||
r del myzset
|
||||
list [r zadd myzset 10 a 20 b 30 c] [r zrange myzset 0 -1 withscores]
|
||||
} {3 {a 10 b 20 c 30}}
|
||||
|
||||
test {ZADD - Return value is the number of actually added items} {
|
||||
test {ZADD - Return value is the number of actually added items - $encoding} {
|
||||
list [r zadd myzset 5 x 20 b 30 c] [r zrange myzset 0 -1 withscores]
|
||||
} {1 {x 5 a 10 b 20 c 30}}
|
||||
|
||||
test {ZADD - Variadic version does not add nothing on single parsing err} {
|
||||
test {ZADD - Variadic version does not add nothing on single parsing err - $encoding} {
|
||||
r del myzset
|
||||
catch {r zadd myzset 10 a 20 b 30.badscore c} e
|
||||
assert_match {*ERR*not*float*} $e
|
||||
r exists myzset
|
||||
} {0}
|
||||
|
||||
test {ZADD - Variadic version will raise error on missing arg} {
|
||||
test {ZADD - Variadic version will raise error on missing arg - $encoding} {
|
||||
r del myzset
|
||||
catch {r zadd myzset 10 a 20 b 30 c 40} e
|
||||
assert_match {*ERR*syntax*} $e
|
||||
}
|
||||
|
||||
test {ZINCRBY does not work variadic even if shares ZADD implementation} {
|
||||
test {ZINCRBY does not work variadic even if shares ZADD implementation - $encoding} {
|
||||
r del myzset
|
||||
catch {r zincrby myzset 10 a 20 b 30 c} e
|
||||
assert_match {*ERR*wrong*number*arg*} $e
|
||||
@ -221,7 +253,7 @@ start_server {tags {"zset"}} {
|
||||
assert_equal 0 [r zcard zdoesntexist]
|
||||
}
|
||||
|
||||
test "ZREM removes key after last element is removed" {
|
||||
test "ZREM removes key after last element is removed - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp 10 x
|
||||
r zadd ztmp 20 y
|
||||
@ -233,7 +265,7 @@ start_server {tags {"zset"}} {
|
||||
assert_equal 0 [r exists ztmp]
|
||||
}
|
||||
|
||||
test "ZREM variadic version" {
|
||||
test "ZREM variadic version - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp 10 a 20 b 30 c
|
||||
assert_equal 2 [r zrem ztmp x y a b k]
|
||||
@ -242,7 +274,7 @@ start_server {tags {"zset"}} {
|
||||
r exists ztmp
|
||||
} {0}
|
||||
|
||||
test "ZREM variadic version -- remove elements after key deletion" {
|
||||
test "ZREM variadic version -- remove elements after key deletion - $encoding" {
|
||||
r del ztmp
|
||||
r zadd ztmp 10 a 20 b 30 c
|
||||
r zrem ztmp a b c d e f g
|
||||
@ -350,7 +382,7 @@ start_server {tags {"zset"}} {
|
||||
assert_equal 6 [r zscore zset bar]
|
||||
}
|
||||
|
||||
test "ZINCRBY return value" {
|
||||
test "ZINCRBY return value - $encoding" {
|
||||
r del ztmp
|
||||
set retval [r zincrby ztmp 1.0 x]
|
||||
assert {$retval == 1.0}
|
||||
@ -360,7 +392,7 @@ start_server {tags {"zset"}} {
|
||||
create_zset zset {-inf a 1 b 2 c 3 d 4 e 5 f +inf g}
|
||||
}
|
||||
|
||||
test "ZRANGEBYSCORE/ZREVRANGEBYSCORE/ZCOUNT basics" {
|
||||
test "ZRANGEBYSCORE/ZREVRANGEBYSCORE/ZCOUNT basics - $encoding" {
|
||||
create_default_zset
|
||||
|
||||
# inclusive range
|
||||
@ -412,13 +444,13 @@ start_server {tags {"zset"}} {
|
||||
assert_equal {} [r zrangebyscore zset (2.4 (2.6]
|
||||
}
|
||||
|
||||
test "ZRANGEBYSCORE with WITHSCORES" {
|
||||
test "ZRANGEBYSCORE with WITHSCORES - $encoding" {
|
||||
create_default_zset
|
||||
assert_equal {b 1 c 2 d 3} [r zrangebyscore zset 0 3 withscores]
|
||||
assert_equal {d 3 c 2 b 1} [r zrevrangebyscore zset 3 0 withscores]
|
||||
}
|
||||
|
||||
test "ZRANGEBYSCORE with LIMIT" {
|
||||
test "ZRANGEBYSCORE with LIMIT - $encoding" {
|
||||
create_default_zset
|
||||
assert_equal {b c} [r zrangebyscore zset 0 10 LIMIT 0 2]
|
||||
assert_equal {d e f} [r zrangebyscore zset 0 10 LIMIT 2 3]
|
||||
@ -430,14 +462,14 @@ start_server {tags {"zset"}} {
|
||||
assert_equal {} [r zrevrangebyscore zset 10 0 LIMIT 20 10]
|
||||
}
|
||||
|
||||
test "ZRANGEBYSCORE with LIMIT and WITHSCORES" {
|
||||
test "ZRANGEBYSCORE with LIMIT and WITHSCORES - $encoding" {
|
||||
create_default_zset
|
||||
assert_equal {e 4 f 5} [r zrangebyscore zset 2 5 LIMIT 2 3 WITHSCORES]
|
||||
assert_equal {d 3 c 2} [r zrevrangebyscore zset 5 2 LIMIT 2 3 WITHSCORES]
|
||||
assert_equal {} [r zrangebyscore zset 2 5 LIMIT 12 13 WITHSCORES]
|
||||
}
|
||||
|
||||
test "ZRANGEBYSCORE with non-value min or max" {
|
||||
test "ZRANGEBYSCORE with non-value min or max - $encoding" {
|
||||
assert_error "*not*float*" {r zrangebyscore fooz str 1}
|
||||
assert_error "*not*float*" {r zrangebyscore fooz 1 str}
|
||||
assert_error "*not*float*" {r zrangebyscore fooz 1 NaN}
|
||||
@ -449,7 +481,7 @@ start_server {tags {"zset"}} {
|
||||
0 omega}
|
||||
}
|
||||
|
||||
test "ZRANGEBYLEX/ZREVRANGEBYLEX/ZLEXCOUNT basics" {
|
||||
test "ZRANGEBYLEX/ZREVRANGEBYLEX/ZLEXCOUNT basics - $encoding" {
|
||||
create_default_lex_zset
|
||||
|
||||
# inclusive range
|
||||
@ -478,7 +510,7 @@ start_server {tags {"zset"}} {
|
||||
assert_equal {} [r zrevrangebylex zset (hill (omega]
|
||||
}
|
||||
|
||||
test "ZLEXCOUNT advanced" {
|
||||
test "ZLEXCOUNT advanced - $encoding" {
|
||||
create_default_lex_zset
|
||||
|
||||
assert_equal 9 [r zlexcount zset - +]
|
||||
@ -494,7 +526,7 @@ start_server {tags {"zset"}} {
|
||||
assert_equal 1 [r zlexcount zset (maxstring +]
|
||||
}
|
||||
|
||||
test "ZRANGEBYSLEX with LIMIT" {
|
||||
test "ZRANGEBYSLEX with LIMIT - $encoding" {
|
||||
create_default_lex_zset
|
||||
assert_equal {alpha bar} [r zrangebylex zset - \[cool LIMIT 0 2]
|
||||
assert_equal {bar cool} [r zrangebylex zset - \[cool LIMIT 1 2]
|
||||
@ -507,7 +539,7 @@ start_server {tags {"zset"}} {
|
||||
assert_equal {omega hill great foo} [r zrevrangebylex zset + \[d LIMIT 0 4]
|
||||
}
|
||||
|
||||
test "ZRANGEBYLEX with invalid lex range specifiers" {
|
||||
test "ZRANGEBYLEX with invalid lex range specifiers - $encoding" {
|
||||
assert_error "*not*string*" {r zrangebylex fooz foo bar}
|
||||
assert_error "*not*string*" {r zrangebylex fooz \[foo bar}
|
||||
assert_error "*not*string*" {r zrangebylex fooz foo \[bar}
|
||||
@ -515,7 +547,7 @@ start_server {tags {"zset"}} {
|
||||
assert_error "*not*string*" {r zrangebylex fooz -x \[bar}
|
||||
}
|
||||
|
||||
test "ZREMRANGEBYSCORE basics" {
|
||||
test "ZREMRANGEBYSCORE basics - $encoding" {
|
||||
proc remrangebyscore {min max} {
|
||||
create_zset zset {1 a 2 b 3 c 4 d 5 e}
|
||||
assert_equal 1 [r exists zset]
|
||||
@ -571,13 +603,13 @@ start_server {tags {"zset"}} {
|
||||
assert_equal 0 [r exists zset]
|
||||
}
|
||||
|
||||
test "ZREMRANGEBYSCORE with non-value min or max" {
|
||||
test "ZREMRANGEBYSCORE with non-value min or max - $encoding" {
|
||||
assert_error "*not*float*" {r zremrangebyscore fooz str 1}
|
||||
assert_error "*not*float*" {r zremrangebyscore fooz 1 str}
|
||||
assert_error "*not*float*" {r zremrangebyscore fooz 1 NaN}
|
||||
}
|
||||
|
||||
test "ZREMRANGEBYRANK basics" {
|
||||
test "ZREMRANGEBYRANK basics - $encoding" {
|
||||
proc remrangebyrank {min max} {
|
||||
create_zset zset {1 a 2 b 3 c 4 d 5 e}
|
||||
assert_equal 1 [r exists zset]
|
||||
@ -774,7 +806,7 @@ start_server {tags {"zset"}} {
|
||||
assert_equal -inf [r zscore zsetinf3 key]
|
||||
}
|
||||
|
||||
test "$cmd with NaN weights $encoding" {
|
||||
test "$cmd with NaN weights - $encoding" {
|
||||
r del zsetinf1 zsetinf2
|
||||
|
||||
r zadd zsetinf1 1.0 key
|
||||
@ -833,7 +865,7 @@ start_server {tags {"zset"}} {
|
||||
assert_equal {a 1 e 5} [r zrange zsete 0 -1 withscores]
|
||||
}
|
||||
|
||||
test "ZDIFF fuzzing" {
|
||||
test "ZDIFF fuzzing - $encoding" {
|
||||
for {set j 0} {$j < 100} {incr j} {
|
||||
unset -nocomplain s
|
||||
array set s {}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user