]> git.saurik.com Git - redis.git/blobdiff - src/t_zset.c
Typo
[redis.git] / src / t_zset.c
index 27522367fb277e59f95157a1cc5501e67c5707bb..9119e592ffdf38f99a0fefd0430e9a8ee5396b18 100644 (file)
@@ -180,6 +180,82 @@ typedef struct {
     int minex, maxex; /* are min or max exclusive? */
 } zrangespec;
 
+static int zslValueGteMin(double value, zrangespec *spec) {
+    return spec->minex ? (value > spec->min) : (value >= spec->min);
+}
+
+static int zslValueLteMax(double value, zrangespec *spec) {
+    return spec->maxex ? (value < spec->max) : (value <= spec->max);
+}
+
+static int zslValueInRange(double value, zrangespec *spec) {
+    return zslValueGteMin(value,spec) && zslValueLteMax(value,spec);
+}
+
+/* Returns if there is a part of the zset is in range. */
+int zslIsInRange(zskiplist *zsl, zrangespec *range) {
+    zskiplistNode *x;
+
+    /* Test for ranges that will always be empty. */
+    if (range->min > range->max ||
+            (range->min == range->max && (range->minex || range->maxex)))
+        return 0;
+    x = zsl->tail;
+    if (x == NULL || !zslValueGteMin(x->score,range))
+        return 0;
+    x = zsl->header->level[0].forward;
+    if (x == NULL || !zslValueLteMax(x->score,range))
+        return 0;
+    return 1;
+}
+
+/* Find the first node that is contained in the specified range.
+ * Returns NULL when no element is contained in the range. */
+zskiplistNode *zslFirstInRange(zskiplist *zsl, zrangespec range) {
+    zskiplistNode *x;
+    int i;
+
+    /* If everything is out of range, return early. */
+    if (!zslIsInRange(zsl,&range)) return NULL;
+
+    x = zsl->header;
+    for (i = zsl->level-1; i >= 0; i--) {
+        /* Go forward while *OUT* of range. */
+        while (x->level[i].forward &&
+            !zslValueGteMin(x->level[i].forward->score,&range))
+                x = x->level[i].forward;
+    }
+
+    /* The tail is in range, so the previous block should always return a
+     * node that is non-NULL and the last one to be out of range. */
+    x = x->level[0].forward;
+    redisAssert(x != NULL && zslValueInRange(x->score,&range));
+    return x;
+}
+
+/* Find the last node that is contained in the specified range.
+ * Returns NULL when no element is contained in the range. */
+zskiplistNode *zslLastInRange(zskiplist *zsl, zrangespec range) {
+    zskiplistNode *x;
+    int i;
+
+    /* If everything is out of range, return early. */
+    if (!zslIsInRange(zsl,&range)) return NULL;
+
+    x = zsl->header;
+    for (i = zsl->level-1; i >= 0; i--) {
+        /* Go forward while *IN* range. */
+        while (x->level[i].forward &&
+            zslValueLteMax(x->level[i].forward->score,&range))
+                x = x->level[i].forward;
+    }
+
+    /* The header is in range, so the previous block should always return a
+     * node that is non-NULL and in range. */
+    redisAssert(x != NULL && zslValueInRange(x->score,&range));
+    return x;
+}
+
 /* Delete all the elements with score between min and max from the skiplist.
  * Min and mx are inclusive, so a score >= min || score <= max is deleted.
  * Note that this function takes the reference to the hash table view of the
@@ -243,27 +319,11 @@ unsigned long zslDeleteRangeByRank(zskiplist *zsl, unsigned int start, unsigned
     return removed;
 }
 
-/* Find the first node having a score equal or greater than the specified one.
- * Returns NULL if there is no match. */
-zskiplistNode *zslFirstWithScore(zskiplist *zsl, double score) {
-    zskiplistNode *x;
-    int i;
-
-    x = zsl->header;
-    for (i = zsl->level-1; i >= 0; i--) {
-        while (x->level[i].forward && x->level[i].forward->score < score)
-            x = x->level[i].forward;
-    }
-    /* We may have multiple elements with the same score, what we need
-     * is to find the element with both the right score and object. */
-    return x->level[0].forward;
-}
-
 /* Find the rank for an element by both score and key.
  * Returns 0 when the element cannot be found, rank otherwise.
  * Note that the rank is 1-based due to the span of zsl->header to the
  * first element. */
-unsigned long zslistTypeGetRank(zskiplist *zsl, double score, robj *o) {
+unsigned long zslGetRank(zskiplist *zsl, double score, robj *o) {
     zskiplistNode *x;
     unsigned long rank = 0;
     int i;
@@ -287,7 +347,7 @@ unsigned long zslistTypeGetRank(zskiplist *zsl, double score, robj *o) {
 }
 
 /* Finds an element by its rank. The rank argument needs to be 1-based. */
-zskiplistNode* zslistTypeGetElementByRank(zskiplist *zsl, unsigned long rank) {
+zskiplistNode* zslGetElementByRank(zskiplist *zsl, unsigned long rank) {
     zskiplistNode *x;
     unsigned long traversed = 0;
     int i;
@@ -343,141 +403,334 @@ static int zslParseRange(robj *min, robj *max, zrangespec *spec) {
     return REDIS_OK;
 }
 
+/*-----------------------------------------------------------------------------
+ * Ziplist-backed sorted set API
+ *----------------------------------------------------------------------------*/
+
+double zzlGetScore(unsigned char *sptr) {
+    unsigned char *vstr;
+    unsigned int vlen;
+    long long vlong;
+    char buf[128];
+    double score;
+
+    redisAssert(sptr != NULL);
+    redisAssert(ziplistGet(sptr,&vstr,&vlen,&vlong));
+
+    if (vstr) {
+        memcpy(buf,vstr,vlen);
+        buf[vlen] = '\0';
+        score = strtod(buf,NULL);
+    } else {
+        score = vlong;
+    }
+
+    return score;
+}
+
+/* Compare element in sorted set with given element. */
+int zzlCompareElements(unsigned char *eptr, unsigned char *cstr, unsigned int clen) {
+    unsigned char *vstr;
+    unsigned int vlen;
+    long long vlong;
+    unsigned char vbuf[32];
+    int minlen, cmp;
+
+    redisAssert(ziplistGet(eptr,&vstr,&vlen,&vlong));
+    if (vstr == NULL) {
+        /* Store string representation of long long in buf. */
+        vlen = ll2string((char*)vbuf,sizeof(vbuf),vlong);
+        vstr = vbuf;
+    }
+
+    minlen = (vlen < clen) ? vlen : clen;
+    cmp = memcmp(vstr,cstr,minlen);
+    if (cmp == 0) return vlen-clen;
+    return cmp;
+}
+
+unsigned int zzlLength(robj *zobj) {
+    unsigned char *zl = zobj->ptr;
+    return ziplistLen(zl)/2;
+}
+
+unsigned char *zzlFind(robj *zobj, robj *ele, double *score) {
+    unsigned char *zl = zobj->ptr;
+    unsigned char *eptr = ziplistIndex(zl,0), *sptr;
+
+    ele = getDecodedObject(ele);
+    while (eptr != NULL) {
+        sptr = ziplistNext(zl,eptr);
+        redisAssert(sptr != NULL);
+
+        if (ziplistCompare(eptr,ele->ptr,sdslen(ele->ptr))) {
+            /* Matching element, pull out score. */
+            if (score != NULL) *score = zzlGetScore(sptr);
+            decrRefCount(ele);
+            return eptr;
+        }
+
+        /* Move to next element. */
+        eptr = ziplistNext(zl,sptr);
+    }
+
+    decrRefCount(ele);
+    return NULL;
+}
+
+/* Delete (element,score) pair from ziplist. Use local copy of eptr because we
+ * don't want to modify the one given as argument. */
+int zzlDelete(robj *zobj, unsigned char *eptr) {
+    unsigned char *zl = zobj->ptr;
+    unsigned char *p = eptr;
+
+    /* TODO: add function to ziplist API to delete N elements from offset. */
+    zl = ziplistDelete(zl,&p);
+    zl = ziplistDelete(zl,&p);
+    zobj->ptr = zl;
+    return REDIS_OK;
+}
+
+int zzlInsertAt(robj *zobj, robj *ele, double score, unsigned char *eptr) {
+    unsigned char *zl = zobj->ptr;
+    unsigned char *sptr;
+    char scorebuf[128];
+    int scorelen;
+    int offset;
+
+    redisAssert(ele->encoding == REDIS_ENCODING_RAW);
+    scorelen = d2string(scorebuf,sizeof(scorebuf),score);
+    if (eptr == NULL) {
+        zl = ziplistPush(zl,ele->ptr,sdslen(ele->ptr),ZIPLIST_TAIL);
+        zl = ziplistPush(zl,(unsigned char*)scorebuf,scorelen,ZIPLIST_TAIL);
+    } else {
+        /* Keep offset relative to zl, as it might be re-allocated. */
+        offset = eptr-zl;
+        zl = ziplistInsert(zl,eptr,ele->ptr,sdslen(ele->ptr));
+        eptr = zl+offset;
+
+        /* Insert score after the element. */
+        redisAssert((sptr = ziplistNext(zl,eptr)) != NULL);
+        zl = ziplistInsert(zl,sptr,(unsigned char*)scorebuf,scorelen);
+    }
+
+    zobj->ptr = zl;
+    return REDIS_OK;
+}
+
+/* Insert (element,score) pair in ziplist. This function assumes the element is
+ * not yet present in the list. */
+int zzlInsert(robj *zobj, robj *ele, double score) {
+    unsigned char *zl = zobj->ptr;
+    unsigned char *eptr = ziplistIndex(zl,0), *sptr;
+    double s;
+
+    ele = getDecodedObject(ele);
+    while (eptr != NULL) {
+        sptr = ziplistNext(zl,eptr);
+        redisAssert(sptr != NULL);
+        s = zzlGetScore(sptr);
+
+        if (s > score) {
+            /* First element with score larger than score for element to be
+             * inserted. This means we should take its spot in the list to
+             * maintain ordering. */
+            zzlInsertAt(zobj,ele,score,eptr);
+            break;
+        } else if (s == score) {
+            /* Ensure lexicographical ordering for elements. */
+            if (zzlCompareElements(eptr,ele->ptr,sdslen(ele->ptr)) < 0) {
+                zzlInsertAt(zobj,ele,score,eptr);
+                break;
+            }
+        }
+
+        /* Move to next element. */
+        eptr = ziplistNext(zl,sptr);
+    }
+
+    /* Push on tail of list when it was not yet inserted. */
+    if (eptr == NULL)
+        zzlInsertAt(zobj,ele,score,NULL);
+
+    decrRefCount(ele);
+    return REDIS_OK;
+}
 
 /*-----------------------------------------------------------------------------
  * Sorted set commands 
  *----------------------------------------------------------------------------*/
 
 /* This generic command implements both ZADD and ZINCRBY. */
-void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double score, int incr) {
-    robj *zsetobj;
-    zset *zs;
-    zskiplistNode *znode;
+void zaddGenericCommand(redisClient *c, int incr) {
+    static char *nanerr = "resulting score is not a number (NaN)";
+    robj *key = c->argv[1];
+    robj *ele;
+    robj *zobj;
+    robj *curobj;
+    double score, curscore = 0.0;
 
-    zsetobj = lookupKeyWrite(c->db,key);
-    if (zsetobj == NULL) {
-        zsetobj = createZsetObject();
-        dbAdd(c->db,key,zsetobj);
+    if (getDoubleFromObjectOrReply(c,c->argv[2],&score,NULL) != REDIS_OK)
+        return;
+
+    zobj = lookupKeyWrite(c->db,key);
+    if (zobj == NULL) {
+        zobj = createZsetZiplistObject();
+        dbAdd(c->db,key,zobj);
     } else {
-        if (zsetobj->type != REDIS_ZSET) {
+        if (zobj->type != REDIS_ZSET) {
             addReply(c,shared.wrongtypeerr);
             return;
         }
     }
-    zs = zsetobj->ptr;
 
-    /* Since both ZADD and ZINCRBY are implemented here, we need to increment
-     * the score first by the current score if ZINCRBY is called. */
-    if (incr) {
-        /* Read the old score. If the element was not present starts from 0 */
-        dictEntry *de = dictFind(zs->dict,ele);
-        if (de != NULL)
-            score += *(double*)dictGetEntryVal(de);
-
-        if (isnan(score)) {
-            addReplyError(c,"resulting score is not a number (NaN)");
-            /* Note that we don't need to check if the zset may be empty and
-             * should be removed here, as we can only obtain Nan as score if
-             * there was already an element in the sorted set. */
-            return;
-        }
-    }
+    if (zobj->encoding == REDIS_ENCODING_ZIPLIST) {
+        unsigned char *eptr;
+
+        /* Prefer non-encoded element when dealing with ziplists. */
+        ele = c->argv[3];
+        if ((eptr = zzlFind(zobj,ele,&curscore)) != NULL) {
+            if (incr) {
+                score += curscore;
+                if (isnan(score)) {
+                    addReplyError(c,nanerr);
+                    /* Don't need to check if the sorted set is empty, because
+                     * we know it has at least one element. */
+                    return;
+                }
+            }
 
-    /* We need to remove and re-insert the element when it was already present
-     * in the dictionary, to update the skiplist. Note that we delay adding a
-     * pointer to the score because we want to reference the score in the
-     * skiplist node. */
-    if (dictAdd(zs->dict,ele,NULL) == DICT_OK) {
-        dictEntry *de;
+            /* Remove and re-insert when score changed. */
+            if (score != curscore) {
+                redisAssert(zzlDelete(zobj,eptr) == REDIS_OK);
+                redisAssert(zzlInsert(zobj,ele,score) == REDIS_OK);
 
-        /* New element */
-        incrRefCount(ele); /* added to hash */
-        znode = zslInsert(zs->zsl,score,ele);
-        incrRefCount(ele); /* added to skiplist */
+                signalModifiedKey(c->db,key);
+                server.dirty++;
+            }
 
-        /* Update the score in the dict entry */
-        de = dictFind(zs->dict,ele);
-        redisAssert(de != NULL);
-        dictGetEntryVal(de) = &znode->score;
-        signalModifiedKey(c->db,c->argv[1]);
-        server.dirty++;
-        if (incr)
-            addReplyDouble(c,score);
-        else
-            addReply(c,shared.cone);
-    } else {
+            if (incr) /* ZINCRBY */
+                addReplyDouble(c,score);
+            else /* ZADD */
+                addReply(c,shared.czero);
+        } else {
+            redisAssert(zzlInsert(zobj,ele,score) == REDIS_OK);
+
+            signalModifiedKey(c->db,key);
+            server.dirty++;
+
+            if (incr) /* ZINCRBY */
+                addReplyDouble(c,score);
+            else /* ZADD */
+                addReply(c,shared.cone);
+        }
+    } else if (zobj->encoding == REDIS_ENCODING_RAW) {
+        zset *zs = zobj->ptr;
+        zskiplistNode *znode;
         dictEntry *de;
-        robj *curobj;
-        double *curscore;
-        int deleted;
 
-        /* Update score */
+        ele = c->argv[3] = tryObjectEncoding(c->argv[3]);
         de = dictFind(zs->dict,ele);
-        redisAssert(de != NULL);
-        curobj = dictGetEntryKey(de);
-        curscore = dictGetEntryVal(de);
-
-        /* When the score is updated, reuse the existing string object to
-         * prevent extra alloc/dealloc of strings on ZINCRBY. */
-        if (score != *curscore) {
-            deleted = zslDelete(zs->zsl,*curscore,curobj);
-            redisAssert(deleted != 0);
-            znode = zslInsert(zs->zsl,score,curobj);
-            incrRefCount(curobj);
-
-            /* Update the score in the current dict entry */
-            dictGetEntryVal(de) = &znode->score;
-            signalModifiedKey(c->db,c->argv[1]);
+        if (de != NULL) {
+            curobj = dictGetEntryKey(de);
+            curscore = *(double*)dictGetEntryVal(de);
+
+            if (incr) {
+                score += curscore;
+                if (isnan(score)) {
+                    addReplyError(c,nanerr);
+                    /* Don't need to check if the sorted set is empty, because
+                     * we know it has at least one element. */
+                    return;
+                }
+            }
+
+            /* Remove and re-insert when score changed. We can safely delete
+             * the key object from the skiplist, since the dictionary still has
+             * a reference to it. */
+            if (score != curscore) {
+                redisAssert(zslDelete(zs->zsl,curscore,curobj));
+                znode = zslInsert(zs->zsl,score,curobj);
+                incrRefCount(curobj); /* Re-inserted in skiplist. */
+                dictGetEntryVal(de) = &znode->score; /* Update score ptr. */
+
+                signalModifiedKey(c->db,key);
+                server.dirty++;
+            }
+
+            if (incr) /* ZINCRBY */
+                addReplyDouble(c,score);
+            else /* ZADD */
+                addReply(c,shared.czero);
+        } else {
+            znode = zslInsert(zs->zsl,score,ele);
+            incrRefCount(ele); /* Inserted in skiplist. */
+            redisAssert(dictAdd(zs->dict,ele,&znode->score) == DICT_OK);
+            incrRefCount(ele); /* Added to dictionary. */
+
+            signalModifiedKey(c->db,key);
             server.dirty++;
+
+            if (incr) /* ZINCRBY */
+                addReplyDouble(c,score);
+            else /* ZADD */
+                addReply(c,shared.cone);
         }
-        if (incr)
-            addReplyDouble(c,score);
-        else
-            addReply(c,shared.czero);
+    } else {
+        redisPanic("Unknown sorted set encoding");
     }
 }
 
 void zaddCommand(redisClient *c) {
-    double scoreval;
-    if (getDoubleFromObjectOrReply(c,c->argv[2],&scoreval,NULL) != REDIS_OK) return;
-    c->argv[3] = tryObjectEncoding(c->argv[3]);
-    zaddGenericCommand(c,c->argv[1],c->argv[3],scoreval,0);
+    zaddGenericCommand(c,0);
 }
 
 void zincrbyCommand(redisClient *c) {
-    double scoreval;
-    if (getDoubleFromObjectOrReply(c,c->argv[2],&scoreval,NULL) != REDIS_OK) return;
-    c->argv[3] = tryObjectEncoding(c->argv[3]);
-    zaddGenericCommand(c,c->argv[1],c->argv[3],scoreval,1);
+    zaddGenericCommand(c,1);
 }
 
 void zremCommand(redisClient *c) {
-    robj *zsetobj;
-    zset *zs;
-    dictEntry *de;
-    double curscore;
-    int deleted;
+    robj *key = c->argv[1];
+    robj *ele = c->argv[2];
+    robj *zobj;
 
-    if ((zsetobj = lookupKeyWriteOrReply(c,c->argv[1],shared.czero)) == NULL ||
-        checkType(c,zsetobj,REDIS_ZSET)) return;
+    if ((zobj = lookupKeyWriteOrReply(c,key,shared.czero)) == NULL ||
+        checkType(c,zobj,REDIS_ZSET)) return;
 
-    zs = zsetobj->ptr;
-    c->argv[2] = tryObjectEncoding(c->argv[2]);
-    de = dictFind(zs->dict,c->argv[2]);
-    if (de == NULL) {
-        addReply(c,shared.czero);
-        return;
+    if (zobj->encoding == REDIS_ENCODING_ZIPLIST) {
+        unsigned char *eptr;
+
+        if ((eptr = zzlFind(zobj,ele,NULL)) != NULL) {
+            redisAssert(zzlDelete(zobj,eptr) == REDIS_OK);
+            if (zzlLength(zobj) == 0) dbDelete(c->db,key);
+        } else {
+            addReply(c,shared.czero);
+            return;
+        }
+    } else if (zobj->encoding == REDIS_ENCODING_RAW) {
+        zset *zs = zobj->ptr;
+        dictEntry *de;
+        double score;
+
+        de = dictFind(zs->dict,ele);
+        if (de != NULL) {
+            /* Delete from the skiplist */
+            score = *(double*)dictGetEntryVal(de);
+            redisAssert(zslDelete(zs->zsl,score,ele));
+
+            /* Delete from the hash table */
+            dictDelete(zs->dict,ele);
+            if (htNeedsResize(zs->dict)) dictResize(zs->dict);
+            if (dictSize(zs->dict) == 0) dbDelete(c->db,key);
+        } else {
+            addReply(c,shared.czero);
+            return;
+        }
+    } else {
+        redisPanic("Unknown sorted set encoding");
     }
-    /* Delete from the skiplist */
-    curscore = *(double*)dictGetEntryVal(de);
-    deleted = zslDelete(zs->zsl,curscore,c->argv[2]);
-    redisAssert(deleted != 0);
 
-    /* Delete from the hash table */
-    dictDelete(zs->dict,c->argv[2]);
-    if (htNeedsResize(zs->dict)) dictResize(zs->dict);
-    if (dictSize(zs->dict) == 0) dbDelete(c->db,c->argv[1]);
-    signalModifiedKey(c->db,c->argv[1]);
+    signalModifiedKey(c->db,key);
     server.dirty++;
     addReply(c,shared.cone);
 }
@@ -810,10 +1063,10 @@ void zrangeGenericCommand(redisClient *c, int reverse) {
     /* check if starting point is trivial, before searching
      * the element in log(N) time */
     if (reverse) {
-        ln = start == 0 ? zsl->tail : zslistTypeGetElementByRank(zsl, llen-start);
+        ln = start == 0 ? zsl->tail : zslGetElementByRank(zsl, llen-start);
     } else {
         ln = start == 0 ?
-            zsl->header->level[0].forward : zslistTypeGetElementByRank(zsl, start+1);
+            zsl->header->level[0].forward : zslGetElementByRank(zsl, start+1);
     }
 
     /* Return the result in form of a multi-bulk reply */
@@ -847,9 +1100,18 @@ void genericZrangebyscoreCommand(redisClient *c, int reverse, int justcount) {
     int withscores = 0;
     unsigned long rangelen = 0;
     void *replylen = NULL;
+    int minidx, maxidx;
 
     /* Parse the range arguments. */
-    if (zslParseRange(c->argv[2],c->argv[3],&range) != REDIS_OK) {
+    if (reverse) {
+        /* Range is given as [max,min] */
+        maxidx = 2; minidx = 3;
+    } else {
+        /* Range is given as [min,max] */
+        minidx = 2; maxidx = 3;
+    }
+
+    if (zslParseRange(c->argv[minidx],c->argv[maxidx],&range) != REDIS_OK) {
         addReplyError(c,"min or max is not a double");
         return;
     }
@@ -882,33 +1144,11 @@ void genericZrangebyscoreCommand(redisClient *c, int reverse, int justcount) {
     zsetobj = o->ptr;
     zsl = zsetobj->zsl;
 
-    /* If reversed, assume the elements are sorted from high to low score. */
-    ln = zslFirstWithScore(zsl,range.min);
+    /* If reversed, get the last node in range as starting point. */
     if (reverse) {
-        /* If range.min is out of range, ln will be NULL and we need to use
-         * the tail of the skiplist as first node of the range. */
-        if (ln == NULL) ln = zsl->tail;
-
-        /* zslFirstWithScore returns the first element with where with
-         * score >= range.min, so backtrack to make sure the element we use
-         * here has score <= range.min. */
-        while (ln && ln->score > range.min) ln = ln->backward;
-
-        /* Move to the right element according to the range spec. */
-        if (range.minex) {
-            /* Find last element with score < range.min */
-            while (ln && ln->score == range.min) ln = ln->backward;
-        } else {
-            /* Find last element with score <= range.min */
-            while (ln && ln->level[0].forward &&
-                         ln->level[0].forward->score == range.min)
-                ln = ln->level[0].forward;
-        }
+        ln = zslLastInRange(zsl,range);
     } else {
-        if (range.minex) {
-            /* Find first element with score > range.min */
-            while (ln && ln->score == range.min) ln = ln->level[0].forward;
-        }
+        ln = zslFirstInRange(zsl,range);
     }
 
     /* No "first" element in the specified interval. */
@@ -917,40 +1157,24 @@ void genericZrangebyscoreCommand(redisClient *c, int reverse, int justcount) {
         return;
     }
 
-    /* We don't know in advance how many matching elements there
-     * are in the list, so we push this object that will represent
-     * the multi-bulk length in the output buffer, and will "fix"
-     * it later */
+    /* We don't know in advance how many matching elements there are in the
+     * list, so we push this object that will represent the multi-bulk length
+     * in the output buffer, and will "fix" it later */
     if (!justcount)
         replylen = addDeferredMultiBulkLength(c);
 
     /* If there is an offset, just traverse the number of elements without
      * checking the score because that is done in the next loop. */
     while(ln && offset--) {
-        if (reverse)
-            ln = ln->backward;
-        else
-            ln = ln->level[0].forward;
+        ln = reverse ? ln->backward : ln->level[0].forward;
     }
 
     while (ln && limit--) {
-        /* Check if this this element is in range. */
+        /* Abort when the node is no longer in range. */
         if (reverse) {
-            if (range.maxex) {
-                /* Element should have score > range.max */
-                if (ln->score <= range.max) break;
-            } else {
-                /* Element should have score >= range.max */
-                if (ln->score < range.max) break;
-            }
+            if (!zslValueGteMin(ln->score,&range)) break;
         } else {
-            if (range.maxex) {
-                /* Element should have score < range.max */
-                if (ln->score >= range.max) break;
-            } else {
-                /* Element should have score <= range.max */
-                if (ln->score > range.max) break;
-            }
+            if (!zslValueLteMax(ln->score,&range)) break;
         }
 
         /* Do our magic */
@@ -961,10 +1185,8 @@ void genericZrangebyscoreCommand(redisClient *c, int reverse, int justcount) {
                 addReplyDouble(c,ln->score);
         }
 
-        if (reverse)
-            ln = ln->backward;
-        else
-            ln = ln->level[0].forward;
+        /* Move to next node */
+        ln = reverse ? ln->backward : ln->level[0].forward;
     }
 
     if (justcount) {
@@ -1039,7 +1261,7 @@ void zrankGenericCommand(redisClient *c, int reverse) {
     }
 
     score = dictGetEntryVal(de);
-    rank = zslistTypeGetRank(zsl, *score, c->argv[2]);
+    rank = zslGetRank(zsl, *score, c->argv[2]);
     if (rank) {
         if (reverse) {
             addReplyLongLong(c, zsl->length - rank);