Make down migrations respect the foreign_key_check setting

rusqlite_migration/benches/iai.rs

@@ -27,8 +27,8 @@ fn upward(i: u64) {
     let sql_migrations = (0..=i)
         .map(|i| {
             (
-                format!("CREATE TABLE t{}(a, b, c);", i),
-                format!("DROP TABLE t{};", i),
+                format!("CREATE TABLE t{i}(a, b, c);"),
+                format!("DROP TABLE t{i};"),
             )
         })
         .collect::<Vec<_>>();

rusqlite_migration/src/lib.rs

@@ -129,17 +129,25 @@ impl<'u> M<'u> {
     ///
     /// # Please note
     ///
-    /// * PRAGMA statements are discouraged here. They are often better applied outside of
+    /// ## PRAGMA statements
+    ///
+    /// PRAGMA statements are discouraged here. They are often better applied outside of
     /// migrations, because:
     ///   * a PRAGMA executed this way may not be applied consistently. For instance:
     ///     * [`foreign_keys`](https://sqlite.org/pragma.html#pragma_foreign_keys) needs to be
-    ///     executed for each sqlite connection, not just once per database as a migration,
-    ///     * [`journal_mode`](https://sqlite.org/pragma.html#pragma_journal_mode) has no effect
-    ///     when executed inside transactions (that will be the case for the SQL written in `up`).
-    ///   * Multiple SQL commands contaning `PRAGMA` are [not
-    ///   working](https://github.com/rusqlite/rusqlite/pull/794) with the `extra_check` feature of
-    ///   rusqlite.
+    ///     executed for each sqlite connection, not just once per database as a migration. Please
+    ///     see the [`Self::foreign_key_check()`] method to maintain foreign key constraints during
+    ///     migrations instead.
+    ///     * [`journal_mode`][jm] has no effect when executed inside transactions (that will be
+    ///       the case for the SQL written in `up`).
+    ///   * Multiple SQL commands contaning `PRAGMA` are [not working][ru794] with the
+    ///     `extra_check` feature of rusqlite.
+    ///
+    /// ## Misc.
+    ///
     /// * SQL commands should end with a “;”.
+    /// * You can use the `include_str!` macro to include whole files or opt for the
+    ///   `from-directory` feature of this crate.
     ///
     /// # Example
     ///
@@ -148,6 +156,9 @@ impl<'u> M<'u> {
     ///
     /// M::up("CREATE TABLE animals (name TEXT);");
     /// ```
+    ///
+    /// [ru794]: https://github.com/rusqlite/rusqlite/pull/794
+    /// [jm]: https://sqlite.org/pragma.html#pragma_journal_mode
     pub const fn up(sql: &'u str) -> Self {
         Self {
             up: sql,
@@ -240,16 +251,54 @@ impl<'u> M<'u> {
     }
 
     /// Enable an automatic validation of foreign keys before the migration transaction is closed.
-    /// This will cause the migration to fail if `PRAGMA foreign_key_check` returns any rows.
+    /// This works both for upward and downward migrations.
+    ///
+    /// This will cause the migration to fail if [`PRAGMA foreign_key_check`][fkc] returns any
+    /// foreign key check violations.
+    ///
+    /// # Turning `PRAGMA foreign_keys` ON and OFF
+    ///
+    /// By default with SQLite, foreign key constraints are not checked (that [may change in the
+    /// future][fk]). If you wish to check this, you need to manually turn [`PRAGMA
+    /// foreign_keys`][fk] ON. However, the [documentation for “Making Other Kinds Of Table Schema
+    /// Changes”][doc_other_migration] suggests turning this OFF before running the migrations.
+    ///
+    /// This if you want to enforce foreign key checks, it seems best to disable it first (in case
+    /// future versions of SQLite enable it by default), then run the migrations, then enable it,
+    /// as in the example below.
+    ///
+    /// Please make sure you **do not** call `PRAGMA foreign_keys` from inside the migrations, as
+    /// it would be a no-op (each migration is run inside a transaction).
     ///
     /// # Example
     ///
     /// ```
-    /// use rusqlite_migration::M;
+    /// use rusqlite::{params, Connection};
+    /// use rusqlite_migration::{Migrations, M};
     ///
-    /// M::up("CREATE TABLE animals (name TEXT);")
-    ///     .foreign_key_check();
+    /// let migrations = Migrations::new(vec![
+    ///     M::up("CREATE TABLE animals (name TEXT);")
+    ///         .foreign_key_check(), // Let’s pretend this is necessary here
+    /// ]);
+    ///
+    /// let mut conn = Connection::open_in_memory().unwrap();
+    ///
+    /// // Turn foreign key constraints off for the duration of the migration
+    /// conn.pragma_update(None, "foreign_keys", &"OFF").unwrap();
+    /// conn.pragma_update(None, "journal_mode", &"WAL").unwrap();
+    ///
+    /// migrations.to_latest(&mut conn).unwrap();
+    ///
+    /// // Restore foreign key constraints checks
+    /// conn.pragma_update(None, "foreign_keys", &"ON").unwrap();
+    ///
+    /// conn.execute("INSERT INTO animals (name) VALUES (?1)", params!["dog"])
+    ///     .unwrap();
     /// ```
+    ///
+    /// [fk]: https://www.sqlite.org/pragma.html#pragma_foreign_keys
+    /// [fkc]: https://www.sqlite.org/pragma.html#pragma_foreign_key_check
+    /// [doc_other_migration]: https://www.sqlite.org/lang_altertable.html#making_other_kinds_of_table_schema_changes
     pub const fn foreign_key_check(mut self) -> Self {
         self.foreign_key_check = true;
         self
@@ -336,8 +385,8 @@ impl<'m> Migrations<'m> {
     /// subdirectories in accordance with the given pattern:
     /// `{usize id indicating the order}-{convinient migration name}`
     ///
-    /// Those directories must contain at lest an `up.sql` file containing a valid upward migration.
-    /// They can also contain a `down.sql` file containing a downward migration.
+    /// Those directories must contain at lest an `up.sql` file containing a valid upward
+    /// migration. They can also contain a `down.sql` file containing a downward migration.
     ///
     /// ## Example structure
     ///
@@ -503,6 +552,10 @@ impl<'m> Migrations<'m> {
 
                 tx.execute_batch(down)
                     .map_err(|e| Error::with_sql(e, down))?;
+
+                if m.foreign_key_check {
+                    validate_foreign_keys(&tx)?;
+                }
             } else {
                 unreachable!();
             }

rusqlite_migration/src/tests/asynch.rs

@@ -2,8 +2,8 @@ use std::iter::FromIterator;
 
 use crate::{
     tests::helpers::{
-        all_valid, m_invalid0, m_invalid1, m_invalid_fk, m_valid0, m_valid10, m_valid11, m_valid20,
-        m_valid21, m_valid_fk,
+        all_valid, m_invalid0, m_invalid1, m_invalid_down_fk, m_invalid_fk, m_valid0, m_valid10,
+        m_valid11, m_valid20, m_valid21, m_valid_fk,
     },
     AsyncMigrations, Error, MigrationDefinitionError,
 };
@@ -51,6 +51,19 @@ async fn invalid_fk_check_test() {
     ));
 }
 
+#[tokio::test]
+async fn invalid_down_fk_check_test() {
+    let migrations = AsyncMigrations::new(vec![m_invalid_down_fk()]);
+
+    let mut conn = AsyncConnection::open_in_memory().await.unwrap();
+    migrations.to_latest(&mut conn).await.unwrap();
+
+    assert!(matches!(
+        dbg!(migrations.to_version(&mut conn, 0).await),
+        Err(Error::ForeignKeyCheck(_))
+    ));
+}
+
 #[tokio::test]
 async fn all_valid_test() {
     assert_eq!(Ok(()), AsyncMigrations::new(all_valid()).validate().await)

rusqlite_migration/src/tests/helpers.rs

@@ -30,6 +30,21 @@ pub fn m_valid_fk() -> M<'static> {
     .foreign_key_check()
 }
 
+pub fn m_invalid_down_fk() -> M<'static> {
+    M::up(
+        "CREATE TABLE fk1(a PRIMARY KEY); \
+        CREATE TABLE fk2( \
+            a, \
+            FOREIGN KEY(a) REFERENCES fk1(a) \
+        ); \
+        INSERT INTO fk1 (a) VALUES ('foo'); \
+        INSERT INTO fk2 (a) VALUES ('foo'); \
+    ",
+    )
+    .foreign_key_check()
+    .down("DROP TABLE fk1;")
+}
+
 // All valid Ms in the right order
 pub fn all_valid() -> Vec<M<'static>> {
     vec![

rusqlite_migration/src/tests/synch.rs

@@ -10,6 +10,21 @@ use crate::{
 
 use super::helpers::{m_invalid0, m_invalid1, m_valid20, m_valid21};
 
+fn m_invalid_down_fk() -> M<'static> {
+    M::up(
+        "CREATE TABLE fk1(a PRIMARY KEY); \
+        CREATE TABLE fk2( \
+            a, \
+            FOREIGN KEY(a) REFERENCES fk1(a) \
+        ); \
+        INSERT INTO fk1 (a) VALUES ('foo'); \
+        INSERT INTO fk2 (a) VALUES ('foo'); \
+    ",
+    )
+    .foreign_key_check()
+    .down("DROP TABLE fk1;")
+}
+
 #[test]
 fn empty_migrations_test() {
     let mut conn = Connection::open_in_memory().unwrap();
@@ -330,6 +345,19 @@ fn invalid_fk_check_test() {
     ));
 }
 
+#[test]
+fn invalid_down_fk_check_test() {
+    let migrations = Migrations::new(vec![m_invalid_down_fk()]);
+
+    let mut conn = Connection::open_in_memory().unwrap();
+    migrations.to_latest(&mut conn).unwrap();
+
+    assert!(matches!(
+        dbg!(migrations.to_version(&mut conn, 0)),
+        Err(Error::ForeignKeyCheck(_))
+    ));
+}
+
 #[test]
 fn all_valid_test() {
     assert_eq!(Ok(()), Migrations::new(all_valid()).validate());

rusqlite_migration_tests/tests/async_up_and_down.rs

@@ -154,7 +154,8 @@ async fn test_errors() {
         M::up("CREATE TABLE food (id INTEGER, name TEXT);"), // no down!!!
         // 2
         M::up("CREATE TABLE animal_food (animal_id INTEGER, food_id INTEGER);")
-            .down("DROP TABLE animal_food;"),
+            .down("DROP TABLE animal_food;")
+            .foreign_key_check(),
     ];
 
     {