feat(keys, scan): Support arbitrary glob patterns #2608
Conversation
|
|
||
| if (!util::StringMatch(suffix_glob, user_key.substr(prefix.size()))) { | ||
| continue; | ||
| } | ||
| keys->emplace_back(user_key); | ||
| cnt++; |
There was a problem hiding this comment.
I'm wondering that, if nothing got matched in this limit, e.g. the limit is 10, and in this 10 keys no key is matched, but the 12th key is matched, will a valid cursor be returned so that users can use it to continue the scan?
Also could we add a test case to ensure that?
There was a problem hiding this comment.
it seems like cnt is only incremented when we actually add a key to the result, so the loop will continue until no more keys match the prefix, or limit keys are inserted into the result vector
There was a problem hiding this comment.
Hmmm I think we'd better refactor the logic, so that the SCAN can be quick, instead of a long-time blocking scan. E.g. we can scan a fixed maximum numbers of keys even if the limit/cnt is not reach. It's fine to return even zero matched key to users.
Or maybe we can confirm the logic in Redis?
There was a problem hiding this comment.
Sorry for missing this comment. This sounds good to me after taking a rough review.
Hmmm I think we'd better refactor the logic, so that the SCAN can be quick, instead of a long-time blocking scan. E.g. we can scan a fixed maximum numbers of keys even if the limit/cnt is not reach. It's fine to return even zero matched key to users.
Or maybe we can confirm the logic in Redis?
Yes, Redis will set a max iteration number(10*count) to avoid blocking too long.
|
just to say it, it would be nice to have the typos script run as a precommit hook or lint: it's a bit unideal to get the workflow approved by a reviewer just to find out you've introduced a typo in your commit somewhere |
|
Hi @nathanlo-hrt Seems the Go test is broken, would you mind fixing it to make it mergeable? |
|
Kvrocks currently only supports prefix matching (glob patterns like
ab*).This change implements arbitrary glob patterns for
KEYSandSCAN MATCH [pattern].