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).

1 files changed, 73 insertions, 41 deletions

diff --git a/tests/unit/type/zset.tcl b/tests/unit/type/zset.tcl
index 0170d2bfa..96647f778 100644
--- a/tests/unit/type/zset.tcl
+++ b/tests/unit/type/zset.tcl
@@ -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 {}