-
Notifications
You must be signed in to change notification settings - Fork 178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added support for Seek on iterator and IteratorWithRange on storage #85
Conversation
iterator.go
Outdated
@@ -9,6 +9,7 @@ type Iterator interface { | |||
Key() string | |||
Value() (interface{}, error) | |||
Release() | |||
Seek(key []byte) bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
goka.Iterator
has keys as strings. Does Seek()
make sense when based on strings?
Hi guys, any update on this ? |
Actually yes, there was this question above: The key passed to As far as I remember, |
Sure ;)
|
;) What I meant is that the whole interface of
Similarly, creating the iterator would ideally be done with strings:
Is there any issue with that? |
;) I see... In fact I added like that so that it's complient with the interface of the leveldb library. |
I guessed so. The Still, since we work with string keys at the goka package level, I think it would make sense to keep string keys there. Or change all keys to byte slices, but that would break way too many things. |
Change iterator Seek method to take string and not []byte
ok get it.. I update the code |
Excellent! And this one should take strings too: Besides these |
ok let me fix that
…On Mon, Feb 19, 2018 at 3:54 PM, Diogo Behrens ***@***.***> wrote:
Excellent! And this one should take strings too: func (v *View)
IteratorWithRange(start, limit []byte) (Iterator, error) {
Besides these []byte thing, the PR looks pretty good to me. Thanks for
contributing.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#85 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAUSdQJt2ylXwJRtUc0VDiFKlcoRaUWoks5tWYsNgaJpZM4R5epp>
.
|
something like that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
As discuss in issue #82 Here is the pull request.
I write light test on memory.. I'm looking forward for your code review.