From e99b890955aa11264a8fa2a2b9302267ca902670 Mon Sep 17 00:00:00 2001 From: Bara' Hasheesh Date: Tue, 15 Sep 2026 13:53:40 +0000 Subject: [PATCH 1/3] remove redundant isnan checks in `HINCRBYFLOAT`, `INCREX` Signed-off-by: Bara' Hasheesh --- src/t_hash.c | 8 ++++---- src/t_string.c | 4 ---- tests/unit/type/hash.tcl | 15 +++++++++++++++ tests/unit/type/incr.tcl | 11 +++++++++++ 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/t_hash.c b/src/t_hash.c index d8a1a58aeb2..40e0a90c7e7 100644 --- a/src/t_hash.c +++ b/src/t_hash.c @@ -1280,8 +1280,8 @@ void hincrbyfloatCommand(client *c) { mstime_t expiry = EXPIRY_NONE; if (getLongDoubleFromObjectOrReply(c, c->argv[3], &incr, NULL) != C_OK) return; - if (isnan(incr) || isinf(incr)) { - addReplyError(c, "value is NaN or Infinity"); + if (isinf(incr)) { + addReplyError(c, "value is Infinity"); return; } if ((o = hashTypeLookupWriteOrCreate(c, c->argv[1])) == NULL) return; @@ -1300,8 +1300,8 @@ void hincrbyfloatCommand(client *c) { } value += incr; - if (isnan(value) || isinf(value)) { - addReplyError(c, "increment would produce NaN or Infinity"); + if (isinf(value)) { + addReplyError(c, "increment would produce Infinity"); return; } diff --git a/src/t_string.c b/src/t_string.c index 8f9a874f575..3d23f8f4373 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -880,10 +880,6 @@ void increxCommand(client *c) { return; } value_ld = oldvalue_ld + incr_ld; - if (isnan(value_ld)) { - addReplyError(c, "Increment is not a valid float"); - return; - } if (isinf(value_ld)) { addReplyArrayLen(c, 2); addReplyHumanLongDouble(c, oldvalue_ld); diff --git a/tests/unit/type/hash.tcl b/tests/unit/type/hash.tcl index 5fcd639ef68..ad5d8601cef 100644 --- a/tests/unit/type/hash.tcl +++ b/tests/unit/type/hash.tcl @@ -960,6 +960,21 @@ start_server {tags {"hash"}} { assert_error "*value is NaN or Infinity*" {r hincrbyfloat hfoo field +inf} assert_equal 0 [r exists hfoo] } {} {valgrind:skip} + + test {HINCRBYFLOAT NaN increment is rejected by the parser, not the NaN check} { + r del hfoo + assert_error "*value is not a valid float*" {r hincrbyfloat hfoo field nan} + } + + test {HINCRBYFLOAT cannot reach a NaN result} { + r del hfoo + r hset hfoo field inf + assert_error "*would produce Infinity*" {r hincrbyfloat hfoo field 1} + assert_equal inf [r hget hfoo field] + # An infinite increment is dropped before the result is ever computed. + assert_error "*value is Infinity*" {r hincrbyfloat hfoo field -inf} + assert_equal inf [r hget hfoo field] + } {} {valgrind:skip} } start_server {config "minimal.conf" tags {"hash" "external:skip"} overrides {io-threads 4 io-threads-always-active yes hash-max-listpack-entries 0}} { diff --git a/tests/unit/type/incr.tcl b/tests/unit/type/incr.tcl index 8c28458ee95..79ebfc52503 100644 --- a/tests/unit/type/incr.tcl +++ b/tests/unit/type/incr.tcl @@ -151,6 +151,17 @@ start_server {tags {"incr"}} { # perform divisions. } {ERR *would produce*} {valgrind:skip} + test {INCRBYFLOAT NaN increment is rejected by the parser} { + r set foo 0 + assert_error "*value is not a valid float*" {r incrbyfloat foo nan} + } + + test {INCRBYFLOAT reaches the NaN result check via inf + -inf} { + r set foo inf + assert_error "*would produce NaN or Infinity*" {r incrbyfloat foo -inf} + assert_equal inf [r get foo] + } {} {valgrind:skip} + test {INCRBYFLOAT decrement} { r set foo 1 roundFloat [r incrbyfloat foo -1.1] From acf6bf6dc32bd82601d47b73e58e31521502d046 Mon Sep 17 00:00:00 2001 From: Bara' Hasheesh Date: Tue, 15 Sep 2026 14:38:31 +0000 Subject: [PATCH 2/3] Fixing test check Signed-off-by: Bara' Hasheesh --- tests/unit/type/hash.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/type/hash.tcl b/tests/unit/type/hash.tcl index ad5d8601cef..d01b88d7c51 100644 --- a/tests/unit/type/hash.tcl +++ b/tests/unit/type/hash.tcl @@ -957,7 +957,7 @@ start_server {tags {"hash"}} { # On some platforms strtold("+inf") with valgrind returns a non-inf result test {HINCRBYFLOAT does not allow NaN or Infinity} { - assert_error "*value is NaN or Infinity*" {r hincrbyfloat hfoo field +inf} + assert_error "*value is Infinity*" {r hincrbyfloat hfoo field +inf} assert_equal 0 [r exists hfoo] } {} {valgrind:skip} From 5f375d1b3c0b6797381fead18282dcc83dc12f8e Mon Sep 17 00:00:00 2001 From: Bara' Hasheesh Date: Wed, 16 Sep 2026 05:15:40 +0000 Subject: [PATCH 3/3] Review feedback - Revert back error message - Update test names Signed-off-by: Bara' Hasheesh --- src/t_hash.c | 2 +- tests/unit/type/hash.tcl | 8 ++++---- tests/unit/type/incr.tcl | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/t_hash.c b/src/t_hash.c index 40e0a90c7e7..e4a86338464 100644 --- a/src/t_hash.c +++ b/src/t_hash.c @@ -1281,7 +1281,7 @@ void hincrbyfloatCommand(client *c) { if (getLongDoubleFromObjectOrReply(c, c->argv[3], &incr, NULL) != C_OK) return; if (isinf(incr)) { - addReplyError(c, "value is Infinity"); + addReplyError(c, "value is NaN or Infinity"); return; } if ((o = hashTypeLookupWriteOrCreate(c, c->argv[1])) == NULL) return; diff --git a/tests/unit/type/hash.tcl b/tests/unit/type/hash.tcl index d01b88d7c51..71f76d171cd 100644 --- a/tests/unit/type/hash.tcl +++ b/tests/unit/type/hash.tcl @@ -957,22 +957,22 @@ start_server {tags {"hash"}} { # On some platforms strtold("+inf") with valgrind returns a non-inf result test {HINCRBYFLOAT does not allow NaN or Infinity} { - assert_error "*value is Infinity*" {r hincrbyfloat hfoo field +inf} + assert_error "*value is NaN or Infinity*" {r hincrbyfloat hfoo field +inf} assert_equal 0 [r exists hfoo] } {} {valgrind:skip} - test {HINCRBYFLOAT NaN increment is rejected by the parser, not the NaN check} { + test {HINCRBYFLOAT rejects a NaN increment} { r del hfoo assert_error "*value is not a valid float*" {r hincrbyfloat hfoo field nan} } - test {HINCRBYFLOAT cannot reach a NaN result} { + test {HINCRBYFLOAT with an infinite field value} { r del hfoo r hset hfoo field inf assert_error "*would produce Infinity*" {r hincrbyfloat hfoo field 1} assert_equal inf [r hget hfoo field] # An infinite increment is dropped before the result is ever computed. - assert_error "*value is Infinity*" {r hincrbyfloat hfoo field -inf} + assert_error "*value is NaN or Infinity*" {r hincrbyfloat hfoo field -inf} assert_equal inf [r hget hfoo field] } {} {valgrind:skip} } diff --git a/tests/unit/type/incr.tcl b/tests/unit/type/incr.tcl index 79ebfc52503..658ef352cd1 100644 --- a/tests/unit/type/incr.tcl +++ b/tests/unit/type/incr.tcl @@ -151,12 +151,12 @@ start_server {tags {"incr"}} { # perform divisions. } {ERR *would produce*} {valgrind:skip} - test {INCRBYFLOAT NaN increment is rejected by the parser} { + test {INCRBYFLOAT rejects a NaN increment} { r set foo 0 assert_error "*value is not a valid float*" {r incrbyfloat foo nan} } - test {INCRBYFLOAT reaches the NaN result check via inf + -inf} { + test {INCRBYFLOAT with an infinite field value} { r set foo inf assert_error "*would produce NaN or Infinity*" {r incrbyfloat foo -inf} assert_equal inf [r get foo]