diff options
author | guybe7 <guy.benoish@redislabs.com> | 2021-11-18 10:47:49 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-11-18 10:47:49 +0200 |
commit | af7489886dab9362e4661a3015ac606025710686 (patch) | |
tree | c51a26adb22eb4e21a71c9ff19a5b5f68e2ddbcd | |
parent | 91e77a0cfb5c7e4bc6473ae04353e48ad9e8697b (diff) | |
download | redis-af7489886dab9362e4661a3015ac606025710686.tar.gz |
Obliterate STRALGO! add LCS (which only works on keys) (#9799)
Drop the STRALGO command, now LCS is a command of its own and it only works on keys (not input strings).
The motivation is that STRALGO's syntax was really messed-up...
- assumes all (future) string algorithms will take similar arguments
- mixes command that takes keys and one that doesn't in the same command.
- make it nearly impossible to expose the right key spec in COMMAND INFO (issues cluster clients)
- hard for cluster clients to determine the key names (firstkey, lastkey, etc)
- hard for ACL / flags (is it a read command?)
This is a breaking change.
-rw-r--r-- | src/server.c | 23 | ||||
-rw-r--r-- | src/server.h | 2 | ||||
-rw-r--r-- | src/t_string.c | 89 | ||||
-rw-r--r-- | tests/support/util.tcl | 2 | ||||
-rw-r--r-- | tests/unit/type/string.tcl | 22 |
5 files changed, 41 insertions, 97 deletions
diff --git a/src/server.c b/src/server.c index d587b76c5..a10fdd43c 100644 --- a/src/server.c +++ b/src/server.c @@ -523,20 +523,6 @@ struct redisCommand clientSubcommands[] = { {NULL}, }; -struct redisCommand stralgoSubcommands[] = { - {"lcs",stralgoCommand,-5, - "read-only @string", - {{"read incomplete", /* We can't use "keyword" here because we may give false information. */ - KSPEC_BS_UNKNOWN,{{0}}, - KSPEC_FK_UNKNOWN,{{0}}}}, - lcsGetKeys}, - - {"help",stralgoCommand,2, - "ok-loading ok-stale @string"}, - - {NULL}, -}; - struct redisCommand pubsubSubcommands[] = { {"channels",pubsubCommand,-2, "pub-sub ok-loading ok-stale"}, @@ -2038,9 +2024,12 @@ struct redisCommand redisCommandTable[] = { "sentinel", .subcommands=aclSubcommands}, - {"stralgo",NULL,-2, - "", - .subcommands=stralgoSubcommands}, + {"lcs",lcsCommand,-3, + "read-only @string", + {{"read", + KSPEC_BS_INDEX,.bs.index={1}, + KSPEC_FK_RANGE,.fk.range={1,1,0}}}, + lcsGetKeys}, {"reset",resetCommand,1, "no-script ok-stale ok-loading fast @connection"}, diff --git a/src/server.h b/src/server.h index 1ee073fdb..d3ccdb080 100644 --- a/src/server.h +++ b/src/server.h @@ -3014,7 +3014,7 @@ void xdelCommand(client *c); void xtrimCommand(client *c); void lolwutCommand(client *c); void aclCommand(client *c); -void stralgoCommand(client *c); +void lcsCommand(client *c); void resetCommand(client *c); void failoverCommand(client *c); diff --git a/src/t_string.c b/src/t_string.c index e1917958b..d4334b1eb 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -726,40 +726,33 @@ void strlenCommand(client *c) { addReplyLongLong(c,stringObjectLen(o)); } - -/* STRALGO -- Implement complex algorithms on strings. - * - * STRALGO <algorithm> ... arguments ... */ -void stralgoLCS(client *c); /* This implements the LCS algorithm. */ -void stralgoCommand(client *c) { - char *subcmd = c->argv[1]->ptr; - - if (c->argc == 2 && !strcasecmp(subcmd,"help")) { - const char *help[] = { -"LCS", -" Run the longest common subsequence algorithm.", -NULL - }; - addReplyHelp(c, help); - } else if (!strcasecmp(subcmd,"lcs")) { - stralgoLCS(c); - } else { - addReplySubcommandSyntaxError(c); - return; - } -} - -/* STRALGO <algo> [IDX] [LEN] [MINMATCHLEN <len>] [WITHMATCHLEN] - * STRINGS <string> <string> | KEYS <keya> <keyb> - */ -void stralgoLCS(client *c) { +/* LCS key1 key2 [LEN] [IDX] [MINMATCHLEN <len>] [WITHMATCHLEN] */ +void lcsCommand(client *c) { uint32_t i, j; long long minmatchlen = 0; sds a = NULL, b = NULL; int getlen = 0, getidx = 0, withmatchlen = 0; robj *obja = NULL, *objb = NULL; - for (j = 2; j < (uint32_t)c->argc; j++) { + obja = lookupKeyRead(c->db,c->argv[1]); + objb = lookupKeyRead(c->db,c->argv[2]); + if ((obja && obja->type != OBJ_STRING) || + (objb && objb->type != OBJ_STRING)) + { + addReplyError(c, + "The specified keys must contain string values"); + /* Don't cleanup the objects, we need to do that + * only after calling getDecodedObject(). */ + obja = NULL; + objb = NULL; + goto cleanup; + } + obja = obja ? getDecodedObject(obja) : createStringObject("",0); + objb = objb ? getDecodedObject(objb) : createStringObject("",0); + a = obja->ptr; + b = objb->ptr; + + for (j = 3; j < (uint32_t)c->argc; j++) { char *opt = c->argv[j]->ptr; int moreargs = (c->argc-1) - j; @@ -774,37 +767,6 @@ void stralgoLCS(client *c) { != C_OK) goto cleanup; if (minmatchlen < 0) minmatchlen = 0; j++; - } else if (!strcasecmp(opt,"STRINGS") && moreargs > 1) { - if (a != NULL) { - addReplyError(c,"Either use STRINGS or KEYS"); - goto cleanup; - } - a = c->argv[j+1]->ptr; - b = c->argv[j+2]->ptr; - j += 2; - } else if (!strcasecmp(opt,"KEYS") && moreargs > 1) { - if (a != NULL) { - addReplyError(c,"Either use STRINGS or KEYS"); - goto cleanup; - } - obja = lookupKeyRead(c->db,c->argv[j+1]); - objb = lookupKeyRead(c->db,c->argv[j+2]); - if ((obja && obja->type != OBJ_STRING) || - (objb && objb->type != OBJ_STRING)) - { - addReplyError(c, - "The specified keys must contain string values"); - /* Don't cleanup the objects, we need to do that - * only after calling getDecodedObject(). */ - obja = NULL; - objb = NULL; - goto cleanup; - } - obja = obja ? getDecodedObject(obja) : createStringObject("",0); - objb = objb ? getDecodedObject(objb) : createStringObject("",0); - a = obja->ptr; - b = objb->ptr; - j += 2; } else { addReplyErrorObject(c,shared.syntaxerr); goto cleanup; @@ -812,14 +774,9 @@ void stralgoLCS(client *c) { } /* Complain if the user passed ambiguous parameters. */ - if (a == NULL) { - addReplyError(c,"Please specify two strings: " - "STRINGS or KEYS options are mandatory"); - goto cleanup; - } else if (getlen && getidx) { + if (getlen && getidx) { addReplyError(c, - "If you want both the length and indexes, please " - "just use IDX."); + "If you want both the length and indexes, please just use IDX."); goto cleanup; } diff --git a/tests/support/util.tcl b/tests/support/util.tcl index 4b5643629..3feb1e961 100644 --- a/tests/support/util.tcl +++ b/tests/support/util.tcl @@ -631,7 +631,7 @@ proc errorrstat {cmd r} { proc generate_fuzzy_traffic_on_key {key duration} { # Commands per type, blocking commands removed # TODO: extract these from help.h or elsewhere, and improve to include other types - set string_commands {APPEND BITCOUNT BITFIELD BITOP BITPOS DECR DECRBY GET GETBIT GETRANGE GETSET INCR INCRBY INCRBYFLOAT MGET MSET MSETNX PSETEX SET SETBIT SETEX SETNX SETRANGE STRALGO STRLEN} + set string_commands {APPEND BITCOUNT BITFIELD BITOP BITPOS DECR DECRBY GET GETBIT GETRANGE GETSET INCR INCRBY INCRBYFLOAT MGET MSET MSETNX PSETEX SET SETBIT SETEX SETNX SETRANGE LCS STRLEN} set hash_commands {HDEL HEXISTS HGET HGETALL HINCRBY HINCRBYFLOAT HKEYS HLEN HMGET HMSET HSCAN HSET HSETNX HSTRLEN HVALS HRANDFIELD} set zset_commands {ZADD ZCARD ZCOUNT ZINCRBY ZINTERSTORE ZLEXCOUNT ZPOPMAX ZPOPMIN ZRANGE ZRANGEBYLEX ZRANGEBYSCORE ZRANK ZREM ZREMRANGEBYLEX ZREMRANGEBYRANK ZREMRANGEBYSCORE ZREVRANGE ZREVRANGEBYLEX ZREVRANGEBYSCORE ZREVRANK ZSCAN ZSCORE ZUNIONSTORE ZRANDMEMBER} set list_commands {LINDEX LINSERT LLEN LPOP LPOS LPUSH LPUSHX LRANGE LREM LSET LTRIM RPOP RPOPLPUSH RPUSH RPUSHX} diff --git a/tests/unit/type/string.tcl b/tests/unit/type/string.tcl index 187035743..71b62b337 100644 --- a/tests/unit/type/string.tcl +++ b/tests/unit/type/string.tcl @@ -573,29 +573,27 @@ start_server {tags {"string"}} { set rna2 {ATTAAAGGTTTATACCTTCCCAGGTAACAAACCAACCAACTTTCGATCTCTTGTAGATCTGTTCTCTAAACGAACTTTAAAATCTGTGTGGCTGTCACTCGGCTGCATGCTTAGTGCACTCACGCAGTATAATTAATAACTAATTACTGTCGTTGACAGGACACGAGTAACTCGTCTATCTTCTGCAGGCTGCTTACGGTTTCGTCCGTGTTGCAGCCGATCATCAGCACATCTAGGTTT} set rnalcs {ACCTTCCCAGGTAACAAACCAACCAACTTTCGATCTCTTGTAGATCTGTTCTCTAAACGAACTTTAAAATCTGTGTGGCTGTCACTCGGCTGCATGCTTAGTGCACTCACGCAGTATAATTAATAACTAATTACTGTCGTTGACAGGACACGAGTAACTCGTCTATCTTCTGCAGGCTGCTTACGGTTTCGTCCGTGTTGCAGCCGATCATCAGCACATCTAGGTTT} - test {STRALGO LCS string output with STRINGS option} { - r STRALGO LCS STRINGS $rna1 $rna2 + test {LCS basic} { + r set virus1{t} $rna1 + r set virus2{t} $rna2 + r LCS virus1{t} virus2{t} } $rnalcs - test {STRALGO LCS len} { - r STRALGO LCS LEN STRINGS $rna1 $rna2 - } [string length $rnalcs] - - test {LCS with KEYS option} { + test {LCS len} { r set virus1{t} $rna1 r set virus2{t} $rna2 - r STRALGO LCS KEYS virus1{t} virus2{t} - } $rnalcs + r LCS virus1{t} virus2{t} LEN + } [string length $rnalcs] test {LCS indexes} { - dict get [r STRALGO LCS IDX KEYS virus1{t} virus2{t}] matches + dict get [r LCS virus1{t} virus2{t} IDX] matches } {{{238 238} {239 239}} {{236 236} {238 238}} {{229 230} {236 237}} {{224 224} {235 235}} {{1 222} {13 234}}} test {LCS indexes with match len} { - dict get [r STRALGO LCS IDX KEYS virus1{t} virus2{t} WITHMATCHLEN] matches + dict get [r LCS virus1{t} virus2{t} IDX WITHMATCHLEN] matches } {{{238 238} {239 239} 1} {{236 236} {238 238} 1} {{229 230} {236 237} 2} {{224 224} {235 235} 1} {{1 222} {13 234} 222}} test {LCS indexes with match len and minimum match len} { - dict get [r STRALGO LCS IDX KEYS virus1{t} virus2{t} WITHMATCHLEN MINMATCHLEN 5] matches + dict get [r LCS virus1{t} virus2{t} IDX WITHMATCHLEN MINMATCHLEN 5] matches } {{{1 222} {13 234} 222}} } |