Add key prefix matching to KEYS command #237

Merged
prologic merged 5 commits from biozz/bitcask:keys-prefix-matching into master 8 months ago
biozz commented 8 months ago

Related to #234 and !236.

This is the implementation that was requested in the original issue. I updated KEYS command to be redis-valid and implemented prefix search. There is also a rather interesting test, I could you use some feedback here.

I noticed that it might not be possible to reduce the complexity of the KEYS command. Because even if you use Scan, you will have to store the counter of all found keys before you do WriteBulk of the actual keys.

@prologic here is what you probably had in mind:

s.db.Scan([]byte(prefix), func(key []byte) error {
	conn.WriteBulk(key)
	return nil
})

But there is no way to call conn.WriteArray(n) with the number of keys until you iterate through all of them, hence the second loop over found keys.

Related to #234 and !236. This is the implementation that was requested in the original issue. I updated KEYS command to be redis-valid and implemented prefix search. There is also a rather interesting test, I could you use some feedback here. I noticed that it might not be possible to reduce the complexity of the KEYS command. Because even if you use Scan, you will have to store the counter of all found keys before you do WriteBulk of the actual keys. @prologic here is what you probably had in mind: ``` s.db.Scan([]byte(prefix), func(key []byte) error { conn.WriteBulk(key) return nil }) ``` But there is no way to call `conn.WriteArray(n)` with the number of keys until you iterate through all of them, hence the second loop over found keys.
biozz added 2 commits 8 months ago
2d3634b422 Add key prefix matching to KEYS command
fc7b0b1dc5 Update authors
Owner

Yeah you're right! YOu'd have to accumulate the metched keys sadly.

Yeah you're right! YOu'd have to accumulate the metched keys sadly.
Owner

I think what you've done here in this PR is great though and it will perform much better with a database with a larger keyspace. Iteresting through hundreds or thousands of keys will be slow, so reducing that space with ascan is goo.d

I _think_ what you've done here in this PR is great though and it **will** perform much better with a database with a larger keyspace. Iteresting through hundreds or thousands of keys will be slow, so reducing that space with ascan is goo.d
prologic added 1 commit 8 months ago
826dd0ab6f Merge branch 'master' into keys-prefix-matching
biozz added 1 commit 8 months ago
2dbf81699d Make use of the original gitignore, rename test
biozz added 1 commit 8 months ago
48d48aea0e Revert deleted whitespace
Poster

@prologic @taigrr would you be so kind and give this PR a round of code review?

@prologic @taigrr would you be so kind and give this PR a round of code review?
Owner

@biozz Oh yes so sorry! Work has been "flat out" 🤣

@biozz Oh yes so sorry! Work has been "flat out" 🤣
Owner

I actually looked at the code briefly before. This LGTM 👌

I _actually_ looked at the code briefly before. This LGTM 👌
prologic approved these changes 8 months ago
prologic left a comment
Owner

LGTM

LGTM
prologic merged commit 21a824e13e into master 8 months ago
prologic deleted branch keys-prefix-matching 8 months ago

Reviewers

prologic approved these changes 8 months ago
continuous-integration/drone/pr Build is passing
The pull request has been merged as 21a824e13e.
Sign in to join this conversation.
Loading…
There is no content yet.