Use SQLite 3.35 DROP COLUMN

Author: jayvdb

butane_core/src/db/sqlite.rs

@@ -26,6 +26,9 @@ use crate::{debug, query, Error, Result, SqlType, SqlVal, SqlValRef};
 #[cfg(feature = "datetime")]
 const SQLITE_DT_FORMAT: &str = "%Y-%m-%d %H:%M:%S%.f";
 
+/// The minimum SQLite version required by this backend.
+pub const SQLITE_MIN_VERSION: i32 = 3035000;
+
 /// The name of the sqlite backend.
 pub const BACKEND_NAME: &str = "sqlite";
 /// The internal row creation order field name.
@@ -113,6 +116,12 @@ pub struct SQLiteConnection {
 }
 impl SQLiteConnection {
     fn open(path: impl AsRef<Path>) -> Result<Self> {
+        if rusqlite::version_number() < SQLITE_MIN_VERSION {
+            return Err(Error::IncompatibleSQLite(
+                rusqlite::version(),
+                SQLITE_MIN_VERSION,
+            ));
+        }
         #[cfg(feature = "log")]
         static INIT_SQLITE_LOGGING: Once = Once::new();
 
@@ -643,7 +652,7 @@ fn sql_for_op(current: &mut ADB, op: &Operation) -> Result<String> {
         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::RemoveColumn(tbl, name) => remove_column(current, tbl, name),
         Operation::ChangeColumn(tbl, old, new) => Ok(change_column(current, tbl, old, Some(new))),
     }
 }
@@ -766,21 +775,24 @@ fn add_column(tbl_name: &str, col: &AColumn) -> Result<String> {
     ))
 }
 
-fn remove_column(current: &mut ADB, tbl_name: &str, name: &str) -> String {
-    let old = current
+fn remove_column(current: &mut ADB, tbl_name: &str, name: &str) -> Result<String> {
+    let current_clone = current.clone();
+    let table = current_clone
         .get_table(tbl_name)
-        .and_then(|table| table.column(name))
-        .cloned();
-    match old {
-        Some(col) => change_column(current, tbl_name, &col, None),
-        None => {
-            crate::warn!(
-                "Cannot remove column {} that does not exist from table {}",
-                name,
-                tbl_name
-            );
-            "".to_string()
-        }
+        .ok_or_else(|| Error::TableNotFound(tbl_name.to_string()))?;
+    let col = table
+        .column(name)
+        .ok_or_else(|| Error::ColumnNotFound(tbl_name.to_string(), name.to_string()))?;
+    // "ALTER TABLE b DROP COLUMN fkey;" fails due to sqlite not being
+    // able to remove the attached constraint.
+    if col.reference().is_some() {
+        Ok(change_column(current, tbl_name, col, None))
+    } else {
+        Ok(format!(
+            "ALTER TABLE {} DROP COLUMN {};",
+            helper::quote_reserved_word(tbl_name),
+            helper::quote_reserved_word(name),
+        ))
     }
 }
 

butane_core/src/lib.rs

@@ -314,6 +314,12 @@ pub enum Error {
     #[cfg(feature = "async-adapter")]
     #[error("Crossbeam cannot send/recv, channel disconnected")]
     CrossbeamChannel,
+    #[error("SQLite version {0} is lower than minimum supported {1}")]
+    IncompatibleSQLite(&'static str, i32),
+    #[error("Table \"{0}\" not found in schema definitions")]
+    TableNotFound(String),
+    #[error("Column \"{0}\".\"{1}\" not found in schema definitions")]
+    ColumnNotFound(String, String),
 }
 
 #[cfg(feature = "sqlite")]

butane_core/tests/adb.rs

@@ -286,9 +286,6 @@ async fn add_table_fkey_delete_column(conn: ConnectionAsync) {
     conn.execute("SELECT * from a").await.unwrap();
     conn.execute("SELECT * from b").await.unwrap();
 
-    // "ALTER TABLE b DROP COLUMN fkey;" fails due to sqlite not being
-    // able to remove the attached constraint, however the RemoveColumn
-    // operation already recreates the table, so this works.
     let remove_column_op = Operation::RemoveColumn("b".to_owned(), "fkey".to_owned());
     let sql = backend
         .create_migration_sql(&new, vec![remove_column_op])

butane_core/tests/migration.rs

@@ -182,15 +182,7 @@ fn migration_add_field_sqlite() {
     migration_add_field(
         &mut sqlite_connection(),
         "ALTER TABLE Foo ADD COLUMN baz INTEGER NOT NULL DEFAULT 0;",
-        // The exact details of futzing a DROP COLUMN in sqlite aren't
-        // important (e.g. the temp table naming is certainly not part
-        // of the API contract), but the goal here is to ensure we're
-        // getting sane looking downgrade sql and a test failure if it
-        // changes. If the change is innocuous, this test should just
-        // be updated.
-        "CREATE TABLE Foo__butane_tmp (\"id\" INTEGER NOT NULL PRIMARY KEY,bar TEXT NOT NULL) STRICT;
-INSERT INTO Foo__butane_tmp SELECT \"id\", bar FROM Foo;DROP TABLE Foo;
-ALTER TABLE Foo__butane_tmp RENAME TO Foo;",
+        "ALTER TABLE Foo DROP COLUMN baz;",
     );
 }
 
@@ -212,9 +204,7 @@ fn migration_add_field_with_default_sqlite() {
         &mut sqlite_connection(),
         "ALTER TABLE Foo ADD COLUMN baz INTEGER NOT NULL DEFAULT 42;",
         // See comments on migration_add_field_sqlite
-        "CREATE TABLE Foo__butane_tmp (\"id\" INTEGER NOT NULL PRIMARY KEY,bar TEXT NOT NULL) STRICT;
-            INSERT INTO Foo__butane_tmp SELECT \"id\", bar FROM Foo;
-            DROP TABLE Foo;ALTER TABLE Foo__butane_tmp RENAME TO Foo;",
+        "ALTER TABLE Foo DROP COLUMN baz;",
     );
 }
 
@@ -279,20 +269,8 @@ fn migration_modify_field_pg() {
 fn migration_add_and_remove_field_sqlite() {
     migration_add_and_remove_field(
         &mut sqlite_connection(),
-        // The exact details of futzing a DROP COLUMN in sqlite aren't
-        // important (e.g. the temp table naming is certainly not part
-        // of the API contract), but the goal here is to ensure we're
-        // getting sane looking downgrade sql and a test failure if it
-        // changes. If the change is innocuous, this test should just
-        // be updated.
-        "ALTER TABLE Foo ADD COLUMN baz INTEGER NOT NULL DEFAULT 0;
-            CREATE TABLE Foo__butane_tmp (\"id\" INTEGER NOT NULL PRIMARY KEY,baz INTEGER NOT NULL) STRICT;
-            INSERT INTO Foo__butane_tmp SELECT \"id\", baz FROM Foo;
-            DROP TABLE Foo;ALTER TABLE Foo__butane_tmp RENAME TO Foo;",
-        "ALTER TABLE Foo ADD COLUMN bar TEXT NOT NULL DEFAULT '';
-            CREATE TABLE Foo__butane_tmp (\"id\" INTEGER NOT NULL PRIMARY KEY,bar TEXT NOT NULL) STRICT;
-            INSERT INTO Foo__butane_tmp SELECT \"id\", bar FROM Foo;DROP TABLE Foo;
-            ALTER TABLE Foo__butane_tmp RENAME TO Foo;",
+        "ALTER TABLE Foo ADD COLUMN baz INTEGER NOT NULL DEFAULT 0;ALTER TABLE Foo DROP COLUMN bar;",
+        "ALTER TABLE Foo ADD COLUMN bar TEXT NOT NULL DEFAULT '';ALTER TABLE Foo DROP COLUMN baz;",
     );
 }
 

examples/getting_started/.butane/migrations/20201229_171630604_likes/sqlite_down.sql

@@ -1,11 +1 @@
-CREATE TABLE Post__butane_tmp (
-"id" INTEGER NOT NULL PRIMARY KEY,
-title TEXT NOT NULL,
-body TEXT NOT NULL,
-published INTEGER NOT NULL,
-blog INTEGER NOT NULL,
-byline TEXT
-) STRICT;
-INSERT INTO Post__butane_tmp SELECT "id", title, body, published, blog, byline FROM Post;
-DROP TABLE Post;
-ALTER TABLE Post__butane_tmp RENAME TO Post;
+ALTER TABLE Post DROP COLUMN likes;

examples/getting_started/src/butane_migrations.rs

@@ -330,7 +330,7 @@ pub fn get_migrations() -> Result<MemMigrations, butane::Error> {
       },
       "down": {
         "pg": "ALTER TABLE Post DROP COLUMN likes;\n",
-        "sqlite": "CREATE TABLE Post__butane_tmp (\n\"id\" INTEGER NOT NULL PRIMARY KEY,\ntitle TEXT NOT NULL,\nbody TEXT NOT NULL,\npublished INTEGER NOT NULL,\nblog INTEGER NOT NULL,\nbyline TEXT\n) STRICT;\nINSERT INTO Post__butane_tmp SELECT \"id\", title, body, published, blog, byline FROM Post;\nDROP TABLE Post;\nALTER TABLE Post__butane_tmp RENAME TO Post;\n"
+        "sqlite": "ALTER TABLE Post DROP COLUMN likes;\n"
       }
     },
     "20240115_023841384_dbconstraints": {

examples/getting_started_async/.butane/migrations/20201229_171630604_likes/sqlite_down.sql

@@ -1,11 +1 @@
-CREATE TABLE Post__butane_tmp (
-"id" INTEGER NOT NULL PRIMARY KEY,
-title TEXT NOT NULL,
-body TEXT NOT NULL,
-published INTEGER NOT NULL,
-blog INTEGER NOT NULL,
-byline TEXT
-) STRICT;
-INSERT INTO Post__butane_tmp SELECT "id", title, body, published, blog, byline FROM Post;
-DROP TABLE Post;
-ALTER TABLE Post__butane_tmp RENAME TO Post;
+ALTER TABLE Post DROP COLUMN likes;

examples/getting_started_async/src/butane_migrations.rs

@@ -330,7 +330,7 @@ pub fn get_migrations() -> Result<MemMigrations, butane::Error> {
       },
       "down": {
         "pg": "ALTER TABLE Post DROP COLUMN likes;\n",
-        "sqlite": "CREATE TABLE Post__butane_tmp (\n\"id\" INTEGER NOT NULL PRIMARY KEY,\ntitle TEXT NOT NULL,\nbody TEXT NOT NULL,\npublished INTEGER NOT NULL,\nblog INTEGER NOT NULL,\nbyline TEXT\n) STRICT;\nINSERT INTO Post__butane_tmp SELECT \"id\", title, body, published, blog, byline FROM Post;\nDROP TABLE Post;\nALTER TABLE Post__butane_tmp RENAME TO Post;\n"
+        "sqlite": "ALTER TABLE Post DROP COLUMN likes;\n"
       }
     },
     "20240115_023841384_dbconstraints": {

examples/newtype/.butane/migrations/20240406_035726416_tags/sqlite_down.sql

@@ -7,16 +7,4 @@ FOREIGN KEY (has) REFERENCES "Tag"("tag")
 CREATE TABLE Tag (
 "tag" TEXT NOT NULL PRIMARY KEY
 ) STRICT;
-CREATE TABLE Post__butane_tmp (
-"id" BLOB NOT NULL PRIMARY KEY,
-title TEXT NOT NULL,
-body TEXT NOT NULL,
-published INTEGER NOT NULL,
-blog BLOB NOT NULL,
-byline TEXT,
-likes INTEGER NOT NULL,
-FOREIGN KEY (blog) REFERENCES Blog("id")
-) STRICT;
-INSERT INTO Post__butane_tmp SELECT "id", title, body, published, blog, byline, likes FROM Post;
-DROP TABLE Post;
-ALTER TABLE Post__butane_tmp RENAME TO Post;
+ALTER TABLE Post DROP COLUMN tags;

examples/newtype/src/butane_migrations.rs

@@ -377,7 +377,7 @@ pub fn get_migrations() -> Result<MemMigrations, butane::Error> {
       },
       "down": {
         "pg": "CREATE TABLE Post_tags_Many (\n\"owner\" BYTEA NOT NULL,\nhas TEXT NOT NULL\n);\nCREATE TABLE \"Tag\" (\n\"tag\" TEXT NOT NULL PRIMARY KEY\n);\nALTER TABLE Post DROP COLUMN tags;\nALTER TABLE Post_tags_Many ADD FOREIGN KEY (\"owner\") REFERENCES Post(\"id\");\nALTER TABLE Post_tags_Many ADD FOREIGN KEY (has) REFERENCES \"Tag\"(\"tag\");\n",
-        "sqlite": "CREATE TABLE Post_tags_Many (\n\"owner\" BLOB NOT NULL,\nhas TEXT NOT NULL,\nFOREIGN KEY (\"owner\") REFERENCES Post(\"id\")\nFOREIGN KEY (has) REFERENCES \"Tag\"(\"tag\")\n) STRICT;\nCREATE TABLE Tag (\n\"tag\" TEXT NOT NULL PRIMARY KEY\n) STRICT;\nCREATE TABLE Post__butane_tmp (\n\"id\" BLOB NOT NULL PRIMARY KEY,\ntitle TEXT NOT NULL,\nbody TEXT NOT NULL,\npublished INTEGER NOT NULL,\nblog BLOB NOT NULL,\nbyline TEXT,\nlikes INTEGER NOT NULL,\nFOREIGN KEY (blog) REFERENCES Blog(\"id\")\n) STRICT;\nINSERT INTO Post__butane_tmp SELECT \"id\", title, body, published, blog, byline, likes FROM Post;\nDROP TABLE Post;\nALTER TABLE Post__butane_tmp RENAME TO Post;\n"
+        "sqlite": "CREATE TABLE Post_tags_Many (\n\"owner\" BLOB NOT NULL,\nhas TEXT NOT NULL,\nFOREIGN KEY (\"owner\") REFERENCES Post(\"id\")\nFOREIGN KEY (has) REFERENCES \"Tag\"(\"tag\")\n) STRICT;\nCREATE TABLE Tag (\n\"tag\" TEXT NOT NULL PRIMARY KEY\n) STRICT;\nALTER TABLE Post DROP COLUMN tags;\n"
       }
     }
   },