From 8307be324f7aec69657a1c1019fb7f65fe39559e Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Mon, 6 Feb 2017 21:49:54 +0000 Subject: [PATCH 01/11] Basic implementation of raw_iterator An alternative iterator API that directly maps to RocksDB's iterator API with all the flexibility and unsafety it brings --- src/db.rs | 175 +++++++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 2 +- 2 files changed, 176 insertions(+), 1 deletion(-) diff --git a/src/db.rs b/src/db.rs index 06fdef6..678325c 100644 --- a/src/db.rs +++ b/src/db.rs @@ -100,6 +100,39 @@ pub struct Snapshot<'a> { inner: *const ffi::rocksdb_snapshot_t, } +/// An iterator over a database or column family, with specifiable +/// ranges and direction. +/// +/// ``` +/// use rocksdb::{DB, Direction, IteratorMode}; +/// +/// let mut db = DB::open_default("path/for/rocksdb/storage2").unwrap(); +/// let mut iter = db.iterator(IteratorMode::Start); // Always iterates forward +/// for (key, value) in iter { +/// println!("Saw {:?} {:?}", key, value); +/// } +/// iter = db.iterator(IteratorMode::End); // Always iterates backward +/// for (key, value) in iter { +/// println!("Saw {:?} {:?}", key, value); +/// } +/// iter = db.iterator(IteratorMode::From(b"my key", Direction::Forward)); // From a key in Direction::{forward,reverse} +/// for (key, value) in iter { +/// println!("Saw {:?} {:?}", key, value); +/// } +/// +/// // You can seek with an existing Iterator instance, too +/// iter = db.iterator(IteratorMode::Start); +/// iter.set_mode(IteratorMode::From(b"another key", Direction::Reverse)); +/// for (key, value) in iter { +/// println!("Saw {:?} {:?}", key, value); +/// } +/// ``` +pub struct DBRawIterator { + inner: *mut ffi::rocksdb_iterator_t, + just_seeked: bool, +} + + /// An iterator over a database or column family, with specifiable /// ranges and direction. /// @@ -178,6 +211,122 @@ pub enum IteratorMode<'a> { From(&'a [u8], Direction), } +impl DBRawIterator { + fn new(db: &DB, readopts: &ReadOptions) -> DBRawIterator { + unsafe { + let iterator = ffi::rocksdb_create_iterator(db.inner, readopts.inner); + + let mut rv = DBRawIterator { + inner: iterator, + just_seeked: false, + }; + rv.seek_to_first(); + rv + } + } + + fn new_cf(db: &DB, + cf_handle: ColumnFamily, + readopts: &ReadOptions) + -> Result { + unsafe { + let iterator = ffi::rocksdb_create_iterator_cf(db.inner, readopts.inner, cf_handle.inner); + + let mut rv = DBRawIterator { + inner: iterator, + just_seeked: false, + }; + rv.seek_to_first(); + Ok(rv) + } + } + + pub fn valid(&self) -> bool { + unsafe { ffi::rocksdb_iter_valid(self.inner) != 0 } + } + + pub fn seek_to_first(&mut self) { + unsafe { ffi::rocksdb_iter_seek_to_first(self.inner); } + self.just_seeked = true; + } + + pub fn seek_to_last(&mut self) { + unsafe { ffi::rocksdb_iter_seek_to_last(self.inner); } + self.just_seeked = true; + } + + pub fn seek(&mut self, key: &[u8]) { + unsafe { ffi::rocksdb_iter_seek(self.inner, key.as_ptr() as *const c_char, key.len() as size_t); } + self.just_seeked = true; + } + +/* + pub fn seek_for_prev(&mut self, key: [u8]) { + unsafe { ffi::rocksdb_iter_seek_for_prev(self.inner, key.as_ptr() as *const c_char, key.len() as size_t); } + self.just_seeked = true; + } +*/ + + pub fn next(&mut self) -> bool { + // Initial call to next() after seeking should not move the iterator + // as the iterator would be positioned on the first element + // This behaviour allows the next() method to be used in a while-loop (eg. while iter.next()) + if self.just_seeked { + self.just_seeked = false; + return self.valid(); + } else { + unsafe { ffi::rocksdb_iter_next(self.inner); } + } + + self.valid() + } + + pub fn prev(&mut self) -> bool { + // Initial call to prev() after seeking should not move the iterator + // as the iterator would be positioned on the first element + // This behaviour allows the prev() method to be used in a while-loop (eg. while iter.prev()) + if self.just_seeked { + self.just_seeked = false; + } else { + unsafe { ffi::rocksdb_iter_prev(self.inner); } + } + + self.valid() + } + + pub unsafe fn key<'a>(&'a self) -> Option<&'a [u8]> { + if self.valid() { + let mut key_len: size_t = 0; + let key_len_ptr: *mut size_t = &mut key_len; + let key_ptr = ffi::rocksdb_iter_key(self.inner, key_len_ptr) as *const c_uchar; + + Some(slice::from_raw_parts(key_ptr, key_len as usize)) + } else { + None + } + } + + pub unsafe fn value<'a>(&'a self) -> Option<&'a [u8]> { + if self.valid() { + let mut val_len: size_t = 0; + let val_len_ptr: *mut size_t = &mut val_len; + let val_ptr = ffi::rocksdb_iter_value(self.inner, val_len_ptr) as *const c_uchar; + + Some(slice::from_raw_parts(val_ptr, val_len as usize)) + } else { + None + } + } +} + +impl Drop for DBRawIterator { + fn drop(&mut self) { + unsafe { + ffi::rocksdb_iter_destroy(self.inner); + } + } +} + impl DBIterator { fn new(db: &DB, readopts: &ReadOptions, mode: IteratorMode) -> DBIterator { unsafe { @@ -270,6 +419,20 @@ impl<'a> Snapshot<'a> { DBIterator::new_cf(self.db, cf_handle, &readopts, mode) } + pub fn raw_iterator(&self) -> DBRawIterator { + let mut readopts = ReadOptions::default(); + readopts.set_snapshot(self); + DBRawIterator::new(self.db, &readopts) + } + + pub fn raw_iterator_cf(&self, + cf_handle: ColumnFamily) + -> Result { + let mut readopts = ReadOptions::default(); + readopts.set_snapshot(self); + DBRawIterator::new_cf(self.db, cf_handle, &readopts) + } + pub fn get(&self, key: &[u8]) -> Result, Error> { let mut readopts = ReadOptions::default(); readopts.set_snapshot(self); @@ -549,6 +712,18 @@ impl DB { DBIterator::new_cf(self, cf_handle, &opts, mode) } + pub fn raw_iterator(&self) -> DBRawIterator { + let opts = ReadOptions::default(); + DBRawIterator::new(self, &opts) + } + + pub fn raw_iterator_cf(&self, + cf_handle: ColumnFamily) + -> Result { + let opts = ReadOptions::default(); + DBRawIterator::new_cf(self, cf_handle, &opts) + } + pub fn snapshot(&self) -> Snapshot { Snapshot::new(self) } diff --git a/src/lib.rs b/src/lib.rs index 3f0f5f5..56d193e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -44,7 +44,7 @@ pub mod compaction_filter; mod db; mod db_options; -pub use db::{DBCompactionStyle, DBCompressionType, DBIterator, DBRecoveryMode, DBVector, +pub use db::{DBCompactionStyle, DBCompressionType, DBIterator, DBRawIterator, DBRecoveryMode, DBVector, Direction, IteratorMode, Snapshot, WriteBatch, new_bloom_filter}; pub use merge_operator::MergeOperands; From 6ad575fc04d53ad9a807b91aabde5f031d9e153a Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sun, 5 Mar 2017 12:58:05 +0000 Subject: [PATCH 02/11] Add safe versions of key/value --- src/db.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/db.rs b/src/db.rs index 678325c..6df57bc 100644 --- a/src/db.rs +++ b/src/db.rs @@ -294,7 +294,7 @@ impl DBRawIterator { self.valid() } - pub unsafe fn key<'a>(&'a self) -> Option<&'a [u8]> { + pub unsafe fn key_inner<'a>(&'a self) -> Option<&'a [u8]> { if self.valid() { let mut key_len: size_t = 0; let key_len_ptr: *mut size_t = &mut key_len; @@ -306,7 +306,13 @@ impl DBRawIterator { } } - pub unsafe fn value<'a>(&'a self) -> Option<&'a [u8]> { + pub fn key(&self) -> Option> { + unsafe { + self.key_inner().map(|key| key.to_vec()) + } + } + + pub unsafe fn value_inner<'a>(&'a self) -> Option<&'a [u8]> { if self.valid() { let mut val_len: size_t = 0; let val_len_ptr: *mut size_t = &mut val_len; @@ -317,6 +323,12 @@ impl DBRawIterator { None } } + + pub fn value(&self) -> Option> { + unsafe { + self.value_inner().map(|value| value.to_vec()) + } + } } impl Drop for DBRawIterator { From 05c01f4e9e07e85e413371c6fa6010408b48c31d Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sun, 5 Mar 2017 13:00:57 +0000 Subject: [PATCH 03/11] Implement seek_for_prev --- librocksdb-sys/src/lib.rs | 2 ++ src/db.rs | 4 +--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/librocksdb-sys/src/lib.rs b/librocksdb-sys/src/lib.rs index fb1ce6c..d70cab2 100644 --- a/librocksdb-sys/src/lib.rs +++ b/librocksdb-sys/src/lib.rs @@ -294,6 +294,8 @@ extern "C" { pub fn rocksdb_iter_seek(iterator: *mut rocksdb_iterator_t, k: *const c_char, klen: size_t); + pub fn rocksdb_iter_seek_for_prev(iterator: *mut rocksdb_iterator_t, k: *const c_char, klen: size_t); + pub fn rocksdb_iter_next(iterator: *mut rocksdb_iterator_t); pub fn rocksdb_iter_prev(iterator: *mut rocksdb_iterator_t); diff --git a/src/db.rs b/src/db.rs index 6df57bc..2290072 100644 --- a/src/db.rs +++ b/src/db.rs @@ -260,12 +260,10 @@ impl DBRawIterator { self.just_seeked = true; } -/* - pub fn seek_for_prev(&mut self, key: [u8]) { + pub fn seek_for_prev(&mut self, key: &[u8]) { unsafe { ffi::rocksdb_iter_seek_for_prev(self.inner, key.as_ptr() as *const c_char, key.len() as size_t); } self.just_seeked = true; } -*/ pub fn next(&mut self) -> bool { // Initial call to next() after seeking should not move the iterator From 3bdc7e4bc4598225fd6cf24386734e4757a4aba5 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sun, 5 Mar 2017 14:02:33 +0000 Subject: [PATCH 04/11] Docs for raw iterator --- src/db.rs | 160 +++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 145 insertions(+), 15 deletions(-) diff --git a/src/db.rs b/src/db.rs index 2290072..8094790 100644 --- a/src/db.rs +++ b/src/db.rs @@ -103,28 +103,38 @@ pub struct Snapshot<'a> { /// An iterator over a database or column family, with specifiable /// ranges and direction. /// +/// This iterator is different to the standard ``DBIterator`` as it aims Into +/// replicate the underlying iterator API within RocksDB itself. This should +/// give access to more performance and flexibility but departs from the +/// widely recognised Rust idioms. +/// /// ``` -/// use rocksdb::{DB, Direction, IteratorMode}; +/// use rocksdb::DB; /// /// let mut db = DB::open_default("path/for/rocksdb/storage2").unwrap(); -/// let mut iter = db.iterator(IteratorMode::Start); // Always iterates forward -/// for (key, value) in iter { -/// println!("Saw {:?} {:?}", key, value); +/// let mut iter = db.raw_iterator(); +/// +/// // Forwards iteration +/// iter.seek_to_first(); +/// while iter.next() { +/// println!("Saw {:?} {:?}", iter.key(), iter.value()); /// } -/// iter = db.iterator(IteratorMode::End); // Always iterates backward -/// for (key, value) in iter { -/// println!("Saw {:?} {:?}", key, value); +/// +/// // Reverse iteration +/// iter.seek_to_last(); +/// while iter.prev() { +/// println!("Saw {:?} {:?}", iter.key(), iter.value()); /// } -/// iter = db.iterator(IteratorMode::From(b"my key", Direction::Forward)); // From a key in Direction::{forward,reverse} -/// for (key, value) in iter { -/// println!("Saw {:?} {:?}", key, value); +/// +/// // Seeking +/// iter = iter.seek(b"my key"); +/// while iter.next() { +/// println!("Saw {:?} {:?}", iter.key(), iter.value()); /// } /// -/// // You can seek with an existing Iterator instance, too -/// iter = db.iterator(IteratorMode::Start); -/// iter.set_mode(IteratorMode::From(b"another key", Direction::Reverse)); -/// for (key, value) in iter { -/// println!("Saw {:?} {:?}", key, value); +/// iter = iter.seek_for_prev(b"my key"); +/// while iter.prev() { +/// println!("Saw {:?} {:?}", iter.key(), iter.value()); /// } /// ``` pub struct DBRawIterator { @@ -241,30 +251,131 @@ impl DBRawIterator { } } + /// Returns true if the iterator is valid. pub fn valid(&self) -> bool { unsafe { ffi::rocksdb_iter_valid(self.inner) != 0 } } + /// Seeks to the first key in the database. + /// + /// You must call ``.next()`` before reading the key. + /// + /// # Examples + /// + /// ```rust,no_run + /// // Iterate all keys from the start in lexicographic order + /// + /// iterator.seek_to_first(); + /// + /// while iterator.next() { + /// println!("{:?} {:?}", iterator.key(), iterator.value()); + /// } + /// ``` + /// + /// ```rust,no_run + /// // Read just the first key + /// + /// iterator.seek_to_first(); + /// + /// let is_valid = iterator.next(); + /// if is_valid { + /// println!("{:?} {:?}", iterator.key(), iterator.value()); + /// } else { + /// // There are no keys in the database + /// } + /// ``` pub fn seek_to_first(&mut self) { unsafe { ffi::rocksdb_iter_seek_to_first(self.inner); } self.just_seeked = true; } + /// Seeks to the last key in the database. + /// + /// You must call ``.prev()`` before reading the key. + /// + /// # Examples + /// + /// ```rust,no_run + /// // Iterate all keys from the end in reverse lexicographic order + /// + /// iterator.seek_to_last(); + /// + /// while iterator.prev() { + /// println!("{:?} {:?}", iterator.key(), iterator.value());; + /// } + /// ``` + /// + /// ```rust,no_run + /// // Read just the last key + /// + /// iterator.seek_to_last(); + /// + /// let is_valid = iterator.prev(); + /// if is_valid { + /// println!("{:?} {:?}", iterator.key(), iterator.value()); + /// } else { + /// // There are no keys in the database + /// } + /// ``` pub fn seek_to_last(&mut self) { unsafe { ffi::rocksdb_iter_seek_to_last(self.inner); } self.just_seeked = true; } + /// Seeks to the specified key or the first key that lexicographically follows it. + /// + /// This method will attempt to seek to the specified key. If that key does not exist, it will + /// find and seek to the key that lexicographically follows it instead. + /// + /// Like the other seek methods, you must call ``.next()`` or ``.prev()`` before reading a key. + /// + /// # Examples + /// + /// ```rust,no_run + /// // Read the first key that starts with 'a' + /// + /// iterator.seek(b"a"); + /// + /// let is_valid = iterator.next(); + /// if is_valid { + /// println!("{:?} {:?}", iterator.key(), iterator.value()); + /// } else { + /// // There are no keys in the database + /// } + /// ``` pub fn seek(&mut self, key: &[u8]) { unsafe { ffi::rocksdb_iter_seek(self.inner, key.as_ptr() as *const c_char, key.len() as size_t); } self.just_seeked = true; } + /// Seeks to the specified key, or the first key that lexicographically precedes it. + /// + /// Like ``.seek()`` this method will attempt to seek to the specified key. + /// The difference with ``.seek()`` is that if the specified key do not exist, this method will + /// seek to key that lexicographically precedes it instead. + /// + /// Like the other seek methods, you must call ``.next()`` or ``.prev()`` before reading a key. + /// # Examples + /// + /// ```rust,no_run + /// // Read the last key that starts with 'a' + /// + /// iterator.seek_for_prev(b"b"); + /// + /// let is_valid = iterator.prev(); + /// if is_valid { + /// println!("{:?} {:?}", iterator.key(), iterator.value()); + /// } else { + /// // There are no keys in the database + /// } pub fn seek_for_prev(&mut self, key: &[u8]) { unsafe { ffi::rocksdb_iter_seek_for_prev(self.inner, key.as_ptr() as *const c_char, key.len() as size_t); } self.just_seeked = true; } + /// Seeks to the next key. + /// + /// Returns true if the iterator is valid after this operation. pub fn next(&mut self) -> bool { // Initial call to next() after seeking should not move the iterator // as the iterator would be positioned on the first element @@ -279,6 +390,9 @@ impl DBRawIterator { self.valid() } + /// Seeks to the previous key. + /// + /// Returns true if the iterator is valid after this operation. pub fn prev(&mut self) -> bool { // Initial call to prev() after seeking should not move the iterator // as the iterator would be positioned on the first element @@ -292,6 +406,13 @@ impl DBRawIterator { self.valid() } + /// Returns a slice to the internal buffer storing the current key. + /// + /// This may be slightly more performant to use than the standard ``.key()`` method + /// as it does not copy the key. However, you must be careful to not use the buffer + /// if the iterator's seek position is ever moved by any of the seek commands or the + /// ``.next()`` and ``.previous()`` methods as the underlying buffer may be reused + /// for something else or freed entirely. pub unsafe fn key_inner<'a>(&'a self) -> Option<&'a [u8]> { if self.valid() { let mut key_len: size_t = 0; @@ -304,12 +425,20 @@ impl DBRawIterator { } } + /// Returns a copy of the current key. pub fn key(&self) -> Option> { unsafe { self.key_inner().map(|key| key.to_vec()) } } + /// Returns a slice to the internal buffer storing the current value. + /// + /// This may be slightly more performant to use than the standard ``.value()`` method + /// as it does not copy the value. However, you must be careful to not use the buffer + /// if the iterator's seek position is ever moved by any of the seek commands or the + /// ``.next()`` and ``.previous()`` methods as the underlying buffer may be reused + /// for something else or freed entirely. pub unsafe fn value_inner<'a>(&'a self) -> Option<&'a [u8]> { if self.valid() { let mut val_len: size_t = 0; @@ -322,6 +451,7 @@ impl DBRawIterator { } } + /// Returns a copy of the current value. pub fn value(&self) -> Option> { unsafe { self.value_inner().map(|value| value.to_vec()) From 007616446f9ad66bcbe0c97d3671a366e48a07e1 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sun, 5 Mar 2017 16:54:10 +0000 Subject: [PATCH 05/11] Make doctests runnable --- src/db.rs | 74 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 45 insertions(+), 29 deletions(-) diff --git a/src/db.rs b/src/db.rs index 8094790..f90b4d9 100644 --- a/src/db.rs +++ b/src/db.rs @@ -111,7 +111,7 @@ pub struct Snapshot<'a> { /// ``` /// use rocksdb::DB; /// -/// let mut db = DB::open_default("path/for/rocksdb/storage2").unwrap(); +/// let mut db = DB::open_default("path/for/rocksdb/storage4").unwrap(); /// let mut iter = db.raw_iterator(); /// /// // Forwards iteration @@ -127,12 +127,12 @@ pub struct Snapshot<'a> { /// } /// /// // Seeking -/// iter = iter.seek(b"my key"); +/// iter.seek(b"my key"); /// while iter.next() { /// println!("Saw {:?} {:?}", iter.key(), iter.value()); /// } /// -/// iter = iter.seek_for_prev(b"my key"); +/// iter.seek_for_prev(b"my key"); /// while iter.prev() { /// println!("Saw {:?} {:?}", iter.key(), iter.value()); /// } @@ -262,24 +262,27 @@ impl DBRawIterator { /// /// # Examples /// - /// ```rust,no_run + /// ```rust + /// use rocksdb::DB; + /// + /// let mut db = DB::open_default("path/for/rocksdb/storage5").unwrap(); + /// let mut iter = db.raw_iterator(); + /// /// // Iterate all keys from the start in lexicographic order /// - /// iterator.seek_to_first(); + /// iter.seek_to_first(); /// - /// while iterator.next() { - /// println!("{:?} {:?}", iterator.key(), iterator.value()); + /// while iter.next() { + /// println!("{:?} {:?}", iter.key(), iter.value()); /// } - /// ``` /// - /// ```rust,no_run /// // Read just the first key /// - /// iterator.seek_to_first(); + /// iter.seek_to_first(); /// - /// let is_valid = iterator.next(); + /// let is_valid = iter.next(); /// if is_valid { - /// println!("{:?} {:?}", iterator.key(), iterator.value()); + /// println!("{:?} {:?}", iter.key(), iter.value()); /// } else { /// // There are no keys in the database /// } @@ -295,24 +298,27 @@ impl DBRawIterator { /// /// # Examples /// - /// ```rust,no_run + /// ```rust + /// use rocksdb::DB; + /// + /// let mut db = DB::open_default("path/for/rocksdb/storage6").unwrap(); + /// let mut iter = db.raw_iterator(); + /// /// // Iterate all keys from the end in reverse lexicographic order /// - /// iterator.seek_to_last(); + /// iter.seek_to_last(); /// - /// while iterator.prev() { - /// println!("{:?} {:?}", iterator.key(), iterator.value());; + /// while iter.prev() { + /// println!("{:?} {:?}", iter.key(), iter.value());; /// } - /// ``` /// - /// ```rust,no_run /// // Read just the last key /// - /// iterator.seek_to_last(); + /// iter.seek_to_last(); /// - /// let is_valid = iterator.prev(); + /// let is_valid = iter.prev(); /// if is_valid { - /// println!("{:?} {:?}", iterator.key(), iterator.value()); + /// println!("{:?} {:?}", iter.key(), iter.value()); /// } else { /// // There are no keys in the database /// } @@ -331,14 +337,19 @@ impl DBRawIterator { /// /// # Examples /// - /// ```rust,no_run + /// ```rust + /// use rocksdb::DB; + /// + /// let mut db = DB::open_default("path/for/rocksdb/storage7").unwrap(); + /// let mut iter = db.raw_iterator(); + /// /// // Read the first key that starts with 'a' /// - /// iterator.seek(b"a"); + /// iter.seek(b"a"); /// - /// let is_valid = iterator.next(); + /// let is_valid = iter.next(); /// if is_valid { - /// println!("{:?} {:?}", iterator.key(), iterator.value()); + /// println!("{:?} {:?}", iter.key(), iter.value()); /// } else { /// // There are no keys in the database /// } @@ -357,14 +368,19 @@ impl DBRawIterator { /// Like the other seek methods, you must call ``.next()`` or ``.prev()`` before reading a key. /// # Examples /// - /// ```rust,no_run + /// ```rust + /// use rocksdb::DB; + /// + /// let mut db = DB::open_default("path/for/rocksdb/storage8").unwrap(); + /// let mut iter = db.raw_iterator(); + /// /// // Read the last key that starts with 'a' /// - /// iterator.seek_for_prev(b"b"); + /// iter.seek_for_prev(b"b"); /// - /// let is_valid = iterator.prev(); + /// let is_valid = iter.prev(); /// if is_valid { - /// println!("{:?} {:?}", iterator.key(), iterator.value()); + /// println!("{:?} {:?}", iter.key(), iter.value()); /// } else { /// // There are no keys in the database /// } From cb136318ce44e2103971ac5c76a66a08f362823c Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sun, 5 Mar 2017 16:56:35 +0000 Subject: [PATCH 06/11] Removed seek_for_prev Not implemented in this version of RocksDB --- librocksdb-sys/src/lib.rs | 2 -- src/db.rs | 8 ++++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/librocksdb-sys/src/lib.rs b/librocksdb-sys/src/lib.rs index d70cab2..fb1ce6c 100644 --- a/librocksdb-sys/src/lib.rs +++ b/librocksdb-sys/src/lib.rs @@ -294,8 +294,6 @@ extern "C" { pub fn rocksdb_iter_seek(iterator: *mut rocksdb_iterator_t, k: *const c_char, klen: size_t); - pub fn rocksdb_iter_seek_for_prev(iterator: *mut rocksdb_iterator_t, k: *const c_char, klen: size_t); - pub fn rocksdb_iter_next(iterator: *mut rocksdb_iterator_t); pub fn rocksdb_iter_prev(iterator: *mut rocksdb_iterator_t); diff --git a/src/db.rs b/src/db.rs index f90b4d9..819396f 100644 --- a/src/db.rs +++ b/src/db.rs @@ -132,10 +132,6 @@ pub struct Snapshot<'a> { /// println!("Saw {:?} {:?}", iter.key(), iter.value()); /// } /// -/// iter.seek_for_prev(b"my key"); -/// while iter.prev() { -/// println!("Saw {:?} {:?}", iter.key(), iter.value()); -/// } /// ``` pub struct DBRawIterator { inner: *mut ffi::rocksdb_iterator_t, @@ -359,6 +355,9 @@ impl DBRawIterator { self.just_seeked = true; } +/* + SeekForPrev was added in RocksDB 4.13 but not implemented in the C API until RocksDB 5.0 + /// Seeks to the specified key, or the first key that lexicographically precedes it. /// /// Like ``.seek()`` this method will attempt to seek to the specified key. @@ -388,6 +387,7 @@ impl DBRawIterator { unsafe { ffi::rocksdb_iter_seek_for_prev(self.inner, key.as_ptr() as *const c_char, key.len() as size_t); } self.just_seeked = true; } +*/ /// Seeks to the next key. /// From c8b7e8e7dd8a19dc1b94e85c3e3006ad70d6f503 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sun, 5 Mar 2017 17:14:44 +0000 Subject: [PATCH 07/11] Reimplement DBIterator on top of DBRawIterator --- src/db.rs | 111 ++++++++++++++++++------------------------------------ 1 file changed, 37 insertions(+), 74 deletions(-) diff --git a/src/db.rs b/src/db.rs index 819396f..742d53f 100644 --- a/src/db.rs +++ b/src/db.rs @@ -167,9 +167,8 @@ pub struct DBRawIterator { /// } /// ``` pub struct DBIterator { - inner: *mut ffi::rocksdb_iterator_t, + raw: DBRawIterator, direction: Direction, - just_seeked: bool, } pub enum Direction { @@ -183,28 +182,14 @@ impl Iterator for DBIterator { type Item = KVBytes; fn next(&mut self) -> Option { - let native_iter = self.inner; - if !self.just_seeked { - match self.direction { - Direction::Forward => unsafe { ffi::rocksdb_iter_next(native_iter) }, - Direction::Reverse => unsafe { ffi::rocksdb_iter_prev(native_iter) }, - } - } else { - self.just_seeked = false; - } - if unsafe { ffi::rocksdb_iter_valid(native_iter) != 0 } { - let mut key_len: size_t = 0; - let key_len_ptr: *mut size_t = &mut key_len; - let mut val_len: size_t = 0; - let val_len_ptr: *mut size_t = &mut val_len; - let key_ptr = - unsafe { ffi::rocksdb_iter_key(native_iter, key_len_ptr) as *const c_uchar }; - let key = unsafe { slice::from_raw_parts(key_ptr, key_len as usize) }; - let val_ptr = - unsafe { ffi::rocksdb_iter_value(native_iter, val_len_ptr) as *const c_uchar }; - let val = unsafe { slice::from_raw_parts(val_ptr, val_len as usize) }; - - Some((key.to_vec().into_boxed_slice(), val.to_vec().into_boxed_slice())) + let valid = match self.direction { + Direction::Forward => self.raw.next(), + Direction::Reverse => self.raw.prev(), + }; + + if valid { + // .key() and .value() only ever return None if valid == false, which we've just cheked + Some((self.raw.key().unwrap().into_boxed_slice(), self.raw.value().unwrap().into_boxed_slice())) } else { None } @@ -485,43 +470,34 @@ impl Drop for DBRawIterator { impl DBIterator { fn new(db: &DB, readopts: &ReadOptions, mode: IteratorMode) -> DBIterator { - unsafe { - let iterator = ffi::rocksdb_create_iterator(db.inner, readopts.inner); - - let mut rv = DBIterator { - inner: iterator, - direction: Direction::Forward, // blown away by set_mode() - just_seeked: false, - }; - rv.set_mode(mode); - rv - } + let mut rv = DBIterator { + raw: DBRawIterator::new(db, readopts), + direction: Direction::Forward, // blown away by set_mode() + }; + rv.set_mode(mode); + rv } pub fn set_mode(&mut self, mode: IteratorMode) { - unsafe { - match mode { - IteratorMode::Start => { - ffi::rocksdb_iter_seek_to_first(self.inner); - self.direction = Direction::Forward; - } - IteratorMode::End => { - ffi::rocksdb_iter_seek_to_last(self.inner); - self.direction = Direction::Reverse; - } - IteratorMode::From(key, dir) => { - ffi::rocksdb_iter_seek(self.inner, - key.as_ptr() as *const c_char, - key.len() as size_t); - self.direction = dir; - } - }; - self.just_seeked = true; - } + match mode { + IteratorMode::Start => { + self.raw.seek_to_first(); + self.direction = Direction::Forward; + } + IteratorMode::End => { + self.raw.seek_to_last(); + self.direction = Direction::Reverse; + } + IteratorMode::From(key, dir) => { + // TODO: Should use seek_for_prev when reversing + self.raw.seek(key); + self.direction = dir; + } + }; } pub fn valid(&self) -> bool { - unsafe { ffi::rocksdb_iter_valid(self.inner) != 0 } + self.raw.valid() } fn new_cf(db: &DB, @@ -529,25 +505,12 @@ impl DBIterator { readopts: &ReadOptions, mode: IteratorMode) -> Result { - unsafe { - let iterator = ffi::rocksdb_create_iterator_cf(db.inner, readopts.inner, cf_handle.inner); - - let mut rv = DBIterator { - inner: iterator, - direction: Direction::Forward, // blown away by set_mode() - just_seeked: false, - }; - rv.set_mode(mode); - Ok(rv) - } - } -} - -impl Drop for DBIterator { - fn drop(&mut self) { - unsafe { - ffi::rocksdb_iter_destroy(self.inner); - } + let mut rv = DBIterator { + raw: try!(DBRawIterator::new_cf(db, cf_handle, readopts)), + direction: Direction::Forward, // blown away by set_mode() + }; + rv.set_mode(mode); + Ok(rv) } } From 1270e572d065e190e362a83912587fc33f8732e6 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sun, 5 Mar 2017 17:16:05 +0000 Subject: [PATCH 08/11] Moved some code --- src/db.rs | 56 +++++++++++++++++++++++++++---------------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/src/db.rs b/src/db.rs index 742d53f..99f33f2 100644 --- a/src/db.rs +++ b/src/db.rs @@ -178,24 +178,6 @@ pub enum Direction { pub type KVBytes = (Box<[u8]>, Box<[u8]>); -impl Iterator for DBIterator { - type Item = KVBytes; - - fn next(&mut self) -> Option { - let valid = match self.direction { - Direction::Forward => self.raw.next(), - Direction::Reverse => self.raw.prev(), - }; - - if valid { - // .key() and .value() only ever return None if valid == false, which we've just cheked - Some((self.raw.key().unwrap().into_boxed_slice(), self.raw.value().unwrap().into_boxed_slice())) - } else { - None - } - } -} - pub enum IteratorMode<'a> { Start, End, @@ -478,6 +460,19 @@ impl DBIterator { rv } + fn new_cf(db: &DB, + cf_handle: ColumnFamily, + readopts: &ReadOptions, + mode: IteratorMode) + -> Result { + let mut rv = DBIterator { + raw: try!(DBRawIterator::new_cf(db, cf_handle, readopts)), + direction: Direction::Forward, // blown away by set_mode() + }; + rv.set_mode(mode); + Ok(rv) + } + pub fn set_mode(&mut self, mode: IteratorMode) { match mode { IteratorMode::Start => { @@ -499,18 +494,23 @@ impl DBIterator { pub fn valid(&self) -> bool { self.raw.valid() } +} - fn new_cf(db: &DB, - cf_handle: ColumnFamily, - readopts: &ReadOptions, - mode: IteratorMode) - -> Result { - let mut rv = DBIterator { - raw: try!(DBRawIterator::new_cf(db, cf_handle, readopts)), - direction: Direction::Forward, // blown away by set_mode() +impl Iterator for DBIterator { + type Item = KVBytes; + + fn next(&mut self) -> Option { + let valid = match self.direction { + Direction::Forward => self.raw.next(), + Direction::Reverse => self.raw.prev(), }; - rv.set_mode(mode); - Ok(rv) + + if valid { + // .key() and .value() only ever return None if valid == false, which we've just cheked + Some((self.raw.key().unwrap().into_boxed_slice(), self.raw.value().unwrap().into_boxed_slice())) + } else { + None + } } } From d9725bcff92f83e3b350db42c3aaab235234f211 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sun, 5 Mar 2017 17:30:29 +0000 Subject: [PATCH 09/11] Allow DBIterator to be converted into DBRawIterator --- src/db.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/db.rs b/src/db.rs index 99f33f2..d65896e 100644 --- a/src/db.rs +++ b/src/db.rs @@ -514,6 +514,12 @@ impl Iterator for DBIterator { } } +impl Into for DBIterator { + fn into(self) -> DBRawIterator { + self.raw + } +} + impl<'a> Snapshot<'a> { pub fn new(db: &DB) -> Snapshot { let snapshot = unsafe { ffi::rocksdb_create_snapshot(db.inner) }; From 73d75af5c3fdc86ab550ef15b812c88f3c7848ab Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sun, 5 Mar 2017 17:57:16 +0000 Subject: [PATCH 10/11] Added tests for DBRawIterator --- test/test.rs | 1 + test/test_raw_iterator.rs | 192 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 193 insertions(+) create mode 100644 test/test_raw_iterator.rs diff --git a/test/test.rs b/test/test.rs index 295d4b0..c1a6f0e 100644 --- a/test/test.rs +++ b/test/test.rs @@ -16,6 +16,7 @@ extern crate rocksdb; mod test_iterator; +mod test_raw_iterator; mod test_multithreaded; mod test_column_family; mod test_rocksdb_options; diff --git a/test/test_raw_iterator.rs b/test/test_raw_iterator.rs new file mode 100644 index 0000000..9bb27bc --- /dev/null +++ b/test/test_raw_iterator.rs @@ -0,0 +1,192 @@ +// Copyright 2014 Tyler Neely +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +use rocksdb::DB; + + +fn setup_test_db(name: &str) -> DB { + use std::fs::remove_dir_all; + + let path = "_rust_rocksdb_rawiteratortest_".to_string() + name; + + match remove_dir_all(&path) { + Ok(_) => {} + Err(_) => {} // Don't care if tis fails + } + + DB::open_default(path).unwrap() +} + + +#[test] +pub fn test_forwards_iteration() { + let db = setup_test_db("forwards_iteration"); + db.put(b"k1", b"v1").unwrap(); + db.put(b"k2", b"v2").unwrap(); + db.put(b"k3", b"v3").unwrap(); + db.put(b"k4", b"v4").unwrap(); + + let mut iter = db.raw_iterator(); + iter.seek_to_first(); + + // Shouldn't be valid yet + assert_eq!(iter.valid(), false); + assert_eq!(iter.key(), None); + assert_eq!(iter.value(), None); + + let valid = iter.next(); + + assert_eq!(valid, true); + assert_eq!(iter.valid(), true); + assert_eq!(iter.key(), Some(b"k1".to_vec())); + assert_eq!(iter.value(), Some(b"v1".to_vec())); + + let valid = iter.next(); + + assert_eq!(valid, true); + assert_eq!(iter.valid(), true); + assert_eq!(iter.key(), Some(b"k2".to_vec())); + assert_eq!(iter.value(), Some(b"v2".to_vec())); + + iter.next(); // k3 + iter.next(); // k4 + + let valid = iter.next(); + + // Should be invalid again + assert_eq!(valid, false); + assert_eq!(iter.valid(), false); + assert_eq!(iter.key(), None); + assert_eq!(iter.value(), None); +} + + +#[test] +pub fn test_seek_last() { + let db = setup_test_db("backwards_iteration"); + db.put(b"k1", b"v1").unwrap(); + db.put(b"k2", b"v2").unwrap(); + db.put(b"k3", b"v3").unwrap(); + db.put(b"k4", b"v4").unwrap(); + + let mut iter = db.raw_iterator(); + iter.seek_to_last(); + + // Shouldn't be valid yet + assert_eq!(iter.valid(), false); + assert_eq!(iter.key(), None); + assert_eq!(iter.value(), None); + + let valid = iter.prev(); + + assert_eq!(valid, true); + assert_eq!(iter.valid(), true); + assert_eq!(iter.key(), Some(b"k4".to_vec())); + assert_eq!(iter.value(), Some(b"v4".to_vec())); + + let valid = iter.prev(); + + assert_eq!(valid, true); + assert_eq!(iter.valid(), true); + assert_eq!(iter.key(), Some(b"k3".to_vec())); + assert_eq!(iter.value(), Some(b"v3".to_vec())); + + iter.prev(); // k2 + iter.prev(); // k1 + + let valid = iter.prev(); + + // Should be invalid again + assert_eq!(valid, false); + assert_eq!(iter.valid(), false); + assert_eq!(iter.key(), None); + assert_eq!(iter.value(), None); +} + + +#[test] +pub fn test_seek() { + let db = setup_test_db("seek"); + db.put(b"k1", b"v1").unwrap(); + db.put(b"k2", b"v2").unwrap(); + db.put(b"k3", b"v3").unwrap(); + db.put(b"k4", b"v4").unwrap(); + + let mut iter = db.raw_iterator(); + iter.seek(b"k2"); + + // Shouldn't be valid yet + assert_eq!(iter.valid(), false); + assert_eq!(iter.key(), None); + assert_eq!(iter.value(), None); + + let valid = iter.next(); + + // Should now be valid + assert_eq!(valid, true); + assert_eq!(iter.valid(), true); + assert_eq!(iter.key(), Some(b"k2".to_vec())); + assert_eq!(iter.value(), Some(b"v2".to_vec())); +} + + +#[test] +pub fn test_seek_then_prev() { + let db = setup_test_db("seek_then_prev"); + db.put(b"k1", b"v1").unwrap(); + db.put(b"k2", b"v2").unwrap(); + db.put(b"k3", b"v3").unwrap(); + db.put(b"k4", b"v4").unwrap(); + + let mut iter = db.raw_iterator(); + iter.seek(b"k2"); + + // Shouldn't be valid yet + assert_eq!(iter.valid(), false); + assert_eq!(iter.key(), None); + assert_eq!(iter.value(), None); + + let valid = iter.prev(); + + // Should now be valid + assert_eq!(valid, true); + assert_eq!(iter.valid(), true); + assert_eq!(iter.key(), Some(b"k2".to_vec())); + assert_eq!(iter.value(), Some(b"v2".to_vec())); +} + + +#[test] +pub fn test_seek_to_nonexistant() { + let db = setup_test_db("seek_to_nonexistant"); + db.put(b"k1", b"v1").unwrap(); + db.put(b"k3", b"v3").unwrap(); + db.put(b"k4", b"v4").unwrap(); + + let mut iter = db.raw_iterator(); + iter.seek(b"k2"); + + // Shouldn't be valid yet + assert_eq!(iter.valid(), false); + assert_eq!(iter.key(), None); + assert_eq!(iter.value(), None); + + let valid = iter.next(); + + assert_eq!(valid, true); + assert_eq!(iter.valid(), true); + assert_eq!(iter.key(), Some(b"k3".to_vec())); + assert_eq!(iter.value(), Some(b"v3".to_vec())); +} From 6f1aae1997e1a888799fc6925ee03f719d5a81e6 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sun, 5 Mar 2017 18:25:56 +0000 Subject: [PATCH 11/11] Remove just_seeked from DBRawIterator This as added to make while-loops easy: while iter.next() { ... } But this involves adding a layer of custom logic on top of the RocksDB API. This is probably a bad idea given that this API is meant to closely match the underlying iterator API in RocksDB, so this commit changes that. The new way to iterate is as follows: while iter.valid() { ... iter.next(); } --- src/db.rs | 113 +++++++++++++++----------------------- test/test_raw_iterator.rs | 75 ++----------------------- 2 files changed, 47 insertions(+), 141 deletions(-) diff --git a/src/db.rs b/src/db.rs index d65896e..37a370f 100644 --- a/src/db.rs +++ b/src/db.rs @@ -116,26 +116,28 @@ pub struct Snapshot<'a> { /// /// // Forwards iteration /// iter.seek_to_first(); -/// while iter.next() { +/// while iter.valid() { /// println!("Saw {:?} {:?}", iter.key(), iter.value()); +/// iter.next(); /// } /// /// // Reverse iteration /// iter.seek_to_last(); -/// while iter.prev() { +/// while iter.valid() { /// println!("Saw {:?} {:?}", iter.key(), iter.value()); +/// iter.prev(); /// } /// /// // Seeking /// iter.seek(b"my key"); -/// while iter.next() { +/// while iter.valid() { /// println!("Saw {:?} {:?}", iter.key(), iter.value()); +/// iter.next(); /// } /// /// ``` pub struct DBRawIterator { inner: *mut ffi::rocksdb_iterator_t, - just_seeked: bool, } @@ -169,6 +171,7 @@ pub struct DBRawIterator { pub struct DBIterator { raw: DBRawIterator, direction: Direction, + just_seeked: bool, } pub enum Direction { @@ -187,14 +190,9 @@ pub enum IteratorMode<'a> { impl DBRawIterator { fn new(db: &DB, readopts: &ReadOptions) -> DBRawIterator { unsafe { - let iterator = ffi::rocksdb_create_iterator(db.inner, readopts.inner); - - let mut rv = DBRawIterator { - inner: iterator, - just_seeked: false, - }; - rv.seek_to_first(); - rv + DBRawIterator { + inner: ffi::rocksdb_create_iterator(db.inner, readopts.inner), + } } } @@ -203,14 +201,9 @@ impl DBRawIterator { readopts: &ReadOptions) -> Result { unsafe { - let iterator = ffi::rocksdb_create_iterator_cf(db.inner, readopts.inner, cf_handle.inner); - - let mut rv = DBRawIterator { - inner: iterator, - just_seeked: false, - }; - rv.seek_to_first(); - Ok(rv) + Ok(DBRawIterator { + inner: ffi::rocksdb_create_iterator_cf(db.inner, readopts.inner, cf_handle.inner), + }) } } @@ -221,8 +214,6 @@ impl DBRawIterator { /// Seeks to the first key in the database. /// - /// You must call ``.next()`` before reading the key. - /// /// # Examples /// /// ```rust @@ -235,16 +226,17 @@ impl DBRawIterator { /// /// iter.seek_to_first(); /// - /// while iter.next() { + /// while iter.valid() { /// println!("{:?} {:?}", iter.key(), iter.value()); + /// + /// iter.next(); /// } /// /// // Read just the first key /// /// iter.seek_to_first(); /// - /// let is_valid = iter.next(); - /// if is_valid { + /// if iter.valid() { /// println!("{:?} {:?}", iter.key(), iter.value()); /// } else { /// // There are no keys in the database @@ -252,13 +244,10 @@ impl DBRawIterator { /// ``` pub fn seek_to_first(&mut self) { unsafe { ffi::rocksdb_iter_seek_to_first(self.inner); } - self.just_seeked = true; } /// Seeks to the last key in the database. /// - /// You must call ``.prev()`` before reading the key. - /// /// # Examples /// /// ```rust @@ -271,16 +260,17 @@ impl DBRawIterator { /// /// iter.seek_to_last(); /// - /// while iter.prev() { - /// println!("{:?} {:?}", iter.key(), iter.value());; + /// while iter.valid() { + /// println!("{:?} {:?}", iter.key(), iter.value()); + /// + /// iter.prev(); /// } /// /// // Read just the last key /// /// iter.seek_to_last(); /// - /// let is_valid = iter.prev(); - /// if is_valid { + /// if iter.valid() { /// println!("{:?} {:?}", iter.key(), iter.value()); /// } else { /// // There are no keys in the database @@ -288,7 +278,6 @@ impl DBRawIterator { /// ``` pub fn seek_to_last(&mut self) { unsafe { ffi::rocksdb_iter_seek_to_last(self.inner); } - self.just_seeked = true; } /// Seeks to the specified key or the first key that lexicographically follows it. @@ -296,8 +285,6 @@ impl DBRawIterator { /// This method will attempt to seek to the specified key. If that key does not exist, it will /// find and seek to the key that lexicographically follows it instead. /// - /// Like the other seek methods, you must call ``.next()`` or ``.prev()`` before reading a key. - /// /// # Examples /// /// ```rust @@ -310,8 +297,7 @@ impl DBRawIterator { /// /// iter.seek(b"a"); /// - /// let is_valid = iter.next(); - /// if is_valid { + /// if iter.valid() { /// println!("{:?} {:?}", iter.key(), iter.value()); /// } else { /// // There are no keys in the database @@ -319,7 +305,6 @@ impl DBRawIterator { /// ``` pub fn seek(&mut self, key: &[u8]) { unsafe { ffi::rocksdb_iter_seek(self.inner, key.as_ptr() as *const c_char, key.len() as size_t); } - self.just_seeked = true; } /* @@ -331,7 +316,6 @@ impl DBRawIterator { /// The difference with ``.seek()`` is that if the specified key do not exist, this method will /// seek to key that lexicographically precedes it instead. /// - /// Like the other seek methods, you must call ``.next()`` or ``.prev()`` before reading a key. /// # Examples /// /// ```rust @@ -344,49 +328,28 @@ impl DBRawIterator { /// /// iter.seek_for_prev(b"b"); /// - /// let is_valid = iter.prev(); - /// if is_valid { + /// if iter.valid() { /// println!("{:?} {:?}", iter.key(), iter.value()); /// } else { /// // There are no keys in the database /// } pub fn seek_for_prev(&mut self, key: &[u8]) { unsafe { ffi::rocksdb_iter_seek_for_prev(self.inner, key.as_ptr() as *const c_char, key.len() as size_t); } - self.just_seeked = true; } */ /// Seeks to the next key. /// /// Returns true if the iterator is valid after this operation. - pub fn next(&mut self) -> bool { - // Initial call to next() after seeking should not move the iterator - // as the iterator would be positioned on the first element - // This behaviour allows the next() method to be used in a while-loop (eg. while iter.next()) - if self.just_seeked { - self.just_seeked = false; - return self.valid(); - } else { - unsafe { ffi::rocksdb_iter_next(self.inner); } - } - - self.valid() + pub fn next(&mut self) { + unsafe { ffi::rocksdb_iter_next(self.inner); } } /// Seeks to the previous key. /// /// Returns true if the iterator is valid after this operation. - pub fn prev(&mut self) -> bool { - // Initial call to prev() after seeking should not move the iterator - // as the iterator would be positioned on the first element - // This behaviour allows the prev() method to be used in a while-loop (eg. while iter.prev()) - if self.just_seeked { - self.just_seeked = false; - } else { - unsafe { ffi::rocksdb_iter_prev(self.inner); } - } - - self.valid() + pub fn prev(&mut self) { + unsafe { ffi::rocksdb_iter_prev(self.inner); } } /// Returns a slice to the internal buffer storing the current key. @@ -455,6 +418,7 @@ impl DBIterator { let mut rv = DBIterator { raw: DBRawIterator::new(db, readopts), direction: Direction::Forward, // blown away by set_mode() + just_seeked: false, }; rv.set_mode(mode); rv @@ -468,6 +432,7 @@ impl DBIterator { let mut rv = DBIterator { raw: try!(DBRawIterator::new_cf(db, cf_handle, readopts)), direction: Direction::Forward, // blown away by set_mode() + just_seeked: false, }; rv.set_mode(mode); Ok(rv) @@ -489,6 +454,8 @@ impl DBIterator { self.direction = dir; } }; + + self.just_seeked = true; } pub fn valid(&self) -> bool { @@ -500,12 +467,18 @@ impl Iterator for DBIterator { type Item = KVBytes; fn next(&mut self) -> Option { - let valid = match self.direction { - Direction::Forward => self.raw.next(), - Direction::Reverse => self.raw.prev(), - }; + // Initial call to next() after seeking should not move the iterator + // or the first item will not be returned + if !self.just_seeked { + match self.direction { + Direction::Forward => self.raw.next(), + Direction::Reverse => self.raw.prev(), + } + } else { + self.just_seeked = false; + } - if valid { + if self.raw.valid() { // .key() and .value() only ever return None if valid == false, which we've just cheked Some((self.raw.key().unwrap().into_boxed_slice(), self.raw.value().unwrap().into_boxed_slice())) } else { diff --git a/test/test_raw_iterator.rs b/test/test_raw_iterator.rs index 9bb27bc..2c57280 100644 --- a/test/test_raw_iterator.rs +++ b/test/test_raw_iterator.rs @@ -41,32 +41,20 @@ pub fn test_forwards_iteration() { let mut iter = db.raw_iterator(); iter.seek_to_first(); - // Shouldn't be valid yet - assert_eq!(iter.valid(), false); - assert_eq!(iter.key(), None); - assert_eq!(iter.value(), None); - - let valid = iter.next(); - - assert_eq!(valid, true); assert_eq!(iter.valid(), true); assert_eq!(iter.key(), Some(b"k1".to_vec())); assert_eq!(iter.value(), Some(b"v1".to_vec())); - let valid = iter.next(); + iter.next(); - assert_eq!(valid, true); assert_eq!(iter.valid(), true); assert_eq!(iter.key(), Some(b"k2".to_vec())); assert_eq!(iter.value(), Some(b"v2".to_vec())); iter.next(); // k3 iter.next(); // k4 + iter.next(); // invalid! - let valid = iter.next(); - - // Should be invalid again - assert_eq!(valid, false); assert_eq!(iter.valid(), false); assert_eq!(iter.key(), None); assert_eq!(iter.value(), None); @@ -84,32 +72,20 @@ pub fn test_seek_last() { let mut iter = db.raw_iterator(); iter.seek_to_last(); - // Shouldn't be valid yet - assert_eq!(iter.valid(), false); - assert_eq!(iter.key(), None); - assert_eq!(iter.value(), None); - - let valid = iter.prev(); - - assert_eq!(valid, true); assert_eq!(iter.valid(), true); assert_eq!(iter.key(), Some(b"k4".to_vec())); assert_eq!(iter.value(), Some(b"v4".to_vec())); - let valid = iter.prev(); + iter.prev(); - assert_eq!(valid, true); assert_eq!(iter.valid(), true); assert_eq!(iter.key(), Some(b"k3".to_vec())); assert_eq!(iter.value(), Some(b"v3".to_vec())); iter.prev(); // k2 iter.prev(); // k1 + iter.prev(); // invalid! - let valid = iter.prev(); - - // Should be invalid again - assert_eq!(valid, false); assert_eq!(iter.valid(), false); assert_eq!(iter.key(), None); assert_eq!(iter.value(), None); @@ -127,41 +103,6 @@ pub fn test_seek() { let mut iter = db.raw_iterator(); iter.seek(b"k2"); - // Shouldn't be valid yet - assert_eq!(iter.valid(), false); - assert_eq!(iter.key(), None); - assert_eq!(iter.value(), None); - - let valid = iter.next(); - - // Should now be valid - assert_eq!(valid, true); - assert_eq!(iter.valid(), true); - assert_eq!(iter.key(), Some(b"k2".to_vec())); - assert_eq!(iter.value(), Some(b"v2".to_vec())); -} - - -#[test] -pub fn test_seek_then_prev() { - let db = setup_test_db("seek_then_prev"); - db.put(b"k1", b"v1").unwrap(); - db.put(b"k2", b"v2").unwrap(); - db.put(b"k3", b"v3").unwrap(); - db.put(b"k4", b"v4").unwrap(); - - let mut iter = db.raw_iterator(); - iter.seek(b"k2"); - - // Shouldn't be valid yet - assert_eq!(iter.valid(), false); - assert_eq!(iter.key(), None); - assert_eq!(iter.value(), None); - - let valid = iter.prev(); - - // Should now be valid - assert_eq!(valid, true); assert_eq!(iter.valid(), true); assert_eq!(iter.key(), Some(b"k2".to_vec())); assert_eq!(iter.value(), Some(b"v2".to_vec())); @@ -178,14 +119,6 @@ pub fn test_seek_to_nonexistant() { let mut iter = db.raw_iterator(); iter.seek(b"k2"); - // Shouldn't be valid yet - assert_eq!(iter.valid(), false); - assert_eq!(iter.key(), None); - assert_eq!(iter.value(), None); - - let valid = iter.next(); - - assert_eq!(valid, true); assert_eq!(iter.valid(), true); assert_eq!(iter.key(), Some(b"k3".to_vec())); assert_eq!(iter.value(), Some(b"v3".to_vec()));