Allow whitelisting certain migration changes

crates/sqlez/src/domain.rs

@@ -3,6 +3,10 @@ use crate::connection::Connection;
 pub trait Domain: 'static {
     const NAME: &str;
     const MIGRATIONS: &[&str];
+
+    fn should_allow_migration_change(_index: usize, _old: &str, _new: &str) -> bool {
+        false
+    }
 }
 
 pub trait Migrator: 'static {
@@ -17,7 +21,11 @@ impl Migrator for () {
 
 impl<D: Domain> Migrator for D {
     fn migrate(connection: &Connection) -> anyhow::Result<()> {
-        connection.migrate(Self::NAME, Self::MIGRATIONS)
+        connection.migrate(
+            Self::NAME,
+            Self::MIGRATIONS,
+            Self::should_allow_migration_change,
+        )
     }
 }
 

crates/sqlez/src/migrations.rs

@@ -34,7 +34,12 @@ impl Connection {
     /// Note: Unlike everything else in SQLez, migrations are run eagerly, without first
     /// preparing the SQL statements. This makes it possible to do multi-statement schema
     /// updates in a single string without running into prepare errors.
-    pub fn migrate(&self, domain: &'static str, migrations: &[&'static str]) -> Result<()> {
+    pub fn migrate(
+        &self,
+        domain: &'static str,
+        migrations: &[&'static str],
+        mut should_allow_migration_change: impl FnMut(usize, &str, &str) -> bool,
+    ) -> Result<()> {
         self.with_savepoint("migrating", || {
             // Setup the migrations table unconditionally
             self.exec(indoc! {"
@@ -70,6 +75,9 @@ impl Connection {
                     {
                         // Migration already run. Continue
                         continue;
+                    } else if should_allow_migration_change(index, &completed_migration, &migration)
+                    {
+                        continue;
                     } else {
                         anyhow::bail!(formatdoc! {"
                             Migration changed for {domain} at step {index}
@@ -110,6 +118,7 @@ mod test {
                     a TEXT,
                     b TEXT
                 )"}],
+                disallow_migration_change,
             )
             .unwrap();
 
@@ -138,6 +147,7 @@ mod test {
                         d TEXT
                     )"},
                 ],
+                disallow_migration_change,
             )
             .unwrap();
 
@@ -216,7 +226,11 @@ mod test {
 
         // Run the migration verifying that the row got dropped
         connection
-            .migrate("test", &["DELETE FROM test_table"])
+            .migrate(
+                "test",
+                &["DELETE FROM test_table"],
+                disallow_migration_change,
+            )
             .unwrap();
         assert_eq!(
             connection
@@ -234,7 +248,11 @@ mod test {
 
         // Run the same migration again and verify that the table was left unchanged
         connection
-            .migrate("test", &["DELETE FROM test_table"])
+            .migrate(
+                "test",
+                &["DELETE FROM test_table"],
+                disallow_migration_change,
+            )
             .unwrap();
         assert_eq!(
             connection
@@ -254,27 +272,28 @@ mod test {
             .migrate(
                 "test migration",
                 &[
-                    indoc! {"
+                    "CREATE TABLE test (col INTEGER)",
-                CREATE TABLE test (
+                    "INSERT INTO test (col) VALUES (1)",
-                    col INTEGER
-                )"},
-                    indoc! {"
-                    INSERT INTO test (col) VALUES (1)"},
                 ],
+                disallow_migration_change,
             )
             .unwrap();
 
+        let mut migration_changed = false;
+
         // Create another migration with the same domain but different steps
         let second_migration_result = connection.migrate(
             "test migration",
             &[
-                indoc! {"
+                "CREATE TABLE test (color INTEGER )",
-                CREATE TABLE test (
+                "INSERT INTO test (color) VALUES (1)",
-                    color INTEGER
-                )"},
-                indoc! {"
-                INSERT INTO test (color) VALUES (1)"},
             ],
+            |_, old, new| {
+                assert_eq!(old, "CREATE TABLE test (col INTEGER)");
+                assert_eq!(new, "CREATE TABLE test (color INTEGER)");
+                migration_changed = true;
+                false
+            },
         );
 
         // Verify new migration returns error when run
@@ -286,7 +305,11 @@ mod test {
         let connection = Connection::open_memory(Some("test_create_alter_drop"));
 
         connection
-            .migrate("first_migration", &["CREATE TABLE table1(a TEXT) STRICT;"])
+            .migrate(
+                "first_migration",
+                &["CREATE TABLE table1(a TEXT) STRICT;"],
+                disallow_migration_change,
+            )
             .unwrap();
 
         connection
@@ -307,6 +330,7 @@ mod test {
 
                     ALTER TABLE table2 RENAME TO table1;
                 "}],
+                disallow_migration_change,
             )
             .unwrap();
 
@@ -314,4 +338,8 @@ mod test {
 
         assert_eq!(res, "test text");
     }
+
+    fn disallow_migration_change(_: usize, _: &str, _: &str) -> bool {
+        false
+    }
 }

crates/workspace/src/persistence.rs

@@ -553,7 +553,7 @@ impl Domain for WorkspaceDb {
                             WHEN workspaces.local_paths_array IS NULL OR workspaces.local_paths_array = "" THEN
                                 NULL
                             ELSE
-                                json('[' || '"' || replace(workspaces.local_paths_array, ',', '"' || "," || '"') || '"' || ']')
+                                replace(workspaces.local_paths_array, ',', '\0')
                         END
                 END as paths,
 
@@ -606,6 +606,12 @@ impl Domain for WorkspaceDb {
             CREATE UNIQUE INDEX ix_workspaces_location ON workspaces(ssh_connection_id, paths);
         ),
     ];
+
+    // Allow recovering from bad migration
+    fn should_allow_migration_change(_index: usize, old: &str, new: &str) -> bool {
+        old.starts_with("CREATE TABLE ssh_connections")
+            && new.starts_with("CREATE TABLE ssh_connections")
+    }
 }
 
 db::static_connection!(DB, WorkspaceDb, []);
@@ -1812,6 +1818,7 @@ mod tests {
                         ON DELETE CASCADE
                     ) STRICT;
                 )],
+                |_, _, _| false,
             )
             .unwrap();
         })
@@ -1860,6 +1867,7 @@ mod tests {
                                 REFERENCES workspaces(workspace_id)
                             ON DELETE CASCADE
                         ) STRICT;)],
+                |_, _, _| false,
             )
         })
         .await