Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix ZUNIONSTORE/ZINTERSTORE to never store a NaN score.

When +inf and -inf are added, the result is NaN. We don't want NaN
scores in a sorted set, so agreed on the result of this operation being
zero.
  • Loading branch information...
commit d9e28bcf00a1e52c5e0c8cbc5f2c8c8cb7d7027f 1 parent cbf7e10
Pieter Noordhuis pietern authored
Showing with 28 additions and 0 deletions.
  1. +4 −0 src/t_zset.c
  2. +24 −0 tests/unit/type/zset.tcl
4 src/t_zset.c
View
@@ -541,6 +541,10 @@ int qsortCompareZsetopsrcByCardinality(const void *s1, const void *s2) {
inline static void zunionInterAggregate(double *target, double val, int aggregate) {
if (aggregate == REDIS_AGGR_SUM) {
*target = *target + val;
+ /* The result of adding two doubles is NaN when one variable
+ * is +inf and the other is -inf. When these numbers are added,
+ * we maintain the convention of the result being 0.0. */
+ if (isnan(*target)) *target = 0.0;
} else if (aggregate == REDIS_AGGR_MIN) {
*target = val < *target ? val : *target;
} else if (aggregate == REDIS_AGGR_MAX) {
24 tests/unit/type/zset.tcl
View
@@ -433,6 +433,30 @@ start_server {tags {"zset"}} {
list [r zinterstore zsetc 2 zseta zsetb aggregate max] [r zrange zsetc 0 -1 withscores]
} {2 {b 2 c 3}}
+ foreach cmd {ZUNIONSTORE ZINTERSTORE} {
+ test "$cmd with +inf/-inf scores" {
+ r zadd zsetinf1 +inf key
+ r zadd zsetinf2 +inf key
+ r $cmd zsetinf3 2 zsetinf1 zsetinf2
+ assert_equal inf [r zscore zsetinf3 key]
+
+ r zadd zsetinf1 -inf key
+ r zadd zsetinf2 +inf key
+ r $cmd zsetinf3 2 zsetinf1 zsetinf2
+ assert_equal 0 [r zscore zsetinf3 key]
+
+ r zadd zsetinf1 +inf key
+ r zadd zsetinf2 -inf key
+ r $cmd zsetinf3 2 zsetinf1 zsetinf2
+ assert_equal 0 [r zscore zsetinf3 key]
+
+ r zadd zsetinf1 -inf key
+ r zadd zsetinf2 -inf key
+ r $cmd zsetinf3 2 zsetinf1 zsetinf2
+ assert_equal -inf [r zscore zsetinf3 key]
+ }
+ }
+
tags {"slow"} {
test {ZSETs skiplist implementation backlink consistency test} {
set diff 0

2 comments on commit d9e28bc

amilkr

I think you forgot the following case:

redis> zadd z -inf neginf
(integer) 1
redis> zunionstore out 1 z weights 0
(integer) 1
redis> zrange out 0 -1 withscores
1) "neginf"
2) "nan"
Salvatore Sanfilippo
Owner

Thank you x1000, this is very useful. Apparently there is also a way to reach nan without the need of zunionstore but we are not able to find how this happens.

Please sign in to comment.
Something went wrong with that request. Please try again.