Skip to content

Commit

Permalink
Revert make KEYS to be an exact match if there is no pattern (#964)
Browse files Browse the repository at this point in the history
In #792, the time complexity became ambiguous, fluctuating between
O(1) and O(n), which is a significant difference. And we agree uncertainty
can potentially bring disaster to the business, the right thing to do is
to persuade users to use EXISTS instead of KEYS in this case, to do the
right thing the right way, rather than accommodating this incorrect usage.

This reverts commit d66a06e.
This reverts #792.

Signed-off-by: Binbin <binloveplay1314@qq.com>
  • Loading branch information
enjoy-binbin authored and madolson committed Sep 3, 2024
1 parent be7f15c commit d2d93ea
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 89 deletions.
35 changes: 0 additions & 35 deletions src/db.c
Original file line number Diff line number Diff line change
Expand Up @@ -808,21 +808,6 @@ void randomkeyCommand(client *c) {
decrRefCount(key);
}

/* Returns 1 if the pattern can be an exact match in KEYS context. */
int patternExactMatch(const char *pattern, int length) {
for (int i = 0; i < length; i++) {
if (pattern[i] == '*' || pattern[i] == '?' || pattern[i] == '[') {
/* Wildcard or character class found. Keys can be in anywhere. */
return 0;
} else if (pattern[i] == '\\') {
/* Escaped character. Computing the key name in this case is not
* implemented. We would need a temp buffer. */
return 0;
}
}
return 1;
}

void keysCommand(client *c) {
dictEntry *de;
sds pattern = c->argv[1]->ptr;
Expand All @@ -832,27 +817,7 @@ void keysCommand(client *c) {
allkeys = (pattern[0] == '*' && plen == 1);
if (server.cluster_enabled && !allkeys) {
pslot = patternHashSlot(pattern, plen);
} else if (!server.cluster_enabled) {
pslot = 0;
}

/* Once the pattern can do an exact match, we can convert
* it to a kvstoreDictFind to avoid iterating over all data. */
if (patternExactMatch(pattern, plen) && pslot != -1) {
de = kvstoreDictFind(c->db->keys, pslot, pattern);
if (de) {
robj keyobj;
sds key = dictGetKey(de);
initStaticStringObject(keyobj, key);
if (!keyIsExpired(c->db, &keyobj)) {
addReplyBulkCBuffer(c, key, sdslen(key));
numkeys++;
}
}
setDeferredArrayLen(c, replylen, numkeys);
return;
}

kvstoreDictIterator *kvs_di = NULL;
kvstoreIterator *kvs_it = NULL;
if (pslot != -1) {
Expand Down
55 changes: 1 addition & 54 deletions tests/unit/keyspace.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -40,25 +40,7 @@ start_server {tags {"keyspace"}} {
}
assert_equal [lsort [r keys "{a}*"]] [list "{a}x" "{a}y" "{a}z"]
assert_equal [lsort [r keys "*{b}*"]] [list "{b}a" "{b}b" "{b}c"]
}

test {KEYS with no pattern} {
r debug populate 1000
r set foo bar
r set foo{t} bar
r set foo\\ bar
r del non-exist
assert_equal [r keys foo] {foo}
assert_equal [r keys foo{t}] {foo{t}}
assert_equal [r keys foo\\] {foo\\}
assert_equal [r keys non-exist] {}

# Make sure the return value is consistent with the * one
assert_equal [r keys foo] [r keys *foo]
assert_equal [r keys foo{t}] [r keys *foo{t}]
assert_equal [r keys foo\\] [r keys *foo\\]
assert_equal [r keys non-exist] [r keys *non-exist]
} {} {needs:debug}
}

test {DEL all keys} {
foreach key [r keys *] {r del $key}
Expand Down Expand Up @@ -566,38 +548,3 @@ foreach {type large} [array get largevalue] {
r flushall
} {OK} {singledb:skip}
}

start_cluster 1 0 {tags {"keyspace external:skip cluster"}} {
# SET keys for random slots, for random noise.
set num_keys 0
while {$num_keys < 1000} {
set random_key [randomInt 163840]
r SET $random_key VALUE
incr num_keys 1
}

test {KEYS with hashtag in cluster mode} {
foreach key {"{a}x" "{a}y" "{a}z" "{b}a" "{b}b" "{b}c"} {
r set $key hello
}
assert_equal [lsort [r keys "{a}*"]] [list "{a}x" "{a}y" "{a}z"]
assert_equal [lsort [r keys "*{b}*"]] [list "{b}a" "{b}b" "{b}c"]
}

test {KEYS with no pattern in cluster mode} {
r set foo bar
r set foo{t} bar
r set foo\\ bar
r del non-exist
assert_equal [r keys foo] {foo}
assert_equal [r keys foo{t}] {foo{t}}
assert_equal [r keys foo\\] {foo\\}
assert_equal [r keys non-exist] {}

# Make sure the return value is consistent with the * one
assert_equal [r keys foo] [r keys *foo]
assert_equal [r keys foo{t}] [r keys *foo{t}]
assert_equal [r keys foo\\] [r keys *foo\\]
assert_equal [r keys non-exist] [r keys *non-exist]
}
}

0 comments on commit d2d93ea

Please sign in to comment.