Permalink
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...
1 parent cbf7e10 commit d9e28bcf00a1e52c5e0c8cbc5f2c8c8cb7d7027f @pietern pietern committed Jul 29, 2010
Showing with 28 additions and 0 deletions.
  1. +4 −0 src/t_zset.c
  2. +24 −0 tests/unit/type/zset.tcl
View
4 src/t_zset.c
@@ -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) {
View
24 tests/unit/type/zset.tcl
@@ -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"
@antirez
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.