Skip to content
This repository has been archived by the owner on Dec 1, 2024. It is now read-only.

Refactor initial seek #689

Merged
merged 3 commits into from
Nov 4, 2019
Merged

Refactor initial seek #689

merged 3 commits into from
Nov 4, 2019

Conversation

vweevers
Copy link
Member

@vweevers vweevers commented Oct 20, 2019

To make it more similar to the OutOfRange() method used for manual seeks, and maybe merge that code eventually. With #680 in mind. Taking small steps on purpose.

But I found what seems like dead code, see the second commit. I can't think of a case where we would hit this code path. Tried adding more tests too (I will open a abstract-leveldown PR shortly).

@vweevers vweevers added semver-patch Bug fixes that are backward compatible refactor Requires or pertains to refactoring labels Oct 20, 2019
binding.cc Outdated
if (lt_->compare(dbIterator_->key().ToString()) <= 0)
dbIterator_->Prev();
if (dbIterator_->key().compare(*lt_) >= 0)
assert(false);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ralphtheninja @peakji Can you think of a case where we hit this code path?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is dead code.

If we're using start_ correctly, there can be only 3 possible cases for a valid reversed iterator in L584-L599, so the key should never be less than lt on L600:

seek

@vweevers vweevers marked this pull request as ready for review October 21, 2019 21:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
refactor Requires or pertains to refactoring semver-patch Bug fixes that are backward compatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants