Skip to content
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

Get range #99

Closed
wants to merge 7 commits into from
Closed

Get range #99

wants to merge 7 commits into from

Conversation

kesla
Copy link
Contributor

@kesla kesla commented May 14, 2014

So, this is pretty cool.

I've implemented the getRange-method (#98) without chaning any C++!

I'm building this upon the PR using abstract-leveldown (#50), since the arguments checking then is done in javascript-land and therefore I can implement this in pure js-land!

@juliangruber
Copy link
Member

sweet, lgtm!

@heapwolf
Copy link

nice!

@rvagg
Copy link
Member

rvagg commented May 15, 2014

without changing any C++!

which begs the question: does it belong in LevelDOWN at all then?

discuss

@juliangruber
Copy link
Member

it belongs in leveldown when this eventually gets part of abstract leveldown. if anything, the buffering iterator should make it

@rvagg
Copy link
Member

rvagg commented May 15, 2014

Buffering iterator makes perfect sense, the range get though isn't so clear-cut since it's something that can be done with an addon. I certainly don't think it's a clear candidate for abstractleveldown either as it's far more than a primitive. It's just something that a data store can expose if it makes sense.

@kesla
Copy link
Contributor Author

kesla commented May 16, 2014

I think of the .binding property in this PR as not part of the public leveldown-api (perhaps is should be _binding rather than binding) and without using that a getRange-addon can't be written.

If we implemented the buffering iterator without regards to compatibility with the existing leveldown-compatible backends, we could have a public next method that returns an array of data, and then implement getRange in userland using that.

@heapwolf
Copy link

FWIW, this is possibly one of the most requested features from people I talk to.

@kesla
Copy link
Contributor Author

kesla commented May 17, 2014

I've updated https://github.com/kesla/level-get-range to work with #91 - so from that point of view this PR isn't really needed. I'm calling the iterator.binding directly when available

@kesla
Copy link
Contributor Author

kesla commented May 17, 2014

... and here's streams that outputs arrays (the buffered arrays) of data https://github.com/kesla/level-buffered-streams - so I guess that it's feasible to fullfill some of the use cases in userland.

@rvagg rvagg closed this Aug 26, 2014
@kesla kesla deleted the get-range branch August 26, 2014 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants