From: antirez Date: Wed, 17 Mar 2010 16:14:07 +0000 (+0100) Subject: Merged Pietern patch for VM key args helper function. Fixed an obvious bug in the... X-Git-Url: https://git.saurik.com/redis.git/commitdiff_plain/4583c4f0ea0ef3cdb3cd1b0ede77e5b95be18327?hp=76583ea4555ea199422f56bcafc6be76d9caac7f Merged Pietern patch for VM key args helper function. Fixed an obvious bug in the redis-cli passwd auth stuff --- diff --git a/Makefile b/Makefile index b3e3bf65..df840313 100644 --- a/Makefile +++ b/Makefile @@ -67,7 +67,7 @@ redis-check-dump: $(CHECKDUMPOBJ) $(CC) -c $(CFLAGS) $(DEBUG) $(COMPILE_TIME) $< clean: - rm -rf $(PRGNAME) $(BENCHPRGNAME) $(CLIPRGNAME) *.o *.gcda *.gcno *.gcov + rm -rf $(PRGNAME) $(BENCHPRGNAME) $(CLIPRGNAME) $(CHECKDUMPPRGNAME) *.o *.gcda *.gcno *.gcov dep: $(CC) -MM *.c diff --git a/dict.c b/dict.c index 725c9a51..23f7933b 100644 --- a/dict.c +++ b/dict.c @@ -232,7 +232,7 @@ int dictAdd(dict *ht, void *key, void *val) * operation. */ int dictReplace(dict *ht, void *key, void *val) { - dictEntry *entry; + dictEntry *entry, auxentry; /* Try to add the element. If the key * does not exists dictAdd will suceed. */ @@ -241,8 +241,14 @@ int dictReplace(dict *ht, void *key, void *val) /* It already exists, get the entry */ entry = dictFind(ht, key); /* Free the old value and set the new one */ - dictFreeEntryVal(ht, entry); + /* Set the new value and free the old one. Note that it is important + * to do that in this order, as the value may just be exactly the same + * as the previous one. In this context, think to reference counting, + * you want to increment (set), and then decrement (free), and not the + * reverse. */ + auxentry = *entry; dictSetHashVal(ht, entry, val); + dictFreeEntryVal(ht, &auxentry); return 0; } diff --git a/redis-cli.c b/redis-cli.c index 03755efa..5c1a2a1f 100644 --- a/redis-cli.c +++ b/redis-cli.c @@ -52,6 +52,7 @@ static struct config { long repeat; int dbnum; int interactive; + char *authpw; } config; struct redisCommand { @@ -61,6 +62,7 @@ struct redisCommand { }; static struct redisCommand cmdTable[] = { + {"auth",2,REDIS_CMD_INLINE}, {"get",2,REDIS_CMD_INLINE}, {"set",3,REDIS_CMD_BULK}, {"setnx",3,REDIS_CMD_BULK}, @@ -153,6 +155,7 @@ static struct redisCommand cmdTable[] = { {"hkeys",2,REDIS_CMD_INLINE}, {"hvals",2,REDIS_CMD_INLINE}, {"hgetall",2,REDIS_CMD_INLINE}, + {"hexists",3,REDIS_CMD_BULK}, {NULL,0,0} }; @@ -304,6 +307,27 @@ static int selectDb(int fd) { return 0; } +static int cliLogin(int fd) +{ + int retval = 1; + sds cmd; + char type; + if (config.authpw[0] != '\0') { + cmd = sdsempty(); + cmd = sdscatprintf(cmd,"AUTH %s\r\n",config.authpw); + anetWrite(fd,cmd,sdslen(cmd)); + anetRead(fd,&type,1); + if (type == '+') + retval = 0; + int ret2 = cliReadSingleLineReply(fd,1); + if (ret2) + close(fd); + } else { + retval = 0; + } + return retval; +} + static int cliSendCommand(int argc, char **argv) { struct redisCommand *rc = lookupCommand(argv[0]); int fd, j, retval = 0; @@ -330,6 +354,13 @@ static int cliSendCommand(int argc, char **argv) { return 1; } + /* Login if necessary */ + retval = cliLogin(fd); + if (retval) { + fprintf(stderr,"Authentication failed\n"); + return 1; + } + while(config.repeat--) { /* Build the command to send */ cmd = sdsempty(); @@ -397,6 +428,9 @@ static int parseOptions(int argc, char **argv) { } else if (!strcmp(argv[i],"-n") && !lastarg) { config.dbnum = atoi(argv[i+1]); i++; + } else if (!strcmp(argv[i],"-a") && !lastarg) { + config.authpw = argv[i+1]; + i++; } else if (!strcmp(argv[i],"-i")) { config.interactive = 1; } else { @@ -424,8 +458,8 @@ static sds readArgFromStdin(void) { } static void usage() { - fprintf(stderr, "usage: redis-cli [-h host] [-p port] [-r repeat_times] [-n db_num] [-i] cmd arg1 arg2 arg3 ... argN\n"); - fprintf(stderr, "usage: echo \"argN\" | redis-cli [-h host] [-p port] [-r repeat_times] [-n db_num] cmd arg1 arg2 ... arg(N-1)\n"); + fprintf(stderr, "usage: redis-cli [-h host] [-p port] [-a authpw] [-r repeat_times] [-n db_num] [-i] cmd arg1 arg2 arg3 ... argN\n"); + fprintf(stderr, "usage: echo \"argN\" | redis-cli [-h host] [-a authpw] [-p port] [-r repeat_times] [-n db_num] cmd arg1 arg2 ... arg(N-1)\n"); fprintf(stderr, "\nIf a pipe from standard input is detected this data is used as last argument.\n\n"); fprintf(stderr, "example: cat /etc/passwd | redis-cli set my_passwd\n"); fprintf(stderr, "example: redis-cli get my_passwd\n"); diff --git a/redis.c b/redis.c index 5b9e73c3..fe84898f 100644 --- a/redis.c +++ b/redis.c @@ -694,6 +694,7 @@ static void zinterCommand(redisClient *c); static void hkeysCommand(redisClient *c); static void hvalsCommand(redisClient *c); static void hgetallCommand(redisClient *c); +static void hexistsCommand(redisClient *c); /*================================= Globals ================================= */ @@ -759,6 +760,7 @@ static struct redisCommand cmdTable[] = { {"hkeys",hkeysCommand,2,REDIS_CMD_INLINE,NULL,1,1,1}, {"hvals",hvalsCommand,2,REDIS_CMD_INLINE,NULL,1,1,1}, {"hgetall",hgetallCommand,2,REDIS_CMD_INLINE,NULL,1,1,1}, + {"hexists",hexistsCommand,3,REDIS_CMD_BULK,NULL,1,1,1}, {"incrby",incrbyCommand,3,REDIS_CMD_INLINE|REDIS_CMD_DENYOOM,NULL,1,1,1}, {"decrby",decrbyCommand,3,REDIS_CMD_INLINE|REDIS_CMD_DENYOOM,NULL,1,1,1}, {"getset",getsetCommand,3,REDIS_CMD_BULK|REDIS_CMD_DENYOOM,NULL,1,1,1}, @@ -5392,8 +5394,26 @@ static int qsortCompareZsetopsrcByCardinality(const void *s1, const void *s2) { return size1 - size2; } +#define REDIS_AGGR_SUM 1 +#define REDIS_AGGR_MIN 2 +#define REDIS_AGGR_MAX 3 + +inline static void zunionInterAggregate(double *target, double val, int aggregate) { + if (aggregate == REDIS_AGGR_SUM) { + *target = *target + val; + } else if (aggregate == REDIS_AGGR_MIN) { + *target = val < *target ? val : *target; + } else if (aggregate == REDIS_AGGR_MAX) { + *target = val > *target ? val : *target; + } else { + /* safety net */ + redisAssert(0 != 0); + } +} + static void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { int i, j, zsetnum; + int aggregate = REDIS_AGGR_SUM; zsetopsrc *src; robj *dstobj; zset *dstzset; @@ -5434,19 +5454,28 @@ static void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { /* parse optional extra arguments */ if (j < c->argc) { - int remaining = c->argc-j; + int remaining = c->argc - j; while (remaining) { - if (!strcasecmp(c->argv[j]->ptr,"weights")) { + if (remaining >= (zsetnum + 1) && !strcasecmp(c->argv[j]->ptr,"weights")) { j++; remaining--; - if (remaining < zsetnum) { - zfree(src); - addReplySds(c,sdsnew("-ERR not enough weights for ZUNION/ZINTER\r\n")); - return; - } for (i = 0; i < zsetnum; i++, j++, remaining--) { src[i].weight = strtod(c->argv[j]->ptr, NULL); } + } else if (remaining >= 2 && !strcasecmp(c->argv[j]->ptr,"aggregate")) { + j++; remaining--; + if (!strcasecmp(c->argv[j]->ptr,"sum")) { + aggregate = REDIS_AGGR_SUM; + } else if (!strcasecmp(c->argv[j]->ptr,"min")) { + aggregate = REDIS_AGGR_MIN; + } else if (!strcasecmp(c->argv[j]->ptr,"max")) { + aggregate = REDIS_AGGR_MAX; + } else { + zfree(src); + addReply(c,shared.syntaxerr); + return; + } + j++; remaining--; } else { zfree(src); addReply(c,shared.syntaxerr); @@ -5455,27 +5484,28 @@ static void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { } } + /* sort sets from the smallest to largest, this will improve our + * algorithm's performance */ + qsort(src,zsetnum,sizeof(zsetopsrc), qsortCompareZsetopsrcByCardinality); + dstobj = createZsetObject(); dstzset = dstobj->ptr; if (op == REDIS_OP_INTER) { - /* sort sets from the smallest to largest, this will improve our - * algorithm's performance */ - qsort(src,zsetnum,sizeof(zsetopsrc), qsortCompareZsetopsrcByCardinality); - /* skip going over all entries if the smallest zset is NULL or empty */ if (src[0].dict && dictSize(src[0].dict) > 0) { /* precondition: as src[0].dict is non-empty and the zsets are ordered * from small to large, all src[i > 0].dict are non-empty too */ di = dictGetIterator(src[0].dict); while((de = dictNext(di)) != NULL) { - double *score = zmalloc(sizeof(double)); - *score = 0.0; + double *score = zmalloc(sizeof(double)), value; + *score = src[0].weight * (*(double*)dictGetEntryVal(de)); - for (j = 0; j < zsetnum; j++) { - dictEntry *other = (j == 0) ? de : dictFind(src[j].dict,dictGetEntryKey(de)); + for (j = 1; j < zsetnum; j++) { + dictEntry *other = dictFind(src[j].dict,dictGetEntryKey(de)); if (other) { - *score = *score + src[j].weight * (*(double*)dictGetEntryVal(other)); + value = src[j].weight * (*(double*)dictGetEntryVal(other)); + zunionInterAggregate(score, value, aggregate); } else { break; } @@ -5503,14 +5533,16 @@ static void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { /* skip key when already processed */ if (dictFind(dstzset->dict,dictGetEntryKey(de)) != NULL) continue; - double *score = zmalloc(sizeof(double)); - *score = 0.0; - for (j = 0; j < zsetnum; j++) { - if (!src[j].dict) continue; + double *score = zmalloc(sizeof(double)), value; + *score = src[i].weight * (*(double*)dictGetEntryVal(de)); - dictEntry *other = (i == j) ? de : dictFind(src[j].dict,dictGetEntryKey(de)); + /* because the zsets are sorted by size, its only possible + * for sets at larger indices to hold this entry */ + for (j = (i+1); j < zsetnum; j++) { + dictEntry *other = dictFind(src[j].dict,dictGetEntryKey(de)); if (other) { - *score = *score + src[j].weight * (*(double*)dictGetEntryVal(other)); + value = src[j].weight * (*(double*)dictGetEntryVal(other)); + zunionInterAggregate(score, value, aggregate); } } @@ -5854,7 +5886,7 @@ static void hsetCommand(redisClient *c) { tryObjectEncoding(c->argv[2]); /* note that c->argv[3] is already encoded, as the latest arg * of a bulk command is always integer encoded if possible. */ - if (dictAdd(o->ptr,c->argv[2],c->argv[3]) == DICT_OK) { + if (dictReplace(o->ptr,c->argv[2],c->argv[3])) { incrRefCount(c->argv[2]); } else { update = 1; @@ -6000,6 +6032,26 @@ static void hgetallCommand(redisClient *c) { genericHgetallCommand(c,REDIS_GETALL_KEYS|REDIS_GETALL_VALS); } +static void hexistsCommand(redisClient *c) { + robj *o; + int exists = 0; + + if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.czero)) == NULL || + checkType(c,o,REDIS_HASH)) return; + + if (o->encoding == REDIS_ENCODING_ZIPMAP) { + robj *field; + unsigned char *zm = o->ptr; + + field = getDecodedObject(c->argv[2]); + exists = zipmapExists(zm,field->ptr,sdslen(field->ptr)); + decrRefCount(field); + } else { + exists = dictFind(o->ptr,c->argv[2]) != NULL; + } + addReply(c,exists ? shared.cone : shared.czero); +} + static void convertToRealHash(robj *o) { unsigned char *key, *val, *p, *zm = o->ptr; unsigned int klen, vlen; diff --git a/redis.tcl b/redis.tcl index ac9f8601..ef210cbd 100644 --- a/redis.tcl +++ b/redis.tcl @@ -20,7 +20,7 @@ array set ::redis::multibulkarg {} # Flag commands requiring last argument as a bulk write operation foreach redis_bulk_cmd { - set setnx rpush lpush lset lrem sadd srem sismember echo getset smove zadd zrem zscore zincrby append zrank zrevrank hget hdel + set setnx rpush lpush lset lrem sadd srem sismember echo getset smove zadd zrem zscore zincrby append zrank zrevrank hget hdel hexists } { set ::redis::bulkarg($redis_bulk_cmd) {} } diff --git a/test-redis.tcl b/test-redis.tcl index 66aa0b30..d7a4d7fd 100644 --- a/test-redis.tcl +++ b/test-redis.tcl @@ -1491,6 +1491,14 @@ proc main {server port} { list [$r zunion zsetc 2 zseta zsetb weights 2 3] [$r zrange zsetc 0 -1 withscores] } {4 {a 2 b 7 d 9 c 12}} + test {ZUNION with AGGREGATE MIN} { + list [$r zunion zsetc 2 zseta zsetb aggregate min] [$r zrange zsetc 0 -1 withscores] + } {4 {a 1 b 1 c 2 d 3}} + + test {ZUNION with AGGREGATE MAX} { + list [$r zunion zsetc 2 zseta zsetb aggregate max] [$r zrange zsetc 0 -1 withscores] + } {4 {a 1 b 2 c 3 d 3}} + test {ZINTER basics} { list [$r zinter zsetc 2 zseta zsetb] [$r zrange zsetc 0 -1 withscores] } {2 {b 3 c 5}} @@ -1499,6 +1507,14 @@ proc main {server port} { list [$r zinter zsetc 2 zseta zsetb weights 2 3] [$r zrange zsetc 0 -1 withscores] } {2 {b 7 c 12}} + test {ZINTER with AGGREGATE MIN} { + list [$r zinter zsetc 2 zseta zsetb aggregate min] [$r zrange zsetc 0 -1 withscores] + } {2 {b 1 c 2}} + + test {ZINTER with AGGREGATE MAX} { + list [$r zinter zsetc 2 zseta zsetb aggregate max] [$r zrange zsetc 0 -1 withscores] + } {2 {b 2 c 3}} + test {SORT against sorted sets} { $r del zset $r zadd zset 1 a @@ -1580,6 +1596,98 @@ proc main {server port} { set _ $err } {} + test {HSET in update and insert mode} { + set rv {} + set k [lindex [array names smallhash *] 0] + lappend rv [$r hset smallhash $k newval1] + set smallhash($k) newval1 + lappend rv [$r hget smallhash $k] + lappend rv [$r hset smallhash __foobar123__ newval] + set k [lindex [array names bighash *] 0] + lappend rv [$r hset bighash $k newval2] + set bighash($k) newval2 + lappend rv [$r hget bighash $k] + lappend rv [$r hset bighash __foobar123__ newval] + lappend rv [$r hdel smallhash __foobar123__] + lappend rv [$r hdel bighash __foobar123__] + set _ $rv + } {0 newval1 1 0 newval2 1 1 1} + + test {HGET against non existing key} { + set rv {} + lappend rv [$r hget smallhash __123123123__] + lappend rv [$r hget bighash __123123123__] + set _ $rv + } {{} {}} + + test {HKEYS - small hash} { + lsort [$r hkeys smallhash] + } [lsort [array names smallhash *]] + + test {HKEYS - big hash} { + lsort [$r hkeys bighash] + } [lsort [array names bighash *]] + + test {HVALS - small hash} { + set vals {} + foreach {k v} [array get smallhash] { + lappend vals $v + } + set _ [lsort $vals] + } [lsort [$r hvals smallhash]] + + test {HVALS - big hash} { + set vals {} + foreach {k v} [array get bighash] { + lappend vals $v + } + set _ [lsort $vals] + } [lsort [$r hvals bighash]] + + test {HGETALL - small hash} { + lsort [$r hgetall smallhash] + } [lsort [array get smallhash]] + + test {HGETALL - big hash} { + lsort [$r hgetall bighash] + } [lsort [array get bighash]] + + test {HDEL and return value} { + set rv {} + lappend rv [$r hdel smallhash nokey] + lappend rv [$r hdel bighash nokey] + set k [lindex [array names smallhash *] 0] + lappend rv [$r hdel smallhash $k] + lappend rv [$r hdel smallhash $k] + lappend rv [$r hget smallhash $k] + unset smallhash($k) + set k [lindex [array names bighash *] 0] + lappend rv [$r hdel bighash $k] + lappend rv [$r hdel bighash $k] + lappend rv [$r hget bighash $k] + unset bighash($k) + set _ $rv + } {0 0 1 0 {} 1 0 {}} + + test {HEXISTS} { + set rv {} + set k [lindex [array names smallhash *] 0] + lappend rv [$r hexists smallhash $k] + lappend rv [$r hexists smallhash nokey] + set k [lindex [array names bighash *] 0] + lappend rv [$r hexists bighash $k] + lappend rv [$r hexists bighash nokey] + } {1 0 1 0} + + test {Is a zipmap encoded Hash promoted on big payload?} { + $r hset smallhash foo [string repeat a 1024] + $r debug object smallhash + } {*hashtable*} + + # TODO: + # Randomized test, small and big + # .rdb / AOF consistency test should include hashes + test {EXPIRE - don't set timeouts multiple times} { $r set x foobar set v1 [$r expire x 5]