From 4e54b85a19027855f05b4c825ad4ac0c71fd9fea Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 27 Feb 2015 15:30:15 +0100 Subject: [PATCH] Hash: HSTRLEN (was HVSTRLEN) improved. 1. HVSTRLEN -> HSTRLEN. It's unlikely one needs the length of the key, not clear how the API would work (by value does not make sense) and there will be better names anyway. 2. Default is to return 0 when field is missing. 3. Default is to return 0 when key is missing. 4. The implementation was slower than needed, and produced unnecessary COW. Related issue #2415. --- src/redis.c | 2 +- src/redis.h | 2 +- src/t_hash.c | 15 +++++---------- tests/unit/type/hash.tcl | 22 ++++++++++------------ 4 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/redis.c b/src/redis.c index 41d74f4e..86b5a9eb 100644 --- a/src/redis.c +++ b/src/redis.c @@ -202,7 +202,7 @@ struct redisCommand redisCommandTable[] = { {"hincrbyfloat",hincrbyfloatCommand,4,"wmF",0,NULL,1,1,1,0,0}, {"hdel",hdelCommand,-3,"wF",0,NULL,1,1,1,0,0}, {"hlen",hlenCommand,2,"rF",0,NULL,1,1,1,0,0}, - {"hvstrlen",hvstrlenCommand,3,"rF",0,NULL,1,1,1,0,0}, + {"hstrlen",hstrlenCommand,3,"rF",0,NULL,1,1,1,0,0}, {"hkeys",hkeysCommand,2,"rS",0,NULL,1,1,1,0,0}, {"hvals",hvalsCommand,2,"rS",0,NULL,1,1,1,0,0}, {"hgetall",hgetallCommand,2,"r",0,NULL,1,1,1,0,0}, diff --git a/src/redis.h b/src/redis.h index 142e830e..232ada5e 100644 --- a/src/redis.h +++ b/src/redis.h @@ -1516,7 +1516,7 @@ void hmsetCommand(redisClient *c); void hmgetCommand(redisClient *c); void hdelCommand(redisClient *c); void hlenCommand(redisClient *c); -void hvstrlenCommand(redisClient *c); +void hstrlenCommand(redisClient *c); void zremrangebyrankCommand(redisClient *c); void zunionstoreCommand(redisClient *c); void zinterstoreCommand(redisClient *c); diff --git a/src/t_hash.c b/src/t_hash.c index c05bcdff..2058dfd8 100644 --- a/src/t_hash.c +++ b/src/t_hash.c @@ -701,24 +701,19 @@ void hdelCommand(redisClient *c) { void hlenCommand(redisClient *c) { robj *o; + if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.czero)) == NULL || checkType(c,o,REDIS_HASH)) return; addReplyLongLong(c,hashTypeLength(o)); } -void hvstrlenCommand(redisClient *c) { +void hstrlenCommand(redisClient *c) { robj *o; - robj *value; - if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.nullbulk)) == NULL || - checkType(c,o,REDIS_HASH)) return; - if ((value = hashTypeGetObject(o,c->argv[2])) == NULL) { - addReply(c, shared.nullbulk); - } else { - addReplyLongLong(c,stringObjectLen(value)); - decrRefCount(value); - } + if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.czero)) == NULL || + checkType(c,o,REDIS_HASH)) return; + addReplyLongLong(c,hashTypeGetValueLength(o,c->argv[2])); } static void addHashIteratorCursorToReply(redisClient *c, hashTypeIterator *hi, int what) { diff --git a/tests/unit/type/hash.tcl b/tests/unit/type/hash.tcl index 3d9be1fc..bd446427 100644 --- a/tests/unit/type/hash.tcl +++ b/tests/unit/type/hash.tcl @@ -390,36 +390,34 @@ start_server {tags {"hash"}} { lappend rv [string match "ERR*not*float*" $bigerr] } {1 1} - test {HVSTRLEN against the small hash} { + test {HSTRLEN against the small hash} { set err {} foreach k [array names smallhash *] { - if {[string length $smallhash($k)] ne [r hvstrlen smallhash $k]} { - set err "[string length $smallhash($k)] != [r hvstrlen smallhash $k]" + if {[string length $smallhash($k)] ne [r hstrlen smallhash $k]} { + set err "[string length $smallhash($k)] != [r hstrlen smallhash $k]" break } } set _ $err } {} - test {HVSTRLEN against the big hash} { + test {HSTRLEN against the big hash} { set err {} foreach k [array names bighash *] { - if {[string length $bighash($k)] ne [r hvstrlen bighash $k]} { - set err "[string length $bighash($k)] != [r hvstrlen bighash $k]" + if {[string length $bighash($k)] ne [r hstrlen bighash $k]} { + set err "[string length $bighash($k)] != [r hstrlen bighash $k]" break } } set _ $err } {} - test {HVSTRLEN against non existing key} { + test {HSTRLEN against non existing field} { set rv {} - lappend rv [r hvstrlen smallhash __123123123__] - lappend rv [r hvstrlen bighash __123123123__] + lappend rv [r hstrlen smallhash __123123123__] + lappend rv [r hstrlen bighash __123123123__] set _ $rv - - } {{} {}} - + } {0 0} test {Hash ziplist regression test for large keys} { r hset hash kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk a