Drop constraints before dropping tables

butane_cli/src/lib.rs

@@ -150,7 +150,7 @@ pub fn print_ops(ops: Vec<Operation>) -> Result<()> {
                     println!("  {}: {:?}", column.name(), column.typeid()?);
                 }
             }
-            AddTableConstraints(_) => {}
+            AddTableConstraints(_) | RemoveTableConstraints(_) => {}
             RemoveTable(name) => {
                 println!("Remove table {}", name);
             }

butane_core/src/db/pg.rs

@@ -548,12 +548,25 @@ where
 fn sql_for_op(current: &mut ADB, op: &Operation) -> Result<String> {
     match op {
         Operation::AddTable(table) => Ok(create_table(table, false)?),
-        Operation::AddTableConstraints(table) => Ok(create_table_constraints(table)),
+        Operation::AddTableConstraints(table) => Ok(create_table_fkey_constraints(table)),
         Operation::AddTableIfNotExists(table) => Ok(create_table(table, true)?),
         Operation::RemoveTable(name) => Ok(drop_table(name)),
+        Operation::RemoveTableConstraints(table) => remove_table_fkey_constraints(table),
         Operation::AddColumn(tbl, col) => add_column(tbl, col),
         Operation::RemoveColumn(tbl, name) => Ok(remove_column(tbl, name)),
-        Operation::ChangeColumn(tbl, old, new) => change_column(current, tbl, old, new),
+        Operation::ChangeColumn(tbl, old, new) => {
+            let table = current.get_table(tbl);
+            if let Some(table) = table {
+                change_column(table, old, new)
+            } else {
+                crate::warn!(
+                    "Cannot alter column {} from table {} that does not exist",
+                    &old.name(),
+                    tbl
+                );
+                Ok(String::new())
+            }
+        }
     }
 }
 
@@ -573,16 +586,26 @@ fn create_table(table: &ATable, allow_exists: bool) -> Result<String> {
     ))
 }
 
-fn create_table_constraints(table: &ATable) -> String {
+fn create_table_fkey_constraints(table: &ATable) -> String {
     table
         .columns
         .iter()
         .filter(|column| column.reference().is_some())
-        .map(|column| define_constraint(&table.name, column))
+        .map(|column| define_fkey_constraint(&table.name, column))
         .collect::<Vec<String>>()
         .join("\n")
 }
 
+fn remove_table_fkey_constraints(table: &ATable) -> Result<String> {
+    Ok(table
+        .columns
+        .iter()
+        .filter(|column| column.reference().is_some())
+        .map(|column| drop_fkey_constraints(table, column))
+        .collect::<Result<Vec<String>>>()?
+        .join("\n"))
+}
+
 fn define_column(col: &AColumn) -> Result<String> {
     let mut constraints: Vec<String> = Vec::new();
     if !col.nullable() {
@@ -609,7 +632,7 @@ fn define_column(col: &AColumn) -> Result<String> {
     ))
 }
 
-fn define_constraint(table_name: &str, column: &AColumn) -> String {
+fn define_fkey_constraint(table_name: &str, column: &AColumn) -> String {
     let reference = column
         .reference()
         .as_ref()
@@ -628,6 +651,11 @@ fn define_constraint(table_name: &str, column: &AColumn) -> String {
     }
 }
 
+fn drop_fkey_constraints(table: &ATable, column: &AColumn) -> Result<String> {
+    let mut modified_column = column.clone();
+    modified_column.remove_reference();
+    change_column(table, column, &modified_column)
+}
 fn col_sqltype(col: &AColumn) -> Result<Cow<str>> {
     match col.typeid()? {
         TypeIdentifier::Name(name) => Ok(Cow::Owned(name)),
@@ -672,7 +700,7 @@ fn add_column(tbl_name: &str, col: &AColumn) -> Result<String> {
         helper::sql_literal_value(&default)?
     )];
     if col.reference().is_some() {
-        stmts.push(define_constraint(tbl_name, col));
+        stmts.push(define_fkey_constraint(tbl_name, col));
     }
     let result = stmts.join("\n");
     Ok(result)
@@ -686,22 +714,9 @@ fn remove_column(tbl_name: &str, name: &str) -> String {
     )
 }
 
-fn change_column(
-    current: &mut ADB,
-    tbl_name: &str,
-    old: &AColumn,
-    new: &AColumn,
-) -> Result<String> {
+fn change_column(table: &ATable, old: &AColumn, new: &AColumn) -> Result<String> {
     use helper::quote_reserved_word;
-    let table = current.get_table(tbl_name);
-    if table.is_none() {
-        crate::warn!(
-            "Cannot alter column {} from table {} that does not exist",
-            &old.name(),
-            tbl_name
-        );
-        return Ok(String::new());
-    }
+    let tbl_name = &table.name;
 
     // Let's figure out what changed about the column
     let mut stmts: Vec<String> = Vec::new();
@@ -801,7 +816,7 @@ fn change_column(
             ));
         }
         if new.reference().is_some() {
-            stmts.push(define_constraint(tbl_name, new));
+            stmts.push(define_fkey_constraint(tbl_name, new));
         }
     }
 

butane_core/src/db/sqlite.rs

@@ -501,6 +501,7 @@ fn sql_for_op(current: &mut ADB, op: &Operation) -> Result<String> {
         Operation::AddTableConstraints(_table) => Ok("".to_owned()),
         Operation::AddTableIfNotExists(table) => Ok(create_table(table, true)),
         Operation::RemoveTable(name) => Ok(drop_table(name)),
+        Operation::RemoveTableConstraints(_table) => Ok("".to_owned()),
         Operation::AddColumn(tbl, col) => add_column(tbl, col),
         Operation::RemoveColumn(tbl, name) => Ok(remove_column(current, tbl, name)),
         Operation::ChangeColumn(tbl, old, new) => Ok(change_column(current, tbl, old, Some(new))),

butane_core/src/migrations/adb.rs

@@ -273,6 +273,7 @@ impl ADB {
                 self.tables.insert(table.name.clone(), table);
             }
             RemoveTable(name) => self.remove_table(&name),
+            RemoveTableConstraints(_) => {}
             AddColumn(table, col) => {
                 if let Some(t) = self.tables.get_mut(&table) {
                     t.add_column(col);
@@ -483,6 +484,11 @@ impl AColumn {
     pub fn add_reference(&mut self, reference: &ARef) {
         self.reference = Some(reference.clone())
     }
+    /// Remove the column that this column refers to.
+    pub fn remove_reference(&mut self) {
+        self.reference = None;
+    }
+    /// Get the type identifier.
     pub fn typeid(&self) -> Result<TypeIdentifier> {
         match &self.sqltype {
             DeferredSqlType::KnownId(t) => Ok(t.clone()),
@@ -572,15 +578,16 @@ pub fn create_many_table(
 }
 
 /// Individual operation use to apply a migration.
+/// The order of operations in a diff roughly follows this enum order.
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 pub enum Operation {
     //future improvement: support column renames
     /// Add a table.
     AddTable(ATable),
-    /// Add table constraints referring to other tables, if the backend supports it.
-    AddTableConstraints(ATable),
     /// Add a table, if it doesnt already exist.
     AddTableIfNotExists(ATable),
+    /// Remove table constraints referring to other tables, if the backend supports it.
+    RemoveTableConstraints(ATable),
     /// Remove named table.
     RemoveTable(String),
     /// Add a table column.
@@ -589,6 +596,8 @@ pub enum Operation {
     RemoveColumn(String, String),
     /// Change a table columns type.
     ChangeColumn(String, AColumn, AColumn),
+    /// Add table constraints referring to other tables, if the backend supports it.
+    AddTableConstraints(ATable),
 }
 
 /// Determine the operations necessary to move the database schema from `old` to `new`.
@@ -607,7 +616,13 @@ pub fn diff(old: &ADB, new: &ADB) -> Vec<Operation> {
     }
 
     // Remove tables
-    for removed in old_names.difference(&new_names) {
+    let removed_tables = old_names.difference(&new_names);
+    for removed in removed_tables.clone() {
+        let removed: &str = removed.as_ref();
+        let table = old.tables.get(removed).expect("no table").clone();
+        ops.push(Operation::RemoveTableConstraints(table));
+    }
+    for removed in removed_tables {
         ops.push(Operation::RemoveTable((*removed).to_string()));
     }
 

butane_core/src/migrations/mod.rs

@@ -197,7 +197,7 @@ where
                 Operation::ChangeColumn(table_name, _, _) => {
                     modified_tables.push(table_name.clone())
                 }
-                Operation::RemoveTable(_) => {}
+                Operation::RemoveTable(_) | Operation::RemoveTableConstraints(_) => {}
             }
         }
 

butane_core/tests/migration.rs

@@ -42,8 +42,11 @@ fn remove_table() {
 
     let ops = diff(&old, &new);
 
-    let expected_op = Operation::RemoveTable("a".to_owned());
-    assert_eq!(ops, vec![expected_op]);
+    let expected_ops = vec![
+        Operation::RemoveTableConstraints(table),
+        Operation::RemoveTable("a".to_owned()),
+    ];
+    assert_eq!(ops, expected_ops);
 }
 
 #[test]