Dst bug: fix seqeuence migration precheck.

crates/engine/src/update.rs

@@ -199,16 +199,21 @@ fn auto_migrate_database(
                     .clone();
 
                 // Convert `SequenceDef` min/max to `AlgebraicValue`s of the correct type.
-                let min = AlgebraicValue::from_i128(&ty, sequence_def.min_value.unwrap_or(1)).ok_or_else(|| {
-                    anyhow::anyhow!("Precheck failed: added sequence {sequence_name} has invalid min value")
-                })?;
-
-                let max =
-                    AlgebraicValue::from_i128(&ty, sequence_def.max_value.unwrap_or(i128::MAX)).ok_or_else(|| {
-                        anyhow::anyhow!("Precheck failed: added sequence {sequence_name} has invalid max value")
+                let min = ty
+                    .saturating_value_from_i128(sequence_def.min_value.unwrap_or(1))
+                    .ok_or_else(|| {
+                        anyhow::anyhow!("Precheck failed: added sequence {sequence_name} has invalid min value")
                     })?;
 
-                let range = min..max;
+                let max = match sequence_def.max_value {
+                    Some(max) => ty.saturating_value_from_i128(max),
+                    None => ty.saturating_value_from_i128(i128::MAX),
+                }
+                .ok_or_else(|| {
+                    anyhow::anyhow!("Precheck failed: added sequence {sequence_name} has invalid max value")
+                })?;
+
+                let range = min..=max;
                 if stdb
                     .iter_by_col_range_mut(tx, table_id, sequence_def.column, range)?
                     .next()
@@ -988,6 +993,51 @@ mod test {
         Ok(())
     }
 
+    #[test]
+    fn add_sequence_precheck_rejects_existing_column_max_value() -> anyhow::Result<()> {
+        let auth_ctx = AuthCtx::for_testing();
+        let stdb = TestDB::durable()?;
+
+        let module_v1: ModuleDef = {
+            let mut b = RawModuleDefV9Builder::new();
+            b.build_table_with_new_type("seq_t", [("id", AlgebraicType::U8)], true)
+                .with_access(TableAccess::Public)
+                .finish();
+            b.finish().try_into().expect("valid module v1")
+        };
+
+        let module_v2: ModuleDef = {
+            let mut b = RawModuleDefV9Builder::new();
+            b.build_table_with_new_type("seq_t", [("id", AlgebraicType::U8)], true)
+                .with_column_sequence(0)
+                .with_access(TableAccess::Public)
+                .finish();
+            b.finish().try_into().expect("valid module v2")
+        };
+
+        {
+            let mut tx = begin_mut_tx(&stdb);
+            for def in module_v1.tables() {
+                create_table_from_def(&stdb, &mut tx, &module_v1, def)?;
+            }
+            let table_id = stdb.table_id_from_name_mut(&tx, "seq_t")?.expect("seq_t should exist");
+            insert(&stdb, &mut tx, table_id, &product![u8::MAX])?;
+            stdb.commit_tx(tx)?;
+        }
+
+        let mut tx = begin_mut_tx(&stdb);
+        let plan = ponder_migrate(&module_v1, &module_v2)?;
+        let err = update_database(&stdb, &mut tx, auth_ctx, plan, &TestLogger)
+            .err()
+            .expect("adding a sequence over an existing max value should fail");
+        assert!(
+            err.to_string().contains("already has values in range"),
+            "error should mention existing values in range, got: {err}"
+        );
+
+        Ok(())
+    }
+
     /// Verifies that `autoinc` sequence survives a schema migration that adds a column,
     /// and is also correctly persisted across database replay.
     ///

crates/sats/src/algebraic_type.rs

@@ -371,6 +371,35 @@ impl AlgebraicType {
         Self::deserialize(ValueDeserializer::from_ref(value))
     }
 
+    /// Constructs an `AlgebraicValue` of this type from an `i128`
+    /// using saturating conversion.
+    ///
+    /// Returns `None` if this is not an integer type.
+    /// If `value` does not fit in this type,
+    /// it is saturated to the type's range.
+    /// Negative values convert to `0` for unsigned types.
+    pub fn saturating_value_from_i128(&self, value: i128) -> Option<AlgebraicValue> {
+        let signed = |min, max| value.clamp(min, max);
+        let unsigned = |max| value.clamp(0, max);
+
+        match self {
+            Self::I8 => Some((signed(i8::MIN as i128, i8::MAX as i128) as i8).into()),
+            Self::I16 => Some((signed(i16::MIN as i128, i16::MAX as i128) as i16).into()),
+            Self::I32 => Some((signed(i32::MIN as i128, i32::MAX as i128) as i32).into()),
+            Self::I64 => Some((signed(i64::MIN as i128, i64::MAX as i128) as i64).into()),
+            Self::I128 => Some(value.into()),
+            Self::I256 => Some(i256::from(value).into()),
+
+            Self::U8 => Some((unsigned(u8::MAX as i128) as u8).into()),
+            Self::U16 => Some((unsigned(u16::MAX as i128) as u16).into()),
+            Self::U32 => Some((unsigned(u32::MAX as i128) as u32).into()),
+            Self::U64 => Some((unsigned(u64::MAX as i128) as u64).into()),
+            Self::U128 => Some((value.max(0) as u128).into()),
+            Self::U256 => Some(u256::from(value.max(0) as u128).into()),
+            _ => None,
+        }
+    }
+
     #[inline]
     /// Given an AlgebraicType, returns the min value for that type.
     pub fn min_value(&self) -> Option<AlgebraicValue> {

crates/sats/src/algebraic_value.rs

@@ -306,30 +306,6 @@ impl AlgebraicValue {
             _ => false,
         }
     }
-
-    /// Constructs an `AlgebraicValue` from an `i128` according to the given `AlgebraicType`.
-    ///
-    /// Returns `None` if the type is not a supported integer type.
-    pub fn from_i128(ty: &AlgebraicType, value: i128) -> Option<Self> {
-        let val = match ty {
-            AlgebraicType::I8 => (value as i8).into(),
-            AlgebraicType::I16 => (value as i16).into(),
-            AlgebraicType::I32 => (value as i32).into(),
-            AlgebraicType::I64 => (value as i64).into(),
-            AlgebraicType::I128 => value.into(),
-            AlgebraicType::I256 => i256::from(value).into(),
-
-            AlgebraicType::U8 => (value as u8).into(),
-            AlgebraicType::U16 => (value as u16).into(),
-            AlgebraicType::U32 => (value as u32).into(),
-            AlgebraicType::U64 => (value as u64).into(),
-            AlgebraicType::U128 => (value as u128).into(),
-            AlgebraicType::U256 => (u256::from(value as u128)).into(),
-
-            _ => return None,
-        };
-        Some(val)
-    }
 }
 
 impl<T: Into<AlgebraicValue>> From<Option<T>> for AlgebraicValue {