Support adding nullable fields

3 files changed, 204 insertions, 40 deletions

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<R: Recordable> Engine<R> {
             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<Field: Eq>(&self, schema: &Vec<(Field, Type)>) -> DBResult<()> {
-        // Validate the record length
-        if self.values.len() != schema.len() {
+    pub fn validate<Field: Eq>(&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<String>,
+    pub data: Vec<u8>,
+    pub maybe_str: Option<String>,
+}
+
+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<Self::Field> {
+        vec![FieldWithNewNullableField::Name]
+    }
+
+    fn into_record(self) -> Vec<Value> {
+        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<Value>) -> 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::<Inst>::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::<InstWithNewNullableField>::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::<InstSingleId>::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::<Inst>::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),
+    }
+}