More comments

Author: Shubham8287

crates/lib/src/db/raw_def/v9.rs

@@ -366,18 +366,18 @@ pub enum RawMiscModuleExportV9 {
     ColumnDefaultValue(RawColumnDefaultValueV9),
 }
 
-/// Marks a field as having a particular default.
+/// Marks a particular table's column as having a particular default.
 #[derive(Debug, Clone, SpacetimeType)]
 #[sats(crate = crate)]
 #[cfg_attr(feature = "test", derive(PartialEq, Eq, PartialOrd, Ord))]
 pub struct RawColumnDefaultValueV9 {
+    /// Identifies which table that has the default value.
+    /// This corresponds to `name` in `RawTableDefV9`.
     pub table: RawIdentifier,
-
-    /// Must be the index of a valid column within `ty`.
+    /// Identifies which column of `table` that has the default value.
     pub col_id: ColId,
-
-    /// A BSATN-encoded [`AlgebraicValue`] valid at `ty`.
-    /// (We can't use `AlgebraicValue` directly because it isn't serializable!)
+    /// A BSATN-encoded [`AlgebraicValue`] valid at the table column's type.
+    /// (We cannot use `AlgebraicValue` directly as it isn't `Spacetimetype`.)
     pub value: Box<[u8]>,
 }
 

crates/schema/src/def.rs

@@ -1116,7 +1116,7 @@ mod tests {
                 true,
             )
             .with_default_column_value(1, AlgebraicValue::Bool(false))
-            .with_default_column_value(1, AlgebraicValue::U16(10))
+            .with_default_column_value(1, AlgebraicValue::unit())
             .finish();
 
         let result: Result<ModuleDef, ValidationErrors> = old_builder.finish().try_into();
@@ -1126,5 +1126,10 @@ mod tests {
             result,
             ValidationError::ColumnDefaultValueMalformed { table, col_id, .. } => *table == apples.clone().into() && *col_id == ColId(1)
         );
+        assert!(result.is_err_and(|e| e
+            .into_iter()
+            .filter(|e| matches!(e, ValidationError::ColumnDefaultValueMalformed { .. }))
+            .count()
+            == 2))
     }
 }

crates/schema/src/def/validate/v9.rs

@@ -80,7 +80,7 @@ pub fn validate(def: RawModuleDefV9) -> Result<ModuleDef> {
         .combine_errors()
         .and_then(|(mut tables, types, reducers)| {
             let sched_exists = check_scheduled_reducers_exist(&tables, &reducers);
-            let default_values_work = default_values_work(misc_exports, &mut validator, &mut tables);
+            let default_values_work = proccess_misc_exports(misc_exports, &validator, &mut tables);
             (sched_exists, default_values_work).combine_errors()?;
 
             Ok((tables, types, reducers))
@@ -348,21 +348,19 @@ impl ModuleValidator<'_> {
     }
 
     fn validate_column_default_value(
-        &mut self,
-        tables: &mut HashMap<Identifier, TableDef>,
-        cdv: RawColumnDefaultValueV9,
-    ) -> Result<()> {
+        &self,
+        tables: &HashMap<Identifier, TableDef>,
+        cdv: &RawColumnDefaultValueV9,
+    ) -> Result<AlgebraicValue> {
         let table_name = identifier(cdv.table.clone())?;
 
         // Extract the table. We cannot make progress otherwise.
-        let table = tables
-            .get_mut(&table_name)
-            .ok_or_else(|| ValidationError::TableNotFound {
-                table: cdv.table.clone(),
-            })?;
+        let table = tables.get(&table_name).ok_or_else(|| ValidationError::TableNotFound {
+            table: cdv.table.clone(),
+        })?;
 
         // Get the column that a default is being added to.
-        let Some(col) = table.columns.get_mut(cdv.col_id.idx()) else {
+        let Some(col) = table.columns.get(cdv.col_id.idx()) else {
             return Err(ValidationError::ColumnNotFound {
                 table: cdv.table.clone(),
                 def: cdv.table.clone(),
@@ -383,17 +381,8 @@ impl ModuleValidator<'_> {
                 }
                 .into()
             });
-        // Ensure there's only one default value.
-        if col.default_value.is_some() {
-            return Err(ValidationError::MultipleColumnDefaultValues {
-                table: cdv.table.clone(),
-                col_id: cdv.col_id,
-            }
-            .into());
-        }
-        col.default_value = field_value.ok();
 
-        Ok(())
+        field_value
     }
 
     /// Validate a type definition.
@@ -944,20 +933,59 @@ fn check_scheduled_reducers_exist(
         .collect_all_errors()
 }
 
-fn default_values_work(
+fn proccess_misc_exports(
     misc_exports: Vec<RawMiscModuleExportV9>,
-    validator: &mut ModuleValidator,
+    validator: &ModuleValidator,
     tables: &mut IdentifierMap<TableDef>,
 ) -> Result<()> {
     misc_exports
         .into_iter()
         .map(|export| match export {
-            RawMiscModuleExportV9::ColumnDefaultValue(fdv) => validator.validate_column_default_value(tables, fdv),
+            RawMiscModuleExportV9::ColumnDefaultValue(cdv) => process_column_default_value(&cdv, validator, tables),
             _ => unimplemented!("unknown misc export"),
         })
         .collect_all_errors::<()>()
 }
 
+fn process_column_default_value(
+    cdv: &RawColumnDefaultValueV9,
+    validator: &ModuleValidator,
+    tables: &mut IdentifierMap<TableDef>,
+) -> Result<()> {
+    // Validate the default value
+    let validated_value = validator.validate_column_default_value(tables, cdv)?;
+
+    let table_name = identifier(cdv.table.clone())?;
+    let table = tables
+        .get_mut(&table_name)
+        .ok_or_else(|| ValidationError::TableNotFound {
+            table: cdv.table.clone(),
+        })?;
+
+    let column = table
+        .columns
+        .get_mut(cdv.col_id.idx())
+        .ok_or_else(|| ValidationError::ColumnNotFound {
+            table: cdv.table.clone(),
+            def: cdv.table.clone(),
+            column: cdv.col_id,
+        })?;
+
+    // Ensure there's only one default value.
+    if column.default_value.is_some() {
+        return Err(ValidationError::MultipleColumnDefaultValues {
+            table: cdv.table.clone(),
+            col_id: cdv.col_id,
+        }
+        .into());
+    }
+
+    // Set the default value
+    column.default_value = Some(validated_value);
+
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     use crate::def::validate::tests::{