Simplify empty database iter handling

This commit changes the API of Cursor::iter_dup_of, and is thus a
breaking change.
without.crypto
Dan Burkert 6 years ago
parent c616e3d51e
commit c64f0b4f82
  1. 99
      src/cursor.rs

@ -1,6 +1,7 @@
use libc::{c_void, size_t, c_uint};
use std::{fmt, ptr, result, slice};
use std::marker::PhantomData; use std::marker::PhantomData;
use std::{fmt, mem, ptr, result, slice};
use libc::{EINVAL, c_void, size_t, c_uint};
use database::Database; use database::Database;
use error::{Error, Result, lmdb_result}; use error::{Error, Result, lmdb_result};
@ -19,11 +20,7 @@ pub trait Cursor<'txn> {
/// Retrieves a key/data pair from the cursor. Depending on the cursor op, /// Retrieves a key/data pair from the cursor. Depending on the cursor op,
/// the current key may be returned. /// the current key may be returned.
fn get(&self, fn get(&self, key: Option<&[u8]>, data: Option<&[u8]>, op: c_uint) -> Result<(Option<&'txn [u8]>, &'txn [u8])> {
key: Option<&[u8]>,
data: Option<&[u8]>,
op: c_uint)
-> Result<(Option<&'txn [u8]>, &'txn [u8])> {
unsafe { unsafe {
let mut key_val = slice_to_val(key); let mut key_val = slice_to_val(key);
let mut data_val = slice_to_val(data); let mut data_val = slice_to_val(data);
@ -52,14 +49,7 @@ pub trait Cursor<'txn> {
/// duplicate data items of each key will be returned before moving on to /// duplicate data items of each key will be returned before moving on to
/// the next key. /// the next key.
fn iter_start(&mut self) -> Iter<'txn> { fn iter_start(&mut self) -> Iter<'txn> {
// When the db is empty, this returns NotFound, which means the iterator should not even Iter::new(self.cursor(), ffi::MDB_FIRST, ffi::MDB_NEXT)
// try to proceed or it too will error with code 22
let complete = self.get(None, None, ffi::MDB_FIRST)
.map(|_| false)
.unwrap_or(true);
let mut iter = Iter::new(self.cursor(), ffi::MDB_GET_CURRENT, ffi::MDB_NEXT);
iter.complete = complete;
iter
} }
/// Iterate over database items starting from the given key. /// Iterate over database items starting from the given key.
@ -69,10 +59,9 @@ pub trait Cursor<'txn> {
/// the next key. /// the next key.
fn iter_from<K>(&mut self, key: K) -> Iter<'txn> where K: AsRef<[u8]> { fn iter_from<K>(&mut self, key: K) -> Iter<'txn> where K: AsRef<[u8]> {
match self.get(Some(key.as_ref()), None, ffi::MDB_SET_RANGE) { match self.get(Some(key.as_ref()), None, ffi::MDB_SET_RANGE) {
Err(Error::NotFound) => Ok(()), Ok(_) | Err(Error::NotFound) => (),
Err(error) => Err(error), Err(error) => panic!("mdb_cursor_get returned an unexpected error: {}", error),
Ok(_) => Ok(()), };
}.unwrap();
Iter::new(self.cursor(), ffi::MDB_GET_CURRENT, ffi::MDB_NEXT) Iter::new(self.cursor(), ffi::MDB_GET_CURRENT, ffi::MDB_NEXT)
} }
@ -86,27 +75,26 @@ pub trait Cursor<'txn> {
/// Iterate over duplicate database items starting from the beginning of the /// Iterate over duplicate database items starting from the beginning of the
/// database. Each item will be returned as an iterator of its duplicates. /// database. Each item will be returned as an iterator of its duplicates.
fn iter_dup_start(&mut self) -> IterDup<'txn> { fn iter_dup_start(&mut self) -> IterDup<'txn> {
self.get(None, None, ffi::MDB_FIRST).unwrap(); IterDup::new(self.cursor(), ffi::MDB_FIRST)
IterDup::new(self.cursor(), ffi::MDB_GET_CURRENT)
} }
/// Iterate over duplicate items in the database starting from the given /// Iterate over duplicate items in the database starting from the given
/// key. Each item will be returned as an iterator of its duplicates. /// key. Each item will be returned as an iterator of its duplicates.
fn iter_dup_from<K>(&mut self, key: &K) -> IterDup<'txn> where K: AsRef<[u8]> { fn iter_dup_from<K>(&mut self, key: &K) -> IterDup<'txn> where K: AsRef<[u8]> {
match self.get(Some(key.as_ref()), None, ffi::MDB_SET_RANGE) { match self.get(Some(key.as_ref()), None, ffi::MDB_SET_RANGE) {
Err(Error::NotFound) => Ok(()), Ok(_) | Err(Error::NotFound) => (),
Err(error) => Err(error), Err(error) => panic!("mdb_cursor_get returned an unexpected error: {}", error),
Ok(_) => Ok(()), };
}.unwrap();
IterDup::new(self.cursor(), ffi::MDB_GET_CURRENT) IterDup::new(self.cursor(), ffi::MDB_GET_CURRENT)
} }
/// Iterate over the duplicates of the item in the database with the given /// Iterate over the duplicates of the item in the database with the given key.
/// key. fn iter_dup_of<K>(&mut self, key: &K) -> Iter<'txn> where K: AsRef<[u8]> {
fn iter_dup_of<K>(&mut self, key: &K) -> Result<Iter<'txn>> where K: match self.get(Some(key.as_ref()), None, ffi::MDB_SET) {
AsRef<[u8]> { Ok(_) | Err(Error::NotFound) => (),
self.get(Some(key.as_ref()), None, ffi::MDB_SET)?; Err(error) => panic!("mdb_cursor_get returned an unexpected error: {}", error),
Ok(Iter::new(self.cursor(), ffi::MDB_GET_CURRENT, ffi::MDB_NEXT_DUP)) };
Iter::new(self.cursor(), ffi::MDB_GET_CURRENT, ffi::MDB_NEXT_DUP)
} }
} }
@ -231,7 +219,6 @@ pub struct Iter<'txn> {
cursor: *mut ffi::MDB_cursor, cursor: *mut ffi::MDB_cursor,
op: c_uint, op: c_uint,
next_op: c_uint, next_op: c_uint,
complete: bool,
_marker: PhantomData<fn(&'txn ())>, _marker: PhantomData<fn(&'txn ())>,
} }
@ -239,7 +226,7 @@ impl <'txn> Iter<'txn> {
/// Creates a new iterator backed by the given cursor. /// Creates a new iterator backed by the given cursor.
fn new<'t>(cursor: *mut ffi::MDB_cursor, op: c_uint, next_op: c_uint) -> Iter<'t> { fn new<'t>(cursor: *mut ffi::MDB_cursor, op: c_uint, next_op: c_uint) -> Iter<'t> {
Iter { cursor: cursor, op: op, next_op: next_op, complete: false, _marker: PhantomData } Iter { cursor: cursor, op: op, next_op: next_op, _marker: PhantomData }
} }
} }
@ -254,26 +241,16 @@ impl <'txn> Iterator for Iter<'txn> {
type Item = (&'txn [u8], &'txn [u8]); type Item = (&'txn [u8], &'txn [u8]);
fn next(&mut self) -> Option<(&'txn [u8], &'txn [u8])> { fn next(&mut self) -> Option<(&'txn [u8], &'txn [u8])> {
if self.complete {
return None
}
let mut key = ffi::MDB_val { mv_size: 0, mv_data: ptr::null_mut() }; let mut key = ffi::MDB_val { mv_size: 0, mv_data: ptr::null_mut() };
let mut data = ffi::MDB_val { mv_size: 0, mv_data: ptr::null_mut() }; let mut data = ffi::MDB_val { mv_size: 0, mv_data: ptr::null_mut() };
let op = mem::replace(&mut self.op, self.next_op);
unsafe { unsafe {
let err_code = ffi::mdb_cursor_get(self.cursor, &mut key, &mut data, self.op); match ffi::mdb_cursor_get(self.cursor, &mut key, &mut data, op) {
// Set the operation for the next get ffi::MDB_SUCCESS => Some((val_to_slice(key), val_to_slice(data))),
self.op = self.next_op; // EINVAL can occur when the cursor was previously seeked to a non-existent value,
if err_code == ffi::MDB_SUCCESS { // e.g. iter_from with a key greater than all values in the database.
Some((val_to_slice(key), val_to_slice(data))) ffi::MDB_NOTFOUND | EINVAL => None,
} else { error => panic!("mdb_cursor_get returned an unexpected error: {}", error),
// The documentation for mdb_cursor_get specifies that it may fail with MDB_NOTFOUND
// and MDB_EINVAL (and we shouldn't be passing in invalid parameters).
// TODO: validate that these are the only failures possible.
debug_assert!(err_code == ffi::MDB_NOTFOUND,
"Unexpected LMDB error {:?}.", Error::from_err_code(err_code));
self.complete = true;
None
} }
} }
} }
@ -310,12 +287,12 @@ impl <'txn> Iterator for IterDup<'txn> {
fn next(&mut self) -> Option<Iter<'txn>> { fn next(&mut self) -> Option<Iter<'txn>> {
let mut key = ffi::MDB_val { mv_size: 0, mv_data: ptr::null_mut() }; let mut key = ffi::MDB_val { mv_size: 0, mv_data: ptr::null_mut() };
let mut data = ffi::MDB_val { mv_size: 0, mv_data: ptr::null_mut() }; let mut data = ffi::MDB_val { mv_size: 0, mv_data: ptr::null_mut() };
let op = mem::replace(&mut self.op, ffi::MDB_NEXT_NODUP);
let err_code = unsafe { let err_code = unsafe {
ffi::mdb_cursor_get(self.cursor, &mut key, &mut data, self.op) ffi::mdb_cursor_get(self.cursor, &mut key, &mut data, op)
}; };
if err_code == ffi::MDB_SUCCESS { if err_code == ffi::MDB_SUCCESS {
self.op = ffi::MDB_NEXT;
Some(Iter::new(self.cursor, ffi::MDB_GET_CURRENT, ffi::MDB_NEXT_DUP)) Some(Iter::new(self.cursor, ffi::MDB_GET_CURRENT, ffi::MDB_NEXT_DUP))
} else { } else {
None None
@ -475,25 +452,33 @@ mod test {
} }
#[test] #[test]
fn test_iter_start_empty() { fn test_iter_empty_database() {
let dir = TempDir::new("test").unwrap(); let dir = TempDir::new("test").unwrap();
let env = Environment::new().open(dir.path()).unwrap(); let env = Environment::new().open(dir.path()).unwrap();
let db = env.open_db(None).unwrap(); let db = env.open_db(None).unwrap();
let txn = env.begin_ro_txn().unwrap(); let txn = env.begin_ro_txn().unwrap();
let mut cursor = txn.open_ro_cursor(db).unwrap(); let mut cursor = txn.open_ro_cursor(db).unwrap();
assert_eq!(0, cursor.iter().count());
assert_eq!(0, cursor.iter_start().count()); assert_eq!(0, cursor.iter_start().count());
assert_eq!(0, cursor.iter_from(b"foo").count());
} }
#[test] #[test]
fn test_iter_empty() { fn test_iter_empty_dup_database() {
let dir = TempDir::new("test").unwrap(); let dir = TempDir::new("test").unwrap();
let env = Environment::new().open(dir.path()).unwrap(); let env = Environment::new().open(dir.path()).unwrap();
let db = env.open_db(None).unwrap(); let db = env.create_db(None, DatabaseFlags::DUP_SORT).unwrap();
let txn = env.begin_ro_txn().unwrap(); let txn = env.begin_ro_txn().unwrap();
let mut cursor = txn.open_ro_cursor(db).unwrap(); let mut cursor = txn.open_ro_cursor(db).unwrap();
assert_eq!(0, cursor.iter().count()); assert_eq!(0, cursor.iter().count());
assert_eq!(0, cursor.iter_start().count());
assert_eq!(0, cursor.iter_from(b"foo").count());
assert_eq!(0, cursor.iter_dup().count());
assert_eq!(0, cursor.iter_dup_start().count());
assert_eq!(0, cursor.iter_dup_from(b"foo").count());
assert_eq!(0, cursor.iter_dup_of(b"foo").count());
} }
#[test] #[test]
@ -547,9 +532,9 @@ mod test {
cursor.iter_dup_from(b"f").flat_map(|x| x).collect::<Vec<_>>()); cursor.iter_dup_from(b"f").flat_map(|x| x).collect::<Vec<_>>());
assert_eq!(items.clone().into_iter().skip(3).take(3).collect::<Vec<(&[u8], &[u8])>>(), assert_eq!(items.clone().into_iter().skip(3).take(3).collect::<Vec<(&[u8], &[u8])>>(),
cursor.iter_dup_of(b"b").unwrap().collect::<Vec<_>>()); cursor.iter_dup_of(b"b").collect::<Vec<_>>());
assert!(cursor.iter_dup_of(b"foo").is_err()); assert_eq!(0, cursor.iter_dup_of(b"foo").count());
} }
#[test] #[test]

Loading…
Cancel
Save