From 591053c55753e00341bd739c00f9ff09ae6e8141 Mon Sep 17 00:00:00 2001 From: Jan Tuomi Date: Mon, 10 Feb 2025 18:17:40 +0200 Subject: Remove typechecking --- log_db/src/common.rs | 87 ---------------------------------------------------- log_db/src/config.rs | 14 ++++----- log_db/src/engine.rs | 86 ++++++++++++++------------------------------------- log_db/src/lib.rs | 14 +++------ log_db/src/record.rs | 64 -------------------------------------- 5 files changed, 34 insertions(+), 231 deletions(-) (limited to 'log_db/src') diff --git a/log_db/src/common.rs b/log_db/src/common.rs index a450c28..2b40f52 100644 --- a/log_db/src/common.rs +++ b/log_db/src/common.rs @@ -183,58 +183,6 @@ pub enum IndexableValue { String(String), } -/// A primitive type -#[derive(Debug, Clone)] -pub enum PrimitiveType { - Int, - Decimal, - String, - Bytes, -} - -/// A primitive type + a nullability bit -#[derive(Debug, Clone)] -pub struct Type { - pub primitive: PrimitiveType, - pub nullable: bool, -} - -impl Type { - pub fn int() -> Self { - Type { - primitive: PrimitiveType::Int, - nullable: false, - } - } - - pub fn decimal() -> Self { - Type { - primitive: PrimitiveType::Decimal, - nullable: false, - } - } - - pub fn string() -> Self { - Type { - primitive: PrimitiveType::String, - nullable: false, - } - } - - pub fn bytes() -> Self { - Type { - primitive: PrimitiveType::Bytes, - nullable: false, - } - } - - pub fn nullable(&mut self) -> Self { - let mut new = self.clone(); - new.nullable = true; - new - } -} - #[derive(Debug, Clone)] pub enum Value { Null, @@ -341,41 +289,6 @@ impl Value { } } -pub fn type_check(value: &Value, value_type: &Type) -> bool { - match (value, value_type) { - ( - Value::Int(_), - Type { - primitive: PrimitiveType::Int, - .. - }, - ) => true, - ( - Value::Decimal(_), - Type { - primitive: PrimitiveType::Decimal, - .. - }, - ) => true, - ( - Value::Bytes(_), - Type { - primitive: PrimitiveType::Bytes, - .. - }, - ) => true, - ( - Value::String(_), - Type { - primitive: PrimitiveType::String, - .. - }, - ) => true, - (Value::Null, Type { nullable: true, .. }) => true, - _ => false, - } -} - pub fn get_secondary_memtable_index_by_field( sks: &Vec, field: &Field, diff --git a/log_db/src/config.rs b/log_db/src/config.rs index 2ed11c9..5b3cf26 100644 --- a/log_db/src/config.rs +++ b/log_db/src/config.rs @@ -1,7 +1,7 @@ use super::*; pub struct Schema { - pub fields: Vec<(F, Type)>, + pub fields: Vec, pub primary_key: F, pub secondary_keys: Vec, } @@ -12,7 +12,7 @@ pub struct ConfigBuilder { write_durability: Option, read_consistency: Option, - schema: Option>, + fields: Option>, primary_key: Option, secondary_keys: Option>, from_record: Option) -> T>, @@ -29,7 +29,7 @@ impl ConfigBuilder { write_durability: None, read_consistency: None, - schema: None, + fields: None, primary_key: None, secondary_keys: None, from_record: None, @@ -71,8 +71,8 @@ impl ConfigBuilder { self } - pub fn schema(mut self, schema: Vec<(F, Type)>) -> Self { - self.schema = Some(schema); + pub fn fields(mut self, schema: Vec) -> Self { + self.fields = Some(schema); self } @@ -98,7 +98,7 @@ impl ConfigBuilder { pub fn initialize(self) -> DBResult> { let schema = self - .schema + .fields .ok_or_else(|| DBError::ValidationError("Schema not set".to_string()))?; let primary_key = self .primary_key @@ -135,7 +135,7 @@ impl ConfigBuilder { #[derive(Clone)] pub struct Config { - pub schema: Vec<(F, Type)>, + pub schema: Vec, pub primary_key: F, pub secondary_keys: Vec, pub from_record: fn(Vec) -> T, diff --git a/log_db/src/engine.rs b/log_db/src/engine.rs index 62a88cb..d593053 100644 --- a/log_db/src/engine.rs +++ b/log_db/src/engine.rs @@ -68,7 +68,7 @@ impl Engine { let primary_key_index = config .schema .iter() - .position(|(field, _)| field == &config.primary_key) + .position(|field| field == &config.primary_key) .ok_or(DBError::ValidationError( "Primary key not found in schema after initialize".to_owned(), ))?; @@ -80,14 +80,9 @@ impl Engine { // If any of the keys is not in the schema or // is not an IndexableValue, return an error for &key in &all_keys { - let (_, value_type) = config.schema.iter().find(|(field, _)| field == key).ok_or( + let _ = config.schema.iter().find(|&field| field == key).ok_or( DBError::ValidationError("Key must be present in the field schema".to_owned()), )?; - - match value_type.primitive { - PrimitiveType::Int | PrimitiveType::String => {} - _ => return Err(DBError::ValidationError("Key must be indexable".to_owned())), - } } let primary_memtable = PrimaryMemtable::new(); let secondary_memtables = config @@ -163,9 +158,6 @@ impl Engine { for ForwardLogReaderItem { record, index } in ForwardLogReader::new_with_index(metadata_file, data_file, from_index) { - // Validate that the values in the record are compatible with the schema - record.validate(&self.config.schema)?; - let log_key = LogKey::new(segnum, index); if record.tombstone { @@ -200,7 +192,7 @@ impl Engine { .config .schema .iter() - .position(|(f, _)| sk_field == f) + .position(|f| sk_field == f) .unwrap(); let sk = record.at(sk_field_index).as_indexable().unwrap(); @@ -221,7 +213,7 @@ impl Engine { .config .schema .iter() - .position(|(f, _)| sk_field == f) + .position(|f| sk_field == f) .unwrap(); let sk = record.at(sk_field_index).as_indexable().unwrap(); @@ -258,22 +250,11 @@ impl Engine { field: &F, values: impl Iterator, ) -> DBResult> { - let field_type = self.get_field_type(field).ok_or(DBError::ValidationError( - "Field not found in schema".to_owned(), - ))?; - let indexables = values .map(|value| { - if type_check(&value, &field_type) { - value.as_indexable().ok_or(DBError::ValidationError( - "Queried value must be indexable".to_owned(), - )) - } else { - Err(DBError::ValidationError(format!( - "Queried value {:?} does not match key type: {:?}", - value, field_type - ))) - } + value.as_indexable().ok_or(DBError::ValidationError( + "Queried value must be indexable".to_owned(), + )) }) .collect::>>()?; @@ -396,35 +377,26 @@ impl Engine { field: &F, range: B, ) -> DBResult> { - fn range_bound_to_indexable( - bound: Bound<&Value>, - field_type: &Type, - ) -> DBResult> { - fn convert(value: &Value, field_type: &Type) -> DBResult { - if !type_check(&value, field_type) { - return Err(DBError::ValidationError(format!( - "Queried value does not match type: {:?}", - field_type - ))); - } - value.as_indexable().ok_or(DBError::ValidationError( - "Queried value must be indexable".to_owned(), - )) - } - + fn range_bound_to_indexable(bound: Bound<&Value>) -> DBResult> { match bound { - Bound::Included(value) => convert(value, field_type).map(Bound::Included), - Bound::Excluded(value) => convert(value, field_type).map(Bound::Excluded), + Bound::Included(value) => value + .as_indexable() + .ok_or(DBError::ValidationError( + "Queried value must be indexable".to_owned(), + )) + .map(Bound::Included), + Bound::Excluded(value) => value + .as_indexable() + .ok_or(DBError::ValidationError( + "Queried value must be indexable".to_owned(), + )) + .map(Bound::Excluded), Bound::Unbounded => Ok(Bound::Unbounded), } } - let field_type = self.get_field_type(field).ok_or(DBError::ValidationError( - "Field not found in schema".to_owned(), - ))?; - - let start_indexable = range_bound_to_indexable(range.start_bound(), field_type)?; - let end_indexable = range_bound_to_indexable(range.end_bound(), field_type)?; + let start_indexable = range_bound_to_indexable(range.start_bound())?; + let end_indexable = range_bound_to_indexable(range.end_bound())?; let indexable_bounds = OwnedBounds::new(start_indexable, end_indexable); @@ -705,15 +677,6 @@ impl Engine { Ok(()) } - #[inline] - fn get_field_type(&self, field: &F) -> Option<&Type> { - self.config - .schema - .iter() - .find(|(f, _)| f == field) - .map(|(_, t)| t) - } - #[inline] pub fn with_exclusive_lock( &mut self, @@ -801,10 +764,7 @@ mod tests { let mut db = DB::configure() .data_dir(data_dir.to_str().unwrap()) - .schema(vec![ - (Field::Id, Type::int()), - (Field::Name, Type::string()), - ]) + .fields(vec![Field::Id, Field::Name]) .primary_key(Field::Id) .secondary_keys(vec![Field::Name]) .from_record(TestInst2::from_record) diff --git a/log_db/src/lib.rs b/log_db/src/lib.rs index e92b68d..05db3dc 100644 --- a/log_db/src/lib.rs +++ b/log_db/src/lib.rs @@ -25,7 +25,7 @@ mod memtable_primary; mod memtable_secondary; mod record; -pub use common::{DBError, DBResult, OwnedBounds, Type, Value}; +pub use common::{DBError, DBResult, OwnedBounds, Value}; pub use config::{ReadConsistency, Schema, WriteDurability}; use common::*; @@ -58,9 +58,6 @@ impl DB { let record = Record::from(&(self.engine.config.into_record)(recordable)); debug!("Upserting record: {:?}", record); - record.validate(&self.engine.config.schema)?; - debug!("Record is valid"); - self.engine .with_exclusive_lock(move |engine| engine.upsert_record(record))?; @@ -297,7 +294,7 @@ mod tests { let mut db = DB::configure() .data_dir(data_dir.to_str().unwrap()) - .schema(vec![(Field::Id, Type::int())]) + .fields(vec![Field::Id]) .primary_key(Field::Id) .from_record(TestInst1::from_record) .into_record(TestInst1::into_record) @@ -384,7 +381,7 @@ mod tests { let mut db = DB::configure() .data_dir(data_dir.to_str().unwrap()) - .schema(vec![(Field::Id, Type::int())]) + .fields(vec![Field::Id]) .primary_key(Field::Id) .from_record(TestInst1::from_record) .into_record(TestInst1::into_record) @@ -438,10 +435,7 @@ mod tests { let mut db = DB::configure() .data_dir(data_dir.to_str().unwrap()) - .schema(vec![ - (Field::Id, Type::int()), - (Field::Name, Type::string()), - ]) + .fields(vec![Field::Id, Field::Name]) .primary_key(Field::Id) .secondary_keys(vec![Field::Name]) .from_record(TestInst2::from_record) diff --git a/log_db/src/record.rs b/log_db/src/record.rs index c373897..8108fb5 100644 --- a/log_db/src/record.rs +++ b/log_db/src/record.rs @@ -48,70 +48,6 @@ impl Record { pub fn at(&self, index: usize) -> &Value { &self.values[index] } - - pub fn validate(&self, schema: &[(Field, Type)]) -> DBResult<()> { - // If there are more values than schema fields, it's an error. - if self.values.len() > schema.len() { - return Err(DBError::ValidationError(format!( - "Record has more fields ({}) than expected by the schema ({})", - self.values.len(), - schema.len() - ))); - } - - for (i, (_, typ)) in schema.iter().enumerate() { - match self.values.get(i) { - Some(value) => { - if !Self::value_matches_type(value, typ) { - return Err(DBError::ValidationError(format!( - "Record field {} has incorrect type: {:?}, expected {:?}", - i, value, typ.primitive - ))); - } - } - // If the field is missing from the record... - None => { - // ...it's allowed only if the schema says the field is nullable. - if !typ.nullable { - return Err(DBError::ValidationError(format!( - "Record is missing field expected by the schema ({:?}) at index {}", - typ, i - ))); - } - } - } - } - - Ok(()) - } - - fn value_matches_type(value: &Value, typ: &Type) -> bool { - match (value, typ) { - (Value::Null, Type { nullable: true, .. }) => true, - ( - Value::Int(_), - Type { - primitive: PrimitiveType::Int, - .. - }, - ) => true, - ( - Value::String(_), - Type { - primitive: PrimitiveType::String, - .. - }, - ) => true, - ( - Value::Bytes(_), - Type { - primitive: PrimitiveType::Bytes, - .. - }, - ) => true, - _ => false, - } - } } #[cfg(test)] -- cgit v1.3