Move len(), is_empty(), and stats() to separate trait

cberner · Feb 4, 2024 · a4e6334 · a4e6334
1 parent 1cb0be6
commit a4e6334
Show file tree

Hide file tree

Showing 10 changed files with 129 additions and 138 deletions.
diff --git a/benches/multithreaded_insert_benchmark.rs b/benches/multithreaded_insert_benchmark.rs
@@ -4,7 +4,7 @@ use tempfile::NamedTempFile;
 
 use rand::rngs::StdRng;
 use rand::{Rng, SeedableRng};
-use redb::{Database, ReadableTable, TableDefinition};
+use redb::{Database, ReadableTableMetadata, TableDefinition};
 use std::time::Instant;
 
 const ELEMENTS: u64 = 1_000_000;

diff --git a/fuzz/fuzz_targets/fuzz_redb.rs b/fuzz/fuzz_targets/fuzz_redb.rs
@@ -1,7 +1,7 @@
 #![no_main]
 
 use libfuzzer_sys::fuzz_target;
-use redb::{AccessGuard, Database, Durability, Error, MultimapTable, MultimapTableDefinition, MultimapValue, ReadableMultimapTable, ReadableTable, Savepoint, StorageBackend, Table, TableDefinition, WriteTransaction};
+use redb::{AccessGuard, Database, Durability, Error, MultimapTable, MultimapTableDefinition, MultimapValue, ReadableMultimapTable, ReadableTable, ReadableTableMetadata, Savepoint, StorageBackend, Table, TableDefinition, WriteTransaction};
 use std::collections::{BTreeMap, BTreeSet, HashSet};
 use std::fmt::Debug;
 use std::io::{ErrorKind, Read, Seek, SeekFrom};

diff --git a/src/lib.rs b/src/lib.rs
@@ -66,8 +66,8 @@ pub use multimap_table::{
     ReadOnlyUntypedMultimapTable, ReadableMultimapTable,
 };
 pub use table::{
-    Drain, DrainFilter, Range, ReadOnlyTable, ReadOnlyUntypedTable, ReadableTable, Table,
-    TableStats,
+    Drain, DrainFilter, Range, ReadOnlyTable, ReadOnlyUntypedTable, ReadableTable,
+    ReadableTableMetadata, Table, TableStats,
 };
 pub use transactions::{DatabaseStats, Durability, ReadTransaction, WriteTransaction};
 pub use tree_store::{AccessGuard, AccessGuardMut, Savepoint};

diff --git a/src/multimap_table.rs b/src/multimap_table.rs
@@ -1,7 +1,7 @@
 use crate::db::TransactionGuard;
 use crate::multimap_table::DynamicCollectionType::{Inline, Subtree};
 use crate::sealed::Sealed;
-use crate::table::TableStats;
+use crate::table::{ReadableTableMetadata, TableStats};
 use crate::tree_store::{
     btree_stats, AllPageNumbersBtreeIter, BranchAccessor, Btree, BtreeMut, BtreeRangeIter,
     BtreeStats, CachePriority, Checksum, LeafAccessor, LeafMutator, Page, PageHint, PageNumber,
@@ -1099,6 +1099,39 @@ impl<'txn, K: Key + 'static, V: Key + 'static> MultimapTable<'txn, K, V> {
     }
 }
 
+impl<'txn, K: Key + 'static, V: Key + 'static> ReadableTableMetadata for MultimapTable<'txn, K, V> {
+    fn stats(&self) -> Result<TableStats> {
+        let tree_stats = multimap_btree_stats(
+            self.tree.get_root().map(|(p, _)| p),
+            &self.mem,
+            K::fixed_width(),
+            V::fixed_width(),
+        )?;
+
+        Ok(TableStats {
+            tree_height: tree_stats.tree_height,
+            leaf_pages: tree_stats.leaf_pages,
+            branch_pages: tree_stats.branch_pages,
+            stored_leaf_bytes: tree_stats.stored_leaf_bytes,
+            metadata_bytes: tree_stats.metadata_bytes,
+            fragmented_bytes: tree_stats.fragmented_bytes,
+        })
+    }
+
+    /// Returns the number of key-value pairs in the table
+    fn len(&self) -> Result<u64> {
+        let mut count = 0;
+        for item in self.iter()? {
+            let (_, values) = item?;
+            for v in values {
+                v?;
+                count += 1;
+            }
+        }
+        Ok(count)
+    }
+}
+
 impl<'txn, K: Key + 'static, V: Key + 'static> ReadableMultimapTable<K, V>
     for MultimapTable<'txn, K, V>
 {
@@ -1133,42 +1166,6 @@ impl<'txn, K: Key + 'static, V: Key + 'static> ReadableMultimapTable<K, V>
             self.mem.clone(),
         ))
     }
-
-    fn stats(&self) -> Result<TableStats> {
-        let tree_stats = multimap_btree_stats(
-            self.tree.get_root().map(|(p, _)| p),
-            &self.mem,
-            K::fixed_width(),
-            V::fixed_width(),
-        )?;
-
-        Ok(TableStats {
-            tree_height: tree_stats.tree_height,
-            leaf_pages: tree_stats.leaf_pages,
-            branch_pages: tree_stats.branch_pages,
-            stored_leaf_bytes: tree_stats.stored_leaf_bytes,
-            metadata_bytes: tree_stats.metadata_bytes,
-            fragmented_bytes: tree_stats.fragmented_bytes,
-        })
-    }
-
-    /// Returns the number of key-value pairs in the table
-    fn len(&self) -> Result<u64> {
-        let mut count = 0;
-        for item in self.iter()? {
-            let (_, values) = item?;
-            for v in values {
-                v?;
-                count += 1;
-            }
-        }
-        Ok(count)
-    }
-
-    /// Returns `true` if the table is empty
-    fn is_empty(&self) -> Result<bool> {
-        self.len().map(|x| x == 0)
-    }
 }
 
 impl<K: Key + 'static, V: Key + 'static> Sealed for MultimapTable<'_, K, V> {}
@@ -1179,7 +1176,7 @@ impl<'txn, K: Key + 'static, V: Key + 'static> Drop for MultimapTable<'txn, K, V
     }
 }
 
-pub trait ReadableMultimapTable<K: Key + 'static, V: Key + 'static>: Sealed {
+pub trait ReadableMultimapTable<K: Key + 'static, V: Key + 'static>: ReadableTableMetadata {
     /// Returns an iterator over all values for the given key. Values are in ascending order.
     fn get<'a>(&self, key: impl Borrow<K::SelfType<'a>>) -> Result<MultimapValue<V>>
     where
@@ -1190,13 +1187,6 @@ pub trait ReadableMultimapTable<K: Key + 'static, V: Key + 'static>: Sealed {
         K: 'a,
         KR: Borrow<K::SelfType<'a>> + 'a;
 
-    /// Retrieves information about storage usage for the table
-    fn stats(&self) -> Result<TableStats>;
-
-    fn len(&self) -> Result<u64>;
-
-    fn is_empty(&self) -> Result<bool>;
-
     /// Returns an double-ended iterator over all elements in the table. Values are in ascending
     /// order.
     fn iter(&self) -> Result<MultimapRange<K, V>> {
@@ -1305,6 +1295,38 @@ impl<K: Key + 'static, V: Key + 'static> ReadOnlyMultimapTable<K, V> {
     }
 }
 
+impl<K: Key + 'static, V: Key + 'static> ReadableTableMetadata for ReadOnlyMultimapTable<K, V> {
+    fn stats(&self) -> Result<TableStats> {
+        let tree_stats = multimap_btree_stats(
+            self.tree.get_root().map(|(p, _)| p),
+            &self.mem,
+            K::fixed_width(),
+            V::fixed_width(),
+        )?;
+
+        Ok(TableStats {
+            tree_height: tree_stats.tree_height,
+            leaf_pages: tree_stats.leaf_pages,
+            branch_pages: tree_stats.branch_pages,
+            stored_leaf_bytes: tree_stats.stored_leaf_bytes,
+            metadata_bytes: tree_stats.metadata_bytes,
+            fragmented_bytes: tree_stats.fragmented_bytes,
+        })
+    }
+
+    fn len(&self) -> Result<u64> {
+        let mut count = 0;
+        for item in self.iter()? {
+            let (_, values) = item?;
+            for v in values {
+                v?;
+                count += 1;
+            }
+        }
+        Ok(count)
+    }
+}
+
 impl<K: Key + 'static, V: Key + 'static> ReadableMultimapTable<K, V>
     for ReadOnlyMultimapTable<K, V>
 {
@@ -1337,40 +1359,6 @@ impl<K: Key + 'static, V: Key + 'static> ReadableMultimapTable<K, V>
             self.mem.clone(),
         ))
     }
-
-    fn stats(&self) -> Result<TableStats> {
-        let tree_stats = multimap_btree_stats(
-            self.tree.get_root().map(|(p, _)| p),
-            &self.mem,
-            K::fixed_width(),
-            V::fixed_width(),
-        )?;
-
-        Ok(TableStats {
-            tree_height: tree_stats.tree_height,
-            leaf_pages: tree_stats.leaf_pages,
-            branch_pages: tree_stats.branch_pages,
-            stored_leaf_bytes: tree_stats.stored_leaf_bytes,
-            metadata_bytes: tree_stats.metadata_bytes,
-            fragmented_bytes: tree_stats.fragmented_bytes,
-        })
-    }
-
-    fn len(&self) -> Result<u64> {
-        let mut count = 0;
-        for item in self.iter()? {
-            let (_, values) = item?;
-            for v in values {
-                v?;
-                count += 1;
-            }
-        }
-        Ok(count)
-    }
-
-    fn is_empty(&self) -> Result<bool> {
-        self.len().map(|x| x == 0)
-    }
 }
 
 impl<K: Key, V: Key> Sealed for ReadOnlyMultimapTable<K, V> {}
diff --git a/src/table.rs b/src/table.rs
@@ -215,24 +215,7 @@ impl<'txn, K: Key + 'static, V: MutInPlaceValue + 'static> Table<'txn, K, V> {
     }
 }
 
-impl<'txn, K: Key + 'static, V: Value + 'static> ReadableTable<K, V> for Table<'txn, K, V> {
-    fn get<'a>(&self, key: impl Borrow<K::SelfType<'a>>) -> Result<Option<AccessGuard<V>>>
-    where
-        K: 'a,
-    {
-        self.tree.get(key.borrow())
-    }
-
-    fn range<'a, KR>(&self, range: impl RangeBounds<KR> + 'a) -> Result<Range<K, V>>
-    where
-        K: 'a,
-        KR: Borrow<K::SelfType<'a>> + 'a,
-    {
-        self.tree
-            .range(&range)
-            .map(|x| Range::new(x, self.transaction.transaction_guard()))
-    }
-
+impl<'txn, K: Key + 'static, V: Value + 'static> ReadableTableMetadata for Table<'txn, K, V> {
     fn stats(&self) -> Result<TableStats> {
         let tree_stats = self.tree.stats()?;
 
@@ -249,9 +232,24 @@ impl<'txn, K: Key + 'static, V: Value + 'static> ReadableTable<K, V> for Table<'
     fn len(&self) -> Result<u64> {
         self.tree.len()
     }
+}
 
-    fn is_empty(&self) -> Result<bool> {
-        self.len().map(|x| x == 0)
+impl<'txn, K: Key + 'static, V: Value + 'static> ReadableTable<K, V> for Table<'txn, K, V> {
+    fn get<'a>(&self, key: impl Borrow<K::SelfType<'a>>) -> Result<Option<AccessGuard<V>>>
+    where
+        K: 'a,
+    {
+        self.tree.get(key.borrow())
+    }
+
+    fn range<'a, KR>(&self, range: impl RangeBounds<KR> + 'a) -> Result<Range<K, V>>
+    where
+        K: 'a,
+        KR: Borrow<K::SelfType<'a>> + 'a,
+    {
+        self.tree
+            .range(&range)
+            .map(|x| Range::new(x, self.transaction.transaction_guard()))
     }
 }
 
@@ -312,7 +310,20 @@ impl<K: Key + 'static, V: Value + 'static> Debug for Table<'_, K, V> {
     }
 }
 
-pub trait ReadableTable<K: Key + 'static, V: Value + 'static>: Sealed {
+pub trait ReadableTableMetadata: Sealed {
+    /// Retrieves information about storage usage for the table
+    fn stats(&self) -> Result<TableStats>;
+
+    /// Returns the number of entries in the table
+    fn len(&self) -> Result<u64>;
+
+    /// Returns `true` if the table is empty
+    fn is_empty(&self) -> Result<bool> {
+        Ok(self.len()? == 0)
+    }
+}
+
+pub trait ReadableTable<K: Key + 'static, V: Value + 'static>: ReadableTableMetadata {
     /// Returns the value corresponding to the given key
     fn get<'a>(&self, key: impl Borrow<K::SelfType<'a>>) -> Result<Option<AccessGuard<V>>>
     where
@@ -365,15 +376,6 @@ pub trait ReadableTable<K: Key + 'static, V: Value + 'static>: Sealed {
         self.iter()?.next_back().transpose()
     }
 
-    /// Retrieves information about storage usage for the table
-    fn stats(&self) -> Result<TableStats>;
-
-    /// Returns the number of entries in the table
-    fn len(&self) -> Result<u64>;
-
-    /// Returns `true` if the table is empty
-    fn is_empty(&self) -> Result<bool>;
-
     /// Returns a double-ended iterator over all elements in the table
     fn iter(&self) -> Result<Range<K, V>> {
         self.range::<K::SelfType<'_>>(..)
@@ -446,24 +448,7 @@ impl<K: Key + 'static, V: Value + 'static> ReadOnlyTable<K, V> {
     }
 }
 
-impl<K: Key + 'static, V: Value + 'static> ReadableTable<K, V> for ReadOnlyTable<K, V> {
-    fn get<'a>(&self, key: impl Borrow<K::SelfType<'a>>) -> Result<Option<AccessGuard<V>>>
-    where
-        K: 'a,
-    {
-        self.tree.get(key.borrow())
-    }
-
-    fn range<'a, KR>(&self, range: impl RangeBounds<KR> + 'a) -> Result<Range<K, V>>
-    where
-        K: 'a,
-        KR: Borrow<K::SelfType<'a>> + 'a,
-    {
-        self.tree
-            .range(&range)
-            .map(|x| Range::new(x, self.transaction_guard.clone()))
-    }
-
+impl<K: Key + 'static, V: Value + 'static> ReadableTableMetadata for ReadOnlyTable<K, V> {
     fn stats(&self) -> Result<TableStats> {
         let tree_stats = self.tree.stats()?;
 
@@ -480,9 +465,24 @@ impl<K: Key + 'static, V: Value + 'static> ReadableTable<K, V> for ReadOnlyTable
     fn len(&self) -> Result<u64> {
         self.tree.len()
     }
+}
 
-    fn is_empty(&self) -> Result<bool> {
-        self.len().map(|x| x == 0)
+impl<K: Key + 'static, V: Value + 'static> ReadableTable<K, V> for ReadOnlyTable<K, V> {
+    fn get<'a>(&self, key: impl Borrow<K::SelfType<'a>>) -> Result<Option<AccessGuard<V>>>
+    where
+        K: 'a,
+    {
+        self.tree.get(key.borrow())
+    }
+
+    fn range<'a, KR>(&self, range: impl RangeBounds<KR> + 'a) -> Result<Range<K, V>>
+    where
+        K: 'a,
+        KR: Borrow<K::SelfType<'a>> + 'a,
+    {
+        self.tree
+            .range(&range)
+            .map(|x| Range::new(x, self.transaction_guard.clone()))
     }
 }
 

diff --git a/tests/backward_compatibility.rs b/tests/backward_compatibility.rs
@@ -1,4 +1,4 @@
-use redb::ReadableTable;
+use redb::{ReadableTable, ReadableTableMetadata};
 
 const ELEMENTS: usize = 3;
 

diff --git a/tests/basic_tests.rs b/tests/basic_tests.rs
@@ -1,7 +1,7 @@
 use redb::backends::InMemoryBackend;
 use redb::{
     Database, Key, MultimapTableDefinition, MultimapTableHandle, Range, ReadableTable,
-    TableDefinition, TableError, TableHandle, TypeName, Value,
+    ReadableTableMetadata, TableDefinition, TableError, TableHandle, TypeName, Value,
 };
 use std::cmp::Ordering;
 #[cfg(not(target_os = "wasi"))]

diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs
@@ -4,7 +4,8 @@ use std::io::ErrorKind;
 use rand::prelude::SliceRandom;
 use rand::Rng;
 use redb::{
-    Builder, Database, Durability, MultimapTableDefinition, ReadableTable, TableDefinition,
+    Builder, Database, Durability, MultimapTableDefinition, ReadableTable, ReadableTableMetadata,
+    TableDefinition,
 };
 use redb::{DatabaseError, ReadableMultimapTable, SavepointError, StorageError, TableError};