diff --git a/src/multimap_table.rs b/src/multimap_table.rs index 62e000d6..0c8ce299 100644 --- a/src/multimap_table.rs +++ b/src/multimap_table.rs @@ -7,7 +7,7 @@ use crate::tree_store::{ RawBtree, RawLeafBuilder, TransactionalMemory, UntypedBtreeMut, BRANCH, LEAF, MAX_VALUE_LENGTH, }; use crate::types::{RedbKey, RedbValue, TypeName}; -use crate::{AccessGuard, Result, StorageError, WriteTransaction}; +use crate::{AccessGuard, MultimapTableHandle, Result, StorageError, WriteTransaction}; use std::borrow::Borrow; use std::cmp::max; use std::convert::TryInto; @@ -720,6 +720,14 @@ pub struct MultimapTable<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static> _value_type: PhantomData, } +impl MultimapTableHandle + for MultimapTable<'_, '_, K, V> +{ + fn name(&self) -> &str { + &self.name + } +} + impl<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static> MultimapTable<'db, 'txn, K, V> { pub(crate) fn new( name: &str, @@ -1120,7 +1128,7 @@ impl<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static> ReadableMultimapTabl } } -impl Sealed for MultimapTable<'_, '_, K, V> {} +impl Sealed for MultimapTable<'_, '_, K, V> {} impl<'db, 'txn, K: RedbKey + 'static, V: RedbKey + 'static> Drop for MultimapTable<'db, 'txn, K, V> diff --git a/src/table.rs b/src/table.rs index b5f1abee..441291b1 100644 --- a/src/table.rs +++ b/src/table.rs @@ -4,8 +4,8 @@ use crate::tree_store::{ PageHint, PageNumber, TransactionalMemory, MAX_VALUE_LENGTH, }; use crate::types::{RedbKey, RedbValue, RedbValueMutInPlace}; -use crate::Result; use crate::{AccessGuard, StorageError, WriteTransaction}; +use crate::{Result, TableHandle}; use std::borrow::Borrow; use std::fmt::{Debug, Formatter}; use std::ops::RangeBounds; @@ -62,6 +62,12 @@ pub struct Table<'db, 'txn, K: RedbKey + 'static, V: RedbValue + 'static> { tree: BtreeMut<'txn, K, V>, } +impl TableHandle for Table<'_, '_, K, V> { + fn name(&self) -> &str { + &self.name + } +} + impl<'db, 'txn, K: RedbKey + 'static, V: RedbValue + 'static> Table<'db, 'txn, K, V> { pub(crate) fn new( name: &str, diff --git a/src/transactions.rs b/src/transactions.rs index 5e50a223..a9eda881 100644 --- a/src/transactions.rs +++ b/src/transactions.rs @@ -360,6 +360,39 @@ impl<'db> TableNamespace<'db> { )) } + #[track_caller] + fn inner_delete(&mut self, name: &str, table_type: TableType) -> Result { + if let Some(location) = self.open_tables.get(name) { + return Err(TableError::TableAlreadyOpen(name.to_string(), location)); + } + + self.table_tree.delete_table(name, table_type) + } + + #[track_caller] + fn delete_table<'txn>( + &mut self, + transaction: &'txn WriteTransaction<'db>, + name: &str, + ) -> Result { + #[cfg(feature = "logging")] + info!("Deleting table: {}", name); + transaction.dirty.store(true, Ordering::Release); + self.inner_delete(name, TableType::Normal) + } + + #[track_caller] + fn delete_multimap_table<'txn>( + &mut self, + transaction: &'txn WriteTransaction<'db>, + name: &str, + ) -> Result { + #[cfg(feature = "logging")] + info!("Deleting multimap table: {}", name); + transaction.dirty.store(true, Ordering::Release); + self.inner_delete(name, TableType::Multimap) + } + pub(crate) fn close_table( &mut self, name: &str, @@ -741,14 +774,10 @@ impl<'db> WriteTransaction<'db> { /// /// Returns a bool indicating whether the table existed pub fn delete_table(&self, definition: impl TableHandle) -> Result { - #[cfg(feature = "logging")] - info!("Deleting table: {}", definition.name()); - self.dirty.store(true, Ordering::Release); - self.tables - .lock() - .unwrap() - .table_tree - .delete_table(definition.name(), TableType::Normal) + let name = definition.name().to_string(); + // Drop the definition so that callers can pass in a `Table` or `MultimapTable` to delete, without getting a TableAlreadyOpen error + drop(definition); + self.tables.lock().unwrap().delete_table(self, &name) } /// Delete the given table @@ -758,14 +787,13 @@ impl<'db> WriteTransaction<'db> { &self, definition: impl MultimapTableHandle, ) -> Result { - #[cfg(feature = "logging")] - info!("Deleting multimap table: {}", definition.name()); - self.dirty.store(true, Ordering::Release); + let name = definition.name().to_string(); + // Drop the definition so that callers can pass in a `Table` or `MultimapTable` to delete, without getting a TableAlreadyOpen error + drop(definition); self.tables .lock() .unwrap() - .table_tree - .delete_table(definition.name(), TableType::Multimap) + .delete_multimap_table(self, &name) } /// List all the tables diff --git a/tests/basic_tests.rs b/tests/basic_tests.rs index ee5eb2c7..28a8fcaf 100644 --- a/tests/basic_tests.rs +++ b/tests/basic_tests.rs @@ -1,7 +1,7 @@ use redb::backends::InMemoryBackend; use redb::{ Database, MultimapTableDefinition, MultimapTableHandle, Range, ReadableTable, RedbKey, - RedbValue, TableDefinition, TableHandle, TypeName, + RedbValue, TableDefinition, TableError, TableHandle, TypeName, }; use std::cmp::Ordering; #[cfg(not(target_os = "wasi"))] @@ -812,6 +812,34 @@ fn delete() { assert_eq!(table.len().unwrap(), 1); } +#[test] +fn delete_open_table() { + let tmpfile = create_tempfile(); + let db = Database::create(tmpfile.path()).unwrap(); + let write_txn = db.begin_write().unwrap(); + { + let table = write_txn.open_table(STR_TABLE).unwrap(); + assert!(matches!( + write_txn.delete_table(STR_TABLE).unwrap_err(), + TableError::TableAlreadyOpen(_, _) + )); + drop(table); + } + write_txn.commit().unwrap(); +} + +#[test] +fn delete_table() { + let tmpfile = create_tempfile(); + let db = Database::create(tmpfile.path()).unwrap(); + let write_txn = db.begin_write().unwrap(); + { + let table = write_txn.open_table(STR_TABLE).unwrap(); + assert!(write_txn.delete_table(table).unwrap()); + } + write_txn.commit().unwrap(); +} + #[test] fn no_dirty_reads() { let tmpfile = create_tempfile();