wbolster / plyvel

Plyvel, a fast and feature-rich Python interface to LevelDB
https://plyvel.readthedocs.io/
Other
530 stars 75 forks source link

Fix reverse iterator #28

Open numion opened 10 years ago

numion commented 10 years ago

Mirroring the implementation of real_next() in real_prev() resolves an issue with reverse iteration.

wbolster commented 10 years ago

Hi, sorry for the late reply.

I'm not sure what the exact problem is. Can you provide a minimal example that illustrates the problem you describe?

numion commented 10 years ago

here is an example:

>>> import plyvel
>>> db = plyvel.DB('/tmp/testdb', create_if_missing=True)
>>> db.put(b'1', b'')
>>> db.put(b'2', b'')

forward iteration works as expected

>>> list(db.iterator(start=b'1', stop=b'2', include_value=False))
['1']

reverse iteration should give the same result

>>> list(db.iterator(start=b'1', stop=b'2', include_value=False, reverse=True))
[]

same here

>>> list(db.iterator(start=b'1', stop=b'1', include_stop=True, include_value=False))
['1']
>>> list(db.iterator(start=b'1', stop=b'1', include_stop=True, include_value=False, reverse=True))
[]
ichernev commented 9 years ago

I have a very similar problem, here's the gist:

import plyvel

db = plyvel.DB('/tmp/testdb/', create_if_missing=True)
db.put(b'key', b'value')
db.put(b'key2', b'value')

print list(db.iterator(start=b'key', include_start=True,
                       stop=b'key1', include_stop=True,
                       reverse=True))
// should print [('key', 'value')], instead prints []

The fix for this particular problem is fixed by:

diff --git a/plyvel/_plyvel.pyx b/plyvel/_plyvel.pyx
index 984a12a..f97cd71 100644
--- a/plyvel/_plyvel.pyx
+++ b/plyvel/_plyvel.pyx
@@ -910,7 +910,8 @@ cdef class Iterator(BaseIterator):
             # After all the stepping back, we might even have ended up
             # *before* the start key. In this case the iterator does not
             # yield any items.
-            if self.start is not None and self.comparator.Compare(self.start_slice, self._iter.key()) >= 0:
+            n = 1 if self.include_start else 0
+            if self.start is not None and self.comparator.Compare(self.start_slice, self._iter.key()) >= n:
                 raise StopIteration

             raise_for_status(self._iter.status())

But making sure going forward and going backward do the same thing makes more sense to me.