From 3063b2f72b0573d773bf23d8e408ba3209eeaae7 Mon Sep 17 00:00:00 2001 From: Christopher Berner Date: Sat, 20 Jan 2024 10:26:30 -0800 Subject: [PATCH 1/2] Cleanup some lifetimes --- src/tree_store/btree.rs | 13 +++++-------- src/tree_store/btree_iters.rs | 17 +++++++---------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/tree_store/btree.rs b/src/tree_store/btree.rs index 7495c85b..f82238a9 100644 --- a/src/tree_store/btree.rs +++ b/src/tree_store/btree.rs @@ -217,7 +217,7 @@ impl UntypedBtreeMut { } } -pub(crate) struct BtreeMut<'a, K: RedbKey, V: RedbValue> { +pub(crate) struct BtreeMut<'a, K: RedbKey + 'static, V: RedbValue + 'static> { mem: Arc, transaction_guard: Arc, root: Arc>>, @@ -227,7 +227,7 @@ pub(crate) struct BtreeMut<'a, K: RedbKey, V: RedbValue> { _lifetime: PhantomData<&'a ()>, } -impl<'a, K: RedbKey + 'a, V: RedbValue + 'a> BtreeMut<'a, K, V> { +impl BtreeMut<'_, K, V> { pub(crate) fn new( root: Option<(PageNumber, Checksum)>, guard: Arc, @@ -552,7 +552,7 @@ impl RawBtree { } } -pub(crate) struct Btree { +pub(crate) struct Btree { mem: Arc, _transaction_guard: Arc, // Cache of the root page to avoid repeated lookups @@ -621,13 +621,10 @@ impl Btree { } } - pub(crate) fn range<'a0, T: RangeBounds + 'a0, KR: Borrow> + 'a0>( + pub(crate) fn range<'a0, T: RangeBounds, KR: Borrow>>( &self, range: &'_ T, - ) -> Result> - where - K: 'a0, - { + ) -> Result> { BtreeRangeIter::new(range, self.root.map(|(p, _)| p), self.mem.clone()) } diff --git a/src/tree_store/btree_iters.rs b/src/tree_store/btree_iters.rs index a2b6843b..f08e4399 100644 --- a/src/tree_store/btree_iters.rs +++ b/src/tree_store/btree_iters.rs @@ -243,7 +243,7 @@ impl Iterator for AllPageNumbersBtreeIter { } } -pub(crate) struct BtreeDrain { +pub(crate) struct BtreeDrain { inner: BtreeRangeIter, free_on_drop: Vec, master_free_list: Arc>>, @@ -298,8 +298,8 @@ impl Drop for BtreeDrain { } pub(crate) struct BtreeDrainFilter< - K: RedbKey, - V: RedbValue, + K: RedbKey + 'static, + V: RedbValue + 'static, F: for<'f> FnMut(K::SelfType<'f>, V::SelfType<'f>) -> bool, > { inner: BtreeRangeIter, @@ -380,7 +380,7 @@ impl FnMut(K::SelfType<'f>, V::SelfType<'f> } } -pub(crate) struct BtreeRangeIter { +pub(crate) struct BtreeRangeIter { left: Option, // Exclusive. The previous element returned right: Option, // Exclusive. The previous element returned include_left: bool, // left is inclusive, instead of exclusive @@ -390,15 +390,12 @@ pub(crate) struct BtreeRangeIter { _value_type: PhantomData, } -impl BtreeRangeIter { - pub(crate) fn new<'a0, T: RangeBounds + 'a0, KR: Borrow> + 'a0>( +impl BtreeRangeIter { + pub(crate) fn new<'a, T: RangeBounds, KR: Borrow>>( query_range: &'_ T, table_root: Option, manager: Arc, - ) -> Result - where - K: 'a0, - { + ) -> Result { if let Some(root) = table_root { let (include_left, left) = match query_range.start_bound() { Bound::Included(k) => find_iter_left::( From 54051c6f426e7caca9ede0d86e47710d899048a4 Mon Sep 17 00:00:00 2001 From: Christopher Berner Date: Thu, 21 Dec 2023 09:18:33 -0800 Subject: [PATCH 2/2] Add static lifetime versions of range() and multimap table get() These methods return reference counted iterators from read only transactions --- src/db.rs | 3 +-- src/multimap_table.rs | 29 +++++++++++++++++++++ src/table.rs | 11 ++++++++ tests/basic_tests.rs | 24 +++++++++++++++++ tests/multimap_tests.rs | 58 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 123 insertions(+), 2 deletions(-) diff --git a/src/db.rs b/src/db.rs index e6606a3e..ab70a1da 100644 --- a/src/db.rs +++ b/src/db.rs @@ -6,8 +6,7 @@ use crate::tree_store::{ }; use crate::types::{RedbKey, RedbValue}; use crate::{ - CompactionError, DatabaseError, Durability, ReadOnlyTable, ReadableTable, SavepointError, - StorageError, + CompactionError, DatabaseError, Durability, ReadOnlyTable, SavepointError, StorageError, }; use crate::{ReadTransaction, Result, WriteTransaction}; use std::fmt::{Debug, Display, Formatter}; diff --git a/src/multimap_table.rs b/src/multimap_table.rs index 5eb6cc5c..cf62121e 100644 --- a/src/multimap_table.rs +++ b/src/multimap_table.rs @@ -1296,6 +1296,35 @@ impl<'txn, K: RedbKey + 'static, V: RedbKey + 'static> ReadOnlyMultimapTable<'tx _lifetime: Default::default(), }) } + + /// This method is like [`ReadableMultimapTable::get()`], but the iterator is reference counted and keeps the transaction + /// alive until it is dropped. + pub fn get<'a>(&self, key: impl Borrow>) -> Result> { + let iter = if let Some(collection) = self.tree.get(key.borrow())? { + DynamicCollection::iter(collection, self.transaction_guard.clone(), self.mem.clone())? + } else { + MultimapValue::new_subtree( + BtreeRangeIter::new::>(&(..), None, self.mem.clone())?, + self.transaction_guard.clone(), + ) + }; + + Ok(iter) + } + + /// This method is like [`ReadableMultimapTable::range()`], but the iterator is reference counted and keeps the transaction + /// alive until it is dropped. + pub fn range<'a, KR>(&self, range: impl RangeBounds) -> Result> + where + KR: Borrow>, + { + let inner = self.tree.range(&range)?; + Ok(MultimapRange::new( + inner, + self.transaction_guard.clone(), + self.mem.clone(), + )) + } } impl<'txn, K: RedbKey + 'static, V: RedbKey + 'static> ReadableMultimapTable diff --git a/src/table.rs b/src/table.rs index 2541df77..061dc69b 100644 --- a/src/table.rs +++ b/src/table.rs @@ -439,6 +439,17 @@ impl<'txn, K: RedbKey + 'static, V: RedbValue + 'static> ReadOnlyTable<'txn, K, _lifetime: Default::default(), }) } + + /// This method is like [`ReadableTable::range()`], but the iterator is reference counted and keeps the transaction + /// alive until it is dropped. + pub fn range<'a, KR>(&self, range: impl RangeBounds) -> Result> + where + KR: Borrow>, + { + self.tree + .range(&range) + .map(|x| Range::new(x, self.transaction_guard.clone())) + } } impl<'txn, K: RedbKey + 'static, V: RedbValue + 'static> ReadableTable diff --git a/tests/basic_tests.rs b/tests/basic_tests.rs index 4138c832..490b0bdb 100644 --- a/tests/basic_tests.rs +++ b/tests/basic_tests.rs @@ -1246,6 +1246,30 @@ fn range_lifetime() { assert!(iter.next().is_none()); } +#[test] +fn range_arc() { + let tmpfile = create_tempfile(); + let db = Database::create(tmpfile.path()).unwrap(); + + let definition: TableDefinition<&str, &str> = TableDefinition::new("x"); + + let write_txn = db.begin_write().unwrap(); + { + let mut table = write_txn.open_table(definition).unwrap(); + table.insert("hello", "world").unwrap(); + } + write_txn.commit().unwrap(); + + let mut iter = { + let read_txn = db.begin_read().unwrap(); + let table = read_txn.open_table(definition).unwrap(); + let start = "hello".to_string(); + table.range::<&str>(start.as_str()..).unwrap() + }; + assert_eq!(iter.next().unwrap().unwrap().1.value(), "world"); + assert!(iter.next().is_none()); +} + #[test] fn drain_lifetime() { let tmpfile = create_tempfile(); diff --git a/tests/multimap_tests.rs b/tests/multimap_tests.rs index 317096b8..45ac8586 100644 --- a/tests/multimap_tests.rs +++ b/tests/multimap_tests.rs @@ -172,6 +172,64 @@ fn range_lifetime() { assert!(iter.next().is_none()); } +#[test] +fn range_arc_lifetime() { + let tmpfile = create_tempfile(); + let db = Database::create(tmpfile.path()).unwrap(); + + let definition: MultimapTableDefinition<&str, &str> = MultimapTableDefinition::new("x"); + + let write_txn = db.begin_write().unwrap(); + { + let mut table = write_txn.open_multimap_table(definition).unwrap(); + table.insert("hello", "world").unwrap(); + } + write_txn.commit().unwrap(); + + let mut iter = { + let read_txn = db.begin_read().unwrap(); + let table = read_txn.open_multimap_table(definition).unwrap(); + let start = "hello".to_string(); + table.range::<&str>(start.as_str()..).unwrap() + }; + assert_eq!( + iter.next() + .unwrap() + .unwrap() + .1 + .next() + .unwrap() + .unwrap() + .value(), + "world" + ); + assert!(iter.next().is_none()); +} + +#[test] +fn get_arc_lifetime() { + let tmpfile = create_tempfile(); + let db = Database::create(tmpfile.path()).unwrap(); + + let definition: MultimapTableDefinition<&str, &str> = MultimapTableDefinition::new("x"); + + let write_txn = db.begin_write().unwrap(); + { + let mut table = write_txn.open_multimap_table(definition).unwrap(); + table.insert("hello", "world").unwrap(); + } + write_txn.commit().unwrap(); + + let mut iter = { + let read_txn = db.begin_read().unwrap(); + let table = read_txn.open_multimap_table(definition).unwrap(); + let start = "hello".to_string(); + table.get(start.as_str()).unwrap() + }; + assert_eq!(iter.next().unwrap().unwrap().value(), "world"); + assert!(iter.next().is_none()); +} + #[test] fn delete() { let tmpfile = create_tempfile();