feat: implement _seek

[MeirionHughes]

No description provided.

[vweevers]

Awesome!

[vweevers]

abstract-leveldown has tests for this, which can (should) be enabled here:

memdown/test.js

Line 18 in d14d582

seek: false

[MeirionHughes]

eek, yeah activating that broke it. :/

[MeirionHughes]

... that was more painful that I was hoping for.

anyway; its not quite like leveldb as I think that puts the iterator on either side of the table and its marked as in invalid. For memdown I've had to make this._tree undefined and a test on _next to signify the iterator is invalid for a given range and target. Tests should (hopefully) be passing now at least.

[vweevers]

its not quite like leveldb as I think that puts the iterator on either side of the table and its marked as in invalid. For memdown I've had to make this._tree undefined and a test on _next to signify the iterator is invalid for a given range and target. Tests should (hopefully) be passing now at least.

What happens when you do two subsequent seeks to different targets (and the second target is in range)? I don't remember if that's valid in leveldown or not.

[vweevers]

cc @peakji, our seek expert

[MeirionHughes]

in principle a new seek should be fine (in memdown) as it goes back to the rgb tree to start fresh. I can add a test tomorrow if there isn't one already.

[MeirionHughes]

also a bit concerned that the seek tests from abstract level iterator don't seem to fail for my code, which no longer converts strings to buffers (if asBuffer or keysAsBuffer is set) before comparing keys. either memdown puts all keys as strings, or the tests are not covering the case where you put buffers keys, but seek a target with a string.

[vweevers]

or the tests are not covering the case where you put buffers as keys, but seek a target with a string

yes, that's the case

[peakji]

What happens when you do two subsequent seeks to different targets (and the second target is in range)? I don't remember if that's valid in leveldown or not.

It is fine to seek() multiple times without doing next() in leveldown, the underlying dbIterator is valid as long as the source contains an entry that comes at or past target.

[vweevers]

Re: buffer vs string targets, keyAsBuffer is not relevant there, because that option only pertains to output (whether you want to yield buffer or string keys). Converting the target to a string won't work if you've stored Buffer keys. And vice versa.

So assuming that we're gonna go with the IDB-style comparator (sort by type, which means buffers are never equal to strings, unlike in leveldown), then I think the best course of action is to update the abstract-leveldown buffer seek test to operate on a buffer data set. In other words, the type of the target must match the type of stored keys.

[vweevers]

@peakji @ralphtheninja Seeing as both @MeirionHughes and I wrote code here, could you review?

[MeirionHughes]

need to check can seek immediately after creating iterator its fine. I've squashed the commits.

[MeirionHughes]

Just waiting on another review before this be merged?

[vweevers]

Just waiting on another review before this be merged?

I was hoping for it but it's been long enough and we've got 100% coverage. Will release tomorrow

[MeirionHughes]

cool thanks. 👍

[vweevers]

4.1.0