Skip to content

Commit

Permalink
chore: make Cursor iterators depend on the cursor's lifetime (#5479)
Browse files Browse the repository at this point in the history
  • Loading branch information
DaniPopes authored Nov 17, 2023
1 parent db5d01e commit 7162228
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 55 deletions.
30 changes: 9 additions & 21 deletions crates/storage/libmdbx-rs/src/codec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@ use derive_more::*;
use std::{borrow::Cow, slice};

/// Implement this to be able to decode data values
pub trait TableObject {
pub trait TableObject: Sized {
/// Decodes the object from the given bytes.
fn decode(data_val: &[u8]) -> Result<Self, Error>
where
Self: Sized;
fn decode(data_val: &[u8]) -> Result<Self, Error>;

/// Decodes the value directly from the given MDBX_val pointer.
///
Expand All @@ -17,14 +15,13 @@ pub trait TableObject {
#[doc(hidden)]
unsafe fn decode_val<K: TransactionKind>(
_: *const ffi::MDBX_txn,
data_val: &ffi::MDBX_val,
data_val: ffi::MDBX_val,
) -> Result<Self, Error>
where
Self: Sized,
{
let s = slice::from_raw_parts(data_val.iov_base as *const u8, data_val.iov_len);

TableObject::decode(s)
Self::decode(s)
}
}

Expand All @@ -36,7 +33,7 @@ impl<'tx> TableObject for Cow<'tx, [u8]> {
#[doc(hidden)]
unsafe fn decode_val<K: TransactionKind>(
_txn: *const ffi::MDBX_txn,
data_val: &ffi::MDBX_val,
data_val: ffi::MDBX_val,
) -> Result<Self, Error> {
let s = slice::from_raw_parts(data_val.iov_base as *const u8, data_val.iov_len);

Expand All @@ -56,10 +53,7 @@ impl<'tx> TableObject for Cow<'tx, [u8]> {
}

impl TableObject for Vec<u8> {
fn decode(data_val: &[u8]) -> Result<Self, Error>
where
Self: Sized,
{
fn decode(data_val: &[u8]) -> Result<Self, Error> {
Ok(data_val.to_vec())
}
}
Expand All @@ -71,7 +65,7 @@ impl TableObject for () {

unsafe fn decode_val<K: TransactionKind>(
_: *const ffi::MDBX_txn,
_: &ffi::MDBX_val,
_: ffi::MDBX_val,
) -> Result<Self, Error> {
Ok(())
}
Expand All @@ -82,19 +76,13 @@ impl TableObject for () {
pub struct ObjectLength(pub usize);

impl TableObject for ObjectLength {
fn decode(data_val: &[u8]) -> Result<Self, Error>
where
Self: Sized,
{
fn decode(data_val: &[u8]) -> Result<Self, Error> {
Ok(Self(data_val.len()))
}
}

impl<const LEN: usize> TableObject for [u8; LEN] {
fn decode(data_val: &[u8]) -> Result<Self, Error>
where
Self: Sized,
{
fn decode(data_val: &[u8]) -> Result<Self, Error> {
if data_val.len() != LEN {
return Err(Error::DecodeErrorLenDiff)
}
Expand Down
58 changes: 26 additions & 32 deletions crates/storage/libmdbx-rs/src/cursor.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
use std::{borrow::Cow, fmt, marker::PhantomData, mem, ptr};

use libc::c_void;

use ffi::{
MDBX_cursor_op, MDBX_FIRST, MDBX_FIRST_DUP, MDBX_GET_BOTH, MDBX_GET_BOTH_RANGE,
MDBX_GET_CURRENT, MDBX_GET_MULTIPLE, MDBX_LAST, MDBX_LAST_DUP, MDBX_NEXT, MDBX_NEXT_DUP,
MDBX_NEXT_MULTIPLE, MDBX_NEXT_NODUP, MDBX_PREV, MDBX_PREV_DUP, MDBX_PREV_MULTIPLE,
MDBX_PREV_NODUP, MDBX_SET, MDBX_SET_KEY, MDBX_SET_LOWERBOUND, MDBX_SET_RANGE,
};

use crate::{
error::{mdbx_result, Error, Result},
flags::*,
mdbx_try_optional,
transaction::{TransactionKind, RW},
TableObject, Transaction,
};
use ffi::{
MDBX_cursor_op, MDBX_FIRST, MDBX_FIRST_DUP, MDBX_GET_BOTH, MDBX_GET_BOTH_RANGE,
MDBX_GET_CURRENT, MDBX_GET_MULTIPLE, MDBX_LAST, MDBX_LAST_DUP, MDBX_NEXT, MDBX_NEXT_DUP,
MDBX_NEXT_MULTIPLE, MDBX_NEXT_NODUP, MDBX_PREV, MDBX_PREV_DUP, MDBX_PREV_MULTIPLE,
MDBX_PREV_NODUP, MDBX_SET, MDBX_SET_KEY, MDBX_SET_LOWERBOUND, MDBX_SET_RANGE,
};
use libc::c_void;
use std::{borrow::Cow, fmt, marker::PhantomData, mem, ptr};

/// A cursor for navigating the items within a database.
pub struct Cursor<K>
Expand Down Expand Up @@ -60,22 +57,20 @@ where
self.cursor
}

/// Returns an Iterator over the raw key value slices
///
/// Note: The lifetime ensures that the transaction is kept alive while entries are used
pub fn into_iter_slices<'cur>(self) -> IntoIter<'cur, K, Cow<'cur, [u8]>, Cow<'cur, [u8]>> {
/// Returns an iterator over the raw key value slices.
#[allow(clippy::needless_lifetimes)]
pub fn iter_slices<'a>(&'a self) -> IntoIter<'a, K, Cow<'a, [u8]>, Cow<'a, [u8]>> {
self.into_iter()
}
/// Returns an Iterator over key value pairs of the cursor
///
/// Note: The lifetime ensures that the transaction is kept alive while entries are used

/// Returns an iterator over database items.
#[allow(clippy::should_implement_trait)]
pub fn into_iter<'cur, Key, Value>(self) -> IntoIter<'cur, K, Key, Value>
pub fn into_iter<Key, Value>(&self) -> IntoIter<'_, K, Key, Value>
where
Key: TableObject,
Value: TableObject,
{
IntoIter::new(self, MDBX_NEXT, MDBX_NEXT)
IntoIter::new(self.clone(), MDBX_NEXT, MDBX_NEXT)
}

/// Retrieves a key/data pair from the cursor. Depending on the cursor op,
Expand Down Expand Up @@ -106,12 +101,12 @@ where
let key_out = {
// MDBX wrote in new key
if key_ptr != key_val.iov_base {
Some(Key::decode_val::<K>(txn, &key_val)?)
Some(Key::decode_val::<K>(txn, key_val)?)
} else {
None
}
};
let data_out = Value::decode_val::<K>(txn, &data_val)?;
let data_out = Value::decode_val::<K>(txn, data_val)?;
Ok((key_out, data_out, v))
})
}
Expand Down Expand Up @@ -335,16 +330,16 @@ where
Ok(Some((found, k.unwrap(), v)))
}

/// Iterate over database items. The iterator will begin with item next
/// after the cursor, and continue until the end of the database. For new
/// cursors, the iterator will begin with the first item in the database.
/// Returns an iterator over database items.
///
/// The iterator will begin with item next after the cursor, and continue until the end of the
/// database. For new cursors, the iterator will begin with the first item in the database.
///
/// For databases with duplicate data items ([DatabaseFlags::DUP_SORT]), the
/// duplicate data items of each key will be returned before moving on to
/// the next key.
pub fn iter<Key, Value>(&mut self) -> Iter<'_, K, Key, Value>
where
Self: Sized,
Key: TableObject,
Value: TableObject,
{
Expand All @@ -358,7 +353,6 @@ where
/// the next key.
pub fn iter_start<Key, Value>(&mut self) -> Iter<'_, K, Key, Value>
where
Self: Sized,
Key: TableObject,
Value: TableObject,
{
Expand Down Expand Up @@ -536,7 +530,7 @@ where
/// The next and subsequent operations to perform.
next_op: ffi::MDBX_cursor_op,

_marker: PhantomData<fn(&'cur (), K, Key, Value)>,
_marker: PhantomData<(&'cur (), Key, Value)>,
},
}

Expand Down Expand Up @@ -570,11 +564,11 @@ where
cursor.txn.txn_execute(|txn| {
match ffi::mdbx_cursor_get(cursor.cursor(), &mut key, &mut data, op) {
ffi::MDBX_SUCCESS => {
let key = match Key::decode_val::<K>(txn, &key) {
let key = match Key::decode_val::<K>(txn, key) {
Ok(v) => v,
Err(e) => return Some(Err(e)),
};
let data = match Value::decode_val::<K>(txn, &data) {
let data = match Value::decode_val::<K>(txn, data) {
Ok(v) => v,
Err(e) => return Some(Err(e)),
};
Expand Down Expand Up @@ -661,11 +655,11 @@ where
cursor.txn.txn_execute(|txn| {
match ffi::mdbx_cursor_get(cursor.cursor(), &mut key, &mut data, op) {
ffi::MDBX_SUCCESS => {
let key = match Key::decode_val::<K>(txn, &key) {
let key = match Key::decode_val::<K>(txn, key) {
Ok(v) => v,
Err(e) => return Some(Err(e)),
};
let data = match Value::decode_val::<K>(txn, &data) {
let data = match Value::decode_val::<K>(txn, data) {
Ok(v) => v,
Err(e) => return Some(Err(e)),
};
Expand Down
2 changes: 1 addition & 1 deletion crates/storage/libmdbx-rs/src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ impl Environment {
let db = Database::freelist_db();
let cursor = txn.cursor(&db)?;

for result in cursor.into_iter_slices() {
for result in cursor.iter_slices() {
let (_key, value) = result?;
if value.len() < size_of::<usize>() {
return Err(Error::Corrupted)
Expand Down
2 changes: 1 addition & 1 deletion crates/storage/libmdbx-rs/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ where

self.txn_execute(|txn| unsafe {
match ffi::mdbx_get(txn, dbi, &key_val, &mut data_val) {
ffi::MDBX_SUCCESS => Key::decode_val::<K>(txn, &data_val).map(Some),
ffi::MDBX_SUCCESS => Key::decode_val::<K>(txn, data_val).map(Some),
ffi::MDBX_NOTFOUND => Ok(None),
err_code => Err(Error::from_err_code(err_code)),
}
Expand Down

0 comments on commit 7162228

Please sign in to comment.