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

Fix iterator moving backwards and created unit test for the JniDBIterator #52

Merged
merged 1 commit into from
Jul 30, 2014

Conversation

Jaap-Jan
Copy link

Iterator follows the following principles

  1. Always points to a correct item, unless empty
  2. Almost always points to the item to be returned next
  3. Except when the last item has been returned. atEnd is set to true at that time and the functions work correctly when the last item has been returned (and the iterator would become invalidated)

@bramklg
Copy link

bramklg commented Mar 10, 2014

We are having the same issues with the existing iterator. Would be nice if this could be fixed / pulled in the leveldbjni repository.

davsclaus added a commit that referenced this pull request Jul 30, 2014
Fix iterator moving backwards and created unit test for the JniDBIterator
@davsclaus davsclaus merged commit bac9134 into fusesource:master Jul 30, 2014
@davsclaus
Copy link
Member

Thanks for this, great work.

@jerrydlamme
Copy link
Contributor

@Jaap-Jan u
Did you run through the DBTest.java ?
Seems a little buggy. Pull request #60 is also complaining about it.

@davsclaus
@chirino
First this commit re-introduce the similar SIGSEGV in issue #40 and test erros happen.
And iterator behavior is re-defined when it moves backward after calling seekToLast().
There are test assertion failures in DBTest.java due to this.
Build fails.

Either we have this commit backed out or fix the issue and update the DBTest.java for new iterator behavior??

@davsclaus
Copy link
Member

@jerrydlamme you are welcome to do a PR with a fix.

@jerrydlamme
Copy link
Contributor

@davsclaus
I actually would prefer file a PR to back out this commit first.
Since this commit itself is not a correct fix. The change of seekToLast() is not correct.

leveldb API says:
// Position at the last key in the source. The iterator is
// Valid() after this call iff the source is not empty.
virtual void SeekToLast() = 0;

This commit put the iterator at a fake position after the last key, which is not correct. The moving back
is thus changed, and assertion failure happens.
API behavior cannot be changed this way.
Plus every commit should at least make build clear.

Can you support this revert ?

Thanks

@davsclaus
Copy link
Member

yeah sure a PR is welcome

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.

None yet

4 participants