Changed SQLez migrations to be executed eagerly

Added fix for terminal working directory's sometimes getting lost
co-authored-by: Kay <kay@zed.dev>

Cargo.lock

@@ -6015,6 +6015,7 @@ dependencies = [
  "libsqlite3-sys",
  "parking_lot 0.11.2",
  "smol",
+ "sqlez_macros",
  "thread_local",
  "uuid 1.2.2",
 ]

crates/sqlez/Cargo.toml

@@ -15,4 +15,7 @@ thread_local = "1.1.4"
 lazy_static = "1.4"
 parking_lot = "0.11.1"
 futures = "0.3"
-uuid = { version = "1.1.2", features = ["v4"] }
+uuid = { version = "1.1.2", features = ["v4"] }
+
+[dev-dependencies]
+sqlez_macros = { path = "../sqlez_macros"}

crates/sqlez/src/migrations.rs

@@ -4,12 +4,36 @@
 // to creating a new db?)
 // Otherwise any missing migrations are run on the connection
 
-use anyhow::{anyhow, Result};
+use std::ffi::CString;
+
+use anyhow::{anyhow, Context, Result};
 use indoc::{formatdoc, indoc};
+use libsqlite3_sys::sqlite3_exec;
 
 use crate::connection::Connection;
 
 impl Connection {
+    fn eager_exec(&self, sql: &str) -> anyhow::Result<()> {
+        let sql_str = CString::new(sql).context("Error creating cstr")?;
+        unsafe {
+            sqlite3_exec(
+                self.sqlite3,
+                sql_str.as_c_str().as_ptr(),
+                None,
+                0 as *mut _,
+                0 as *mut _,
+            );
+        }
+        self.last_error()
+            .with_context(|| format!("Prepare call failed for query:\n{}", sql))?;
+
+        Ok(())
+    }
+
+    /// Migrate the database, for the given domain.
+    /// 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<()> {
         self.with_savepoint("migrating", || {
             // Setup the migrations table unconditionally
@@ -47,7 +71,7 @@ impl Connection {
                     }
                 }
 
-                self.exec(migration)?()?;
+                self.eager_exec(migration)?;
                 store_completed_migration((domain, index, *migration))?;
             }
 
@@ -59,6 +83,7 @@ impl Connection {
 #[cfg(test)]
 mod test {
     use indoc::indoc;
+    use sqlez_macros::sql;
 
     use crate::connection::Connection;
 
@@ -257,4 +282,46 @@ mod test {
         // Verify new migration returns error when run
         assert!(second_migration_result.is_err())
     }
+
+    #[test]
+    fn test_create_alter_drop() {
+        let connection = Connection::open_memory(Some("test_create_alter_drop"));
+
+        connection
+            .migrate(
+                "first_migration",
+                &[sql!( CREATE TABLE table1(a TEXT) STRICT; )],
+            )
+            .unwrap();
+
+        connection
+            .exec(sql!( INSERT INTO table1(a) VALUES ("test text"); ))
+            .unwrap()()
+        .unwrap();
+
+        connection
+            .migrate(
+                "second_migration",
+                &[sql!(
+                    CREATE TABLE table2(b TEXT) STRICT;
+
+                    INSERT INTO table2 (b)
+                    SELECT a FROM table1;
+
+                    DROP TABLE table1;
+
+                    ALTER TABLE table2 RENAME TO table1;
+                )],
+            )
+            .unwrap();
+
+        let res = &connection
+            .select::<String>(sql!(
+                SELECT b FROM table1
+            ))
+            .unwrap()()
+        .unwrap()[0];
+
+        assert_eq!(res, "test text");
+    }
 }

crates/sqlez/src/typed_statements.rs

@@ -7,11 +7,23 @@ use crate::{
 };
 
 impl Connection {
+    /// Prepare a statement which has no bindings and returns nothing.
+    ///
+    /// Note: If there are multiple statements that depend upon each other
+    /// (such as those which make schema changes), preparation will fail.
+    /// Use a true migration instead.
     pub fn exec<'a>(&'a self, query: &str) -> Result<impl 'a + FnMut() -> Result<()>> {
         let mut statement = Statement::prepare(self, query)?;
         Ok(move || statement.exec())
     }
 
+    /// Prepare a statement which takes a binding, but returns nothing.
+    /// The bindings for a given invocation should be passed to the returned
+    /// closure
+    ///
+    /// Note: If there are multiple statements that depend upon each other
+    /// (such as those which make schema changes), preparation will fail.
+    /// Use a true migration instead.
     pub fn exec_bound<'a, B: Bind>(
         &'a self,
         query: &str,
@@ -20,6 +32,11 @@ impl Connection {
         Ok(move |bindings| statement.with_bindings(bindings)?.exec())
     }
 
+    /// Prepare a statement which has no bindings and returns a `Vec<C>`.
+    ///
+    /// Note: If there are multiple statements that depend upon each other
+    /// (such as those which make schema changes), preparation will fail.
+    /// Use a true migration instead.
     pub fn select<'a, C: Column>(
         &'a self,
         query: &str,
@@ -28,6 +45,11 @@ impl Connection {
         Ok(move || statement.rows::<C>())
     }
 
+    /// Prepare a statement which takes a binding and returns a `Vec<C>`.
+    ///
+    /// Note: If there are multiple statements that depend upon each other
+    /// (such as those which make schema changes), preparation will fail.
+    /// Use a true migration instead.
     pub fn select_bound<'a, B: Bind, C: Column>(
         &'a self,
         query: &str,
@@ -36,6 +58,13 @@ impl Connection {
         Ok(move |bindings| statement.with_bindings(bindings)?.rows::<C>())
     }
 
+    /// Prepare a statement that selects a single row from the database.
+    /// Will return none if no rows are returned and will error if more than
+    /// 1 row
+    ///
+    /// Note: If there are multiple statements that depend upon each other
+    /// (such as those which make schema changes), preparation will fail.
+    /// Use a true migration instead.
     pub fn select_row<'a, C: Column>(
         &'a self,
         query: &str,
@@ -44,6 +73,13 @@ impl Connection {
         Ok(move || statement.maybe_row::<C>())
     }
 
+    /// Prepare a statement which takes a binding and selects a single row
+    /// from the database. WIll return none if no rows are returned and will
+    /// error if more than 1 row is returned.
+    ///
+    /// Note: If there are multiple statements that depend upon each other
+    /// (such as those which make schema changes), preparation will fail.
+    /// Use a true migration instead.
     pub fn select_row_bound<'a, B: Bind, C: Column>(
         &'a self,
         query: &str,

crates/terminal_view/src/persistence.rs

@@ -14,6 +14,26 @@ define_connection! {
                 FOREIGN KEY(workspace_id) REFERENCES workspaces(workspace_id)
                 ON DELETE CASCADE
             ) STRICT;
+        ),
+        // Remove the unique constraint on the item_id table
+        // SQLite doesn't have a way of doing this automatically, so
+        // we have to do this silly copying.
+        sql!(
+            CREATE TABLE terminals2 (
+                workspace_id INTEGER,
+                item_id INTEGER,
+                working_directory BLOB,
+                PRIMARY KEY(workspace_id, item_id),
+                FOREIGN KEY(workspace_id) REFERENCES workspaces(workspace_id)
+                ON DELETE CASCADE
+            ) STRICT;
+
+            INSERT INTO terminals2 (workspace_id, item_id, working_directory)
+            SELECT workspace_id, item_id, working_directory FROM terminals;
+
+            DROP TABLE terminals;
+
+            ALTER TABLE terminals2 RENAME TO terminals;
         )];
 }