From f60b372db09f4ed091a55d44ac5013926f4cf64b Mon Sep 17 00:00:00 2001 From: Jan Tuomi Date: Mon, 3 Feb 2025 18:58:49 +0200 Subject: Support adding nullable fields --- log_db/src/engine.rs | 3 + log_db/src/record.rs | 92 +++++++++++++++------------ log_db/tests/integration.rs | 149 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 204 insertions(+), 40 deletions(-) (limited to 'log_db') diff --git a/log_db/src/engine.rs b/log_db/src/engine.rs index 4391451..d028ee0 100644 --- a/log_db/src/engine.rs +++ b/log_db/src/engine.rs @@ -163,6 +163,9 @@ 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.fields)?; + let log_key = LogKey::new(segnum, index); if record.tombstone { diff --git a/log_db/src/record.rs b/log_db/src/record.rs index 6d7cbbf..ab9f8dc 100644 --- a/log_db/src/record.rs +++ b/log_db/src/record.rs @@ -49,57 +49,69 @@ impl Record { &self.values[index] } - pub fn validate(&self, schema: &Vec<(Field, Type)>) -> DBResult<()> { - // Validate the record length - if self.values.len() != schema.len() { + 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 an incorrect number of fields: {}, expected {}", + "Record has more fields ({}) than expected by the schema ({})", self.values.len(), schema.len() ))); } - // Validate that record fields match schema types - for (i, (_, field)) in schema.iter().enumerate() { - match (&self.values[i], field) { - ( - Value::Null, - Type { - nullable: true, - primitive: _, - }, - ) => {} - ( - Value::Int(_), - Type { - primitive: PrimitiveType::Int, - .. - }, - ) => {} - ( - Value::String(_), - Type { - primitive: PrimitiveType::String, - .. - }, - ) => {} - ( - Value::Bytes(_), - Type { - primitive: PrimitiveType::Bytes, - .. - }, - ) => {} - _ => { - return Err(DBError::ValidationError(format!( - "Record field {} has incorrect type: {:?}, expected {:?}", - &i, &self.values[i], &field.primitive - ))); + 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, + } + } } /// A trait that describes how to convert a data structure into a database record and vice versa. diff --git a/log_db/tests/integration.rs b/log_db/tests/integration.rs index 31d2831..5376850 100644 --- a/log_db/tests/integration.rs +++ b/log_db/tests/integration.rs @@ -714,3 +714,152 @@ fn test_rollback_transaction() { assert_eq!(johns.len(), 0); } + +#[derive(Eq, PartialEq, Clone, Debug)] +enum FieldWithNewNullableField { + Id, + Name, + Data, + MaybeStr, +} + +struct InstWithNewNullableField { + pub id: i64, + pub name: Option, + pub data: Vec, + pub maybe_str: Option, +} + +impl Recordable for InstWithNewNullableField { + type Field = FieldWithNewNullableField; + + fn schema() -> Vec<(Self::Field, Type)> { + vec![ + (FieldWithNewNullableField::Id, Type::int()), + (FieldWithNewNullableField::Name, Type::string().nullable()), + (FieldWithNewNullableField::Data, Type::bytes()), + ( + FieldWithNewNullableField::MaybeStr, + Type::string().nullable(), + ), + ] + } + + fn primary_key() -> Self::Field { + FieldWithNewNullableField::Id + } + + fn secondary_keys() -> Vec { + vec![FieldWithNewNullableField::Name] + } + + fn into_record(self) -> Vec { + vec![ + Value::Int(self.id), + match self.name { + Some(name) => Value::String(name), + None => Value::Null, + }, + Value::Bytes(self.data), + match self.maybe_str { + Some(maybe_str) => Value::String(maybe_str), + None => Value::Null, + }, + ] + } + + fn from_record(record: Vec) -> Self { + let mut it = record.into_iter(); + + InstWithNewNullableField { + id: match it.next().unwrap() { + Value::Int(id) => id, + other => panic!("Invalid value type: {:?}", other), + }, + name: match it.next().unwrap() { + Value::String(name) => Some(name), + Value::Null => None, + other => panic!("Invalid value type: {:?}", other), + }, + data: match it.next().unwrap() { + Value::Bytes(data) => data, + other => panic!("Invalid value type: {:?}", other), + }, + maybe_str: match it.next() { + Some(Value::String(maybe_str)) => Some(maybe_str), + Some(Value::Null) => None, + None => None, + other => panic!("Invalid value type: {:?}", other), + }, + } + } +} + +#[test] +fn test_add_nullable_field() { + let data_dir = tmp_dir(); + + // Insert a record with 3 fields + { + let mut db = DB::::configure() + .data_dir(&data_dir) + .initialize() + .expect("Failed to initialize DB instance"); + + db.upsert(Inst { + id: 0, + name: Some("John".to_string()), + data: vec![3, 4, 5], + }) + .unwrap(); + } + + // Insert a record with 4 fields (last is nullable) + let mut db = DB::::configure() + .data_dir(&data_dir) + .initialize() + .expect("Failed to initialize DB instance"); + + db.upsert(InstWithNewNullableField { + id: 1, + name: Some("John".to_string()), + data: vec![3, 4, 5], + maybe_str: None, + }) + .unwrap(); + + let johns = db + .find_by( + &FieldWithNewNullableField::Name, + &Value::String("John".to_string()), + ) + .unwrap(); + + assert_eq!(johns.len(), 2); +} + +#[test] +fn test_add_non_nullable_field() { + let data_dir = tmp_dir(); + + // Insert a record with just one field + { + let mut db = DB::::configure() + .data_dir(&data_dir) + .initialize() + .expect("Failed to initialize DB instance"); + + db.upsert(InstSingleId { id: 0 }).unwrap(); + } + + // Configure the DB with three fields, one of which is non-nullable + // This should fail + match DB::::configure().data_dir(&data_dir).initialize() { + Ok(_) => panic!("Expected initialization to fail"), + Err(DBError::ValidationError(e)) => { + // Expected + error!("Expected: {:?}", e); + } + Err(e) => panic!("Unexpected error: {:?}", e), + } +} -- cgit v1.3