From 1f2b002bf5a3aa44cdae02af856b2094284c7106 Mon Sep 17 00:00:00 2001 From: Christopher Berner Date: Mon, 9 Nov 2026 01:19:27 +0000 Subject: [PATCH] Box the TableTypeMismatch and TypeDefinitionChanged details, and report the requested types A TableTypeMismatch reported only the types a table was stored with, and named it Table<..> even when it was a multimap table. For a table of Option written before T declared a niche, the stored name prints the same as the type the caller asked for, which made the error read as a contradiction. The message no longer names the table type. Under experimental-api-5 the TableTypeMismatch and TypeDefinitionChanged variants of TableError and Error hold a boxed TableTypeMismatch or TypeDefinitionInfo, with accessors, and TableTypeMismatch also reports the requested key and value types. Boxing keeps both error enums at 32 bytes, and private fields let later releases add to them. Assisted-by: Claude Code Claude-Session: https://claude.ai/code/session_01ALhxet6NP69dCfAUv8TAhR --- CHANGELOG.md | 3 + src/error.rs | 247 ++++++++++++++++++++++++++---- src/lib.rs | 2 + src/tree_store/table_tree_base.rs | 52 ++++--- tests/basic_tests.rs | 16 ++ tests/multimap_tests.rs | 12 ++ tests/niche_tests.rs | 6 + 7 files changed, 283 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fee852832..44b618649 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,9 @@ implementation, so implementations of `ReadableTable` must provide them. * `ReadableDatabase` is sealed, and can no longer be implemented outside of redb. * `StorageError` and `Error` no longer implement `From>`. +* The `TableTypeMismatch` and `TypeDefinitionChanged` variants of `TableError` and `Error` hold a + boxed `TableTypeMismatch` or `TypeDefinitionInfo`, whose accessors replace the variants' fields. + A `TableTypeMismatch` also reports the types the table was opened with. * The I/O methods of `StorageBackend` return `BackendError` instead of `io::Error`, as its lock methods already do. `?` still converts an `io::Error`. * `Entry`, `OccupiedEntry` and `VacantEntry` take a second lifetime, for the key, which no longer diff --git a/src/error.rs b/src/error.rs index eaffb687b..6bc040c1a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -251,20 +251,28 @@ impl core::error::Error for StorageError {} #[non_exhaustive] pub enum TableError { /// Table types didn't match. + #[cfg(not(feature = "experimental-api-5"))] TableTypeMismatch { table: String, key: TypeName, value: TypeName, }, + /// The table's types are not the ones it was opened with + #[cfg(feature = "experimental-api-5")] + TableTypeMismatch(Box), /// The table is a multimap table TableIsMultimap(String), /// The table is not a multimap table TableIsNotMultimap(String), + #[cfg(not(feature = "experimental-api-5"))] TypeDefinitionChanged { name: TypeName, alignment: usize, width: Option, }, + /// A type's fixed width or alignment differs from the one it is stored with + #[cfg(feature = "experimental-api-5")] + TypeDefinitionChanged(Box), /// Table name does not match any table in database TableDoesNotExist(String), /// Table name already exists in the database @@ -296,6 +304,7 @@ impl TableError { impl From for Error { fn from(err: TableError) -> Error { match err { + #[cfg(not(feature = "experimental-api-5"))] TableError::TypeDefinitionChanged { name, alignment, @@ -305,9 +314,14 @@ impl From for Error { alignment, width, }, + #[cfg(feature = "experimental-api-5")] + TableError::TypeDefinitionChanged(changed) => Error::TypeDefinitionChanged(changed), + #[cfg(not(feature = "experimental-api-5"))] TableError::TableTypeMismatch { table, key, value } => { Error::TableTypeMismatch { table, key, value } } + #[cfg(feature = "experimental-api-5")] + TableError::TableTypeMismatch(mismatch) => Error::TableTypeMismatch(mismatch), TableError::TableIsMultimap(table) => Error::TableIsMultimap(table), TableError::TableIsNotMultimap(table) => Error::TableIsNotMultimap(table), TableError::TableDoesNotExist(table) => Error::TableDoesNotExist(table), @@ -318,6 +332,185 @@ impl From for Error { } } +impl TableError { + pub(crate) fn type_mismatch( + table: &str, + stored_key: TypeName, + stored_value: TypeName, + requested_key: TypeName, + requested_value: TypeName, + ) -> Self { + #[cfg(feature = "experimental-api-5")] + { + Self::TableTypeMismatch(Box::new(TableTypeMismatch { + table: table.to_string(), + stored_key, + stored_value, + requested_key, + requested_value, + })) + } + #[cfg(not(feature = "experimental-api-5"))] + { + let _ = (requested_key, requested_value); + Self::TableTypeMismatch { + table: table.to_string(), + key: stored_key, + value: stored_value, + } + } + } + + pub(crate) fn type_definition_changed( + name: TypeName, + alignment: usize, + width: Option, + ) -> Self { + #[cfg(feature = "experimental-api-5")] + { + Self::TypeDefinitionChanged(Box::new(TypeDefinitionInfo { + name, + alignment, + width, + })) + } + #[cfg(not(feature = "experimental-api-5"))] + { + Self::TypeDefinitionChanged { + name, + alignment, + width, + } + } + } +} + +/// A table opened with key or value types other than the ones it is stored with. See +/// [`TableError::TableTypeMismatch`]. +#[cfg(feature = "experimental-api-5")] +#[derive(Debug)] +pub struct TableTypeMismatch { + table: String, + stored_key: TypeName, + stored_value: TypeName, + requested_key: TypeName, + requested_value: TypeName, +} + +#[cfg(feature = "experimental-api-5")] +impl TableTypeMismatch { + /// The name of the table + pub fn table(&self) -> &str { + &self.table + } + + /// The key type the table is stored with + pub fn stored_key(&self) -> &TypeName { + &self.stored_key + } + + /// The value type the table is stored with + pub fn stored_value(&self) -> &TypeName { + &self.stored_value + } + + /// The key type the table was opened with + pub fn requested_key(&self) -> &TypeName { + &self.requested_key + } + + /// The value type the table was opened with + pub fn requested_value(&self) -> &TypeName { + &self.requested_value + } +} + +#[cfg(feature = "experimental-api-5")] +impl Display for TableTypeMismatch { + fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { + fmt_type_mismatch( + f, + &self.table, + &self.stored_key, + &self.stored_value, + Some((&self.requested_key, &self.requested_value)), + ) + } +} + +/// A key or value type whose fixed width or alignment differs from the one it is stored with. See +/// [`TableError::TypeDefinitionChanged`]. +#[cfg(feature = "experimental-api-5")] +#[derive(Debug)] +pub struct TypeDefinitionInfo { + name: TypeName, + alignment: usize, + width: Option, +} + +#[cfg(feature = "experimental-api-5")] +impl TypeDefinitionInfo { + /// The type whose definition changed + pub fn name(&self) -> &TypeName { + &self.name + } + + /// The alignment the type is stored with + pub fn alignment(&self) -> usize { + self.alignment + } + + /// The fixed width the type is stored with, or `None` if it is stored as variable width + pub fn width(&self) -> Option { + self.width + } +} + +#[cfg(feature = "experimental-api-5")] +impl Display for TypeDefinitionInfo { + fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { + fmt_type_definition_changed(f, &self.name, self.alignment, self.width) + } +} + +fn fmt_type_definition_changed( + f: &mut Formatter<'_>, + name: &TypeName, + alignment: usize, + width: Option, +) -> core::fmt::Result { + write!( + f, + "Current definition of {} does not match stored definition (width={width:?}, alignment={alignment})", + name.name(), + ) +} + +// Names neither Table nor MultimapTable, since the mismatch may be either +fn fmt_type_mismatch( + f: &mut Formatter<'_>, + table: &str, + key: &TypeName, + value: &TypeName, + requested: Option<(&TypeName, &TypeName)>, +) -> core::fmt::Result { + write!( + f, + "{table} has key type {} and value type {}", + key.name(), + value.name() + )?; + if let Some((requested_key, requested_value)) = requested { + write!( + f, + ", but was opened with key type {} and value type {}", + requested_key.name(), + requested_value.name() + )?; + } + Ok(()) +} + impl From for TableError { fn from(err: StorageError) -> TableError { TableError::Storage(err) @@ -327,27 +520,20 @@ impl From for TableError { impl Display for TableError { fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { match self { + #[cfg(not(feature = "experimental-api-5"))] TableError::TypeDefinitionChanged { name, alignment, width, - } => { - write!( - f, - "Current definition of {} does not match stored definition (width={:?}, alignment={})", - name.name(), - width, - alignment, - ) - } + } => fmt_type_definition_changed(f, name, *alignment, *width), + #[cfg(feature = "experimental-api-5")] + TableError::TypeDefinitionChanged(changed) => changed.fmt(f), + #[cfg(not(feature = "experimental-api-5"))] TableError::TableTypeMismatch { table, key, value } => { - write!( - f, - "{table} is of type Table<{}, {}>", - key.name(), - value.name(), - ) + fmt_type_mismatch(f, table, key, value, None) } + #[cfg(feature = "experimental-api-5")] + TableError::TableTypeMismatch(mismatch) => mismatch.fmt(f), TableError::TableIsMultimap(table) => { write!(f, "{table} is a multimap table") } @@ -818,20 +1004,28 @@ pub enum Error { /// The key does not sort strictly between the entries adjacent to the cursor UnorderedKey, /// Table types didn't match. + #[cfg(not(feature = "experimental-api-5"))] TableTypeMismatch { table: String, key: TypeName, value: TypeName, }, + /// The table's types are not the ones it was opened with + #[cfg(feature = "experimental-api-5")] + TableTypeMismatch(Box), /// The table is a multimap table TableIsMultimap(String), /// The table is not a multimap table TableIsNotMultimap(String), + #[cfg(not(feature = "experimental-api-5"))] TypeDefinitionChanged { name: TypeName, alignment: usize, width: Option, }, + /// A type's fixed width or alignment differs from the one it is stored with + #[cfg(feature = "experimental-api-5")] + TypeDefinitionChanged(Box), /// Table name does not match any table in database TableDoesNotExist(String), /// Table name already exists in the database @@ -910,27 +1104,20 @@ impl Display for Error { "The key does not sort strictly between the entries adjacent to the cursor" ) } + #[cfg(not(feature = "experimental-api-5"))] Error::TypeDefinitionChanged { name, alignment, width, - } => { - write!( - f, - "Current definition of {} does not match stored definition (width={:?}, alignment={})", - name.name(), - width, - alignment, - ) - } + } => fmt_type_definition_changed(f, name, *alignment, *width), + #[cfg(feature = "experimental-api-5")] + Error::TypeDefinitionChanged(changed) => changed.fmt(f), + #[cfg(not(feature = "experimental-api-5"))] Error::TableTypeMismatch { table, key, value } => { - write!( - f, - "{table} is of type Table<{}, {}>", - key.name(), - value.name(), - ) + fmt_type_mismatch(f, table, key, value, None) } + #[cfg(feature = "experimental-api-5")] + Error::TableTypeMismatch(mismatch) => mismatch.fmt(f), Error::TableIsMultimap(table) => { write!(f, "{table} is a multimap table") } diff --git a/src/lib.rs b/src/lib.rs index c6427567d..ad9e36df3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -82,6 +82,8 @@ pub use error::{ SetDurabilityError, StorageError, TableError, TransactionError, }; #[cfg(feature = "experimental-api-5")] +pub use error::{TableTypeMismatch, TypeDefinitionInfo}; +#[cfg(feature = "experimental-api-5")] pub use key_range::KeyRange; #[cfg(feature = "experimental-niches")] pub use legacy_types::Legacy; diff --git a/src/tree_store/table_tree_base.rs b/src/tree_store/table_tree_base.rs index 74543c47d..c26bc4d0a 100644 --- a/src/tree_store/table_tree_base.rs +++ b/src/tree_store/table_tree_base.rs @@ -135,18 +135,18 @@ impl InternalTableDefinition { }; } if self.private_get_key_alignment() != ALIGNMENT { - return Err(TableError::TypeDefinitionChanged { - name: self.private_key_type(), - alignment: self.private_get_key_alignment(), - width: self.private_get_fixed_key_size(), - }); + return Err(TableError::type_definition_changed( + self.private_key_type(), + self.private_get_key_alignment(), + self.private_get_fixed_key_size(), + )); } if self.private_get_value_alignment() != ALIGNMENT { - return Err(TableError::TypeDefinitionChanged { - name: self.private_value_type(), - alignment: self.private_get_value_alignment(), - width: self.private_get_fixed_value_size(), - }); + return Err(TableError::type_definition_changed( + self.private_value_type(), + self.private_get_value_alignment(), + self.private_get_fixed_value_size(), + )); } Ok(()) @@ -169,25 +169,27 @@ impl InternalTableDefinition { let value_matches = stored_value == expected_value || expected_value.matches_legacy(&stored_value); if !key_matches || !value_matches { - return Err(TableError::TableTypeMismatch { - table: name.to_string(), - key: stored_key, - value: stored_value, - }); + return Err(TableError::type_mismatch( + name, + stored_key, + stored_value, + expected_key, + expected_value, + )); } if self.private_get_fixed_key_size() != K::fixed_width() { - return Err(TableError::TypeDefinitionChanged { - name: K::type_name(), - alignment: self.private_get_key_alignment(), - width: self.private_get_fixed_key_size(), - }); + return Err(TableError::type_definition_changed( + K::type_name(), + self.private_get_key_alignment(), + self.private_get_fixed_key_size(), + )); } if self.private_get_fixed_value_size() != V::fixed_width() { - return Err(TableError::TypeDefinitionChanged { - name: V::type_name(), - alignment: self.private_get_value_alignment(), - width: self.private_get_fixed_value_size(), - }); + return Err(TableError::type_definition_changed( + V::type_name(), + self.private_get_value_alignment(), + self.private_get_fixed_value_size(), + )); } Ok(()) diff --git a/tests/basic_tests.rs b/tests/basic_tests.rs index aacc591cd..f5e150db1 100644 --- a/tests/basic_tests.rs +++ b/tests/basic_tests.rs @@ -4582,6 +4582,22 @@ fn nonzero_type_does_not_alias_its_primitive() { write_txn.open_table(nonzero_key), Err(TableError::TableTypeMismatch { .. }) )); + #[cfg(feature = "experimental-api-5")] + { + let Err(TableError::TableTypeMismatch(mismatch)) = write_txn.open_table(nonzero_key) else { + panic!("expected a type mismatch"); + }; + assert_eq!(mismatch.table(), "x"); + assert_eq!(mismatch.stored_key().name(), "u32"); + assert_eq!(mismatch.requested_key().name(), "NonZeroU32"); + assert_eq!(mismatch.requested_value().name(), "u32"); + } + #[cfg(feature = "experimental-api-5")] + assert_eq!( + write_txn.open_table(nonzero_key).err().unwrap().to_string(), + "x has key type u32 and value type u32, but was opened with key type NonZeroU32 and \ + value type u32" + ); assert!(matches!( write_txn.open_table(nonzero_value), Err(TableError::TableTypeMismatch { .. }) diff --git a/tests/multimap_tests.rs b/tests/multimap_tests.rs index 814ab06a7..133745468 100644 --- a/tests/multimap_tests.rs +++ b/tests/multimap_tests.rs @@ -360,6 +360,18 @@ fn wrong_types() { txn.open_multimap_table(wrong_definition), Err(TableError::TableTypeMismatch { .. }) )); + + let message = txn + .open_multimap_table(wrong_definition) + .err() + .unwrap() + .to_string(); + #[cfg(feature = "experimental-api-5")] + let expected = "x has key type u32 and value type u32, but was opened with key type u64 and \ + value type u64"; + #[cfg(not(feature = "experimental-api-5"))] + let expected = "x has key type u32 and value type u32"; + assert_eq!(message, expected); } #[test] diff --git a/tests/niche_tests.rs b/tests/niche_tests.rs index 2d4ed5302..b8f35228b 100644 --- a/tests/niche_tests.rs +++ b/tests/niche_tests.rs @@ -125,6 +125,12 @@ fn tables_written_without_the_niche_open_through_the_legacy_type() { txn.open_table(VALUES), Err(TableError::TableTypeMismatch { .. }) )); + // The error names both spellings, which print the same without the type that was asked for + assert_eq!( + txn.open_table(VALUES).err().unwrap().to_string(), + "values has key type u64 and value type Option<&str>, but was opened with key type u64 \ + and value type niche::Option<&str>" + ); assert!(matches!( txn.open_table(KEYS), Err(TableError::TableTypeMismatch { .. })