From 7ae5eec2c3c25f4eb1e927b6e3d46d98ed1dd9e8 Mon Sep 17 00:00:00 2001 From: Dmitriy Pleshevskiy Date: Sat, 24 Apr 2021 22:39:42 +0300 Subject: [PATCH] chore: add support transactional ddl for client I didn't know that mysql doesn't support transactional ddl. It means that we cannot create table, alter table and etc. in transaction. At the moment migra supports only postgres client, that can be use transaction for ddl. --- migra-cli/src/database/clients/mysql.rs | 4 ++- migra-cli/src/database/clients/postgres.rs | 7 ++++ migra-cli/src/database/connection.rs | 9 ++++- migra-cli/src/database/mod.rs | 1 + migra-cli/src/database/transaction.rs | 4 +-- migra-cli/tests/commands.rs | 33 ------------------- migra-cli/tests/data/Migra_mysql_invalid.toml | 4 --- .../210218232851_create_articles/down.sql | 3 -- .../210218232851_create_articles/up.sql | 8 ----- .../210218233414_create_persons/down.sql | 6 ---- .../210218233414_create_persons/up.sql | 12 ------- .../210218233414_create_persons/up.sql | 10 +++--- 12 files changed, 27 insertions(+), 74 deletions(-) delete mode 100644 migra-cli/tests/data/Migra_mysql_invalid.toml delete mode 100644 migra-cli/tests/data/mysql_invalid/migrations/210218232851_create_articles/down.sql delete mode 100644 migra-cli/tests/data/mysql_invalid/migrations/210218232851_create_articles/up.sql delete mode 100644 migra-cli/tests/data/mysql_invalid/migrations/210218233414_create_persons/down.sql delete mode 100644 migra-cli/tests/data/mysql_invalid/migrations/210218233414_create_persons/up.sql diff --git a/migra-cli/src/database/clients/mysql.rs b/migra-cli/src/database/clients/mysql.rs index 8a7fa98..da1f792 100644 --- a/migra-cli/src/database/clients/mysql.rs +++ b/migra-cli/src/database/clients/mysql.rs @@ -10,7 +10,7 @@ pub struct MySqlConnection { impl OpenDatabaseConnection for MySqlConnection { fn open(connection_string: &str) -> StdResult { - let pool = Pool::new(connection_string)?; + let pool = Pool::new_manual(1, 1, connection_string)?; let conn = pool.get_conn()?; Ok(MySqlConnection { conn }) } @@ -25,6 +25,8 @@ impl DatabaseStatements for MySqlConnection { } } +impl SupportsTransactionalDDL for MySqlConnection {} + impl DatabaseConnection for MySqlConnection { fn batch_execute(&mut self, query: &str) -> StdResult<()> { self.conn.query_drop(query)?; diff --git a/migra-cli/src/database/clients/postgres.rs b/migra-cli/src/database/clients/postgres.rs index cbba7a8..f6e03f7 100644 --- a/migra-cli/src/database/clients/postgres.rs +++ b/migra-cli/src/database/clients/postgres.rs @@ -23,6 +23,13 @@ impl DatabaseStatements for PostgresConnection { } } +impl SupportsTransactionalDDL for PostgresConnection { + #[inline] + fn supports_transactional_ddl(&self) -> bool { + true + } +} + impl DatabaseConnection for PostgresConnection { fn batch_execute(&mut self, query: &str) -> StdResult<()> { self.client.batch_execute(query)?; diff --git a/migra-cli/src/database/connection.rs b/migra-cli/src/database/connection.rs index 0fc5f3b..5ec3f05 100644 --- a/migra-cli/src/database/connection.rs +++ b/migra-cli/src/database/connection.rs @@ -13,7 +13,14 @@ pub trait DatabaseStatements { fn create_migration_table_stmt(&self) -> &'static str; } -pub trait DatabaseConnection: DatabaseStatements { +pub trait SupportsTransactionalDDL { + #[inline] + fn supports_transactional_ddl(&self) -> bool { + false + } +} + +pub trait DatabaseConnection: DatabaseStatements + SupportsTransactionalDDL { fn batch_execute(&mut self, query: &str) -> StdResult<()>; fn execute<'b>(&mut self, query: &str, params: ToSqlParams<'b>) -> StdResult; diff --git a/migra-cli/src/database/mod.rs b/migra-cli/src/database/mod.rs index 7f176f3..a6e3f79 100644 --- a/migra-cli/src/database/mod.rs +++ b/migra-cli/src/database/mod.rs @@ -9,6 +9,7 @@ pub mod prelude { pub use super::adapter::{ToSql, ToSqlParams, TryFromSql}; pub use super::connection::{ AnyConnection, DatabaseConnection, DatabaseStatements, OpenDatabaseConnection, + SupportsTransactionalDDL, }; pub use super::migration::ManageMigration; pub use super::transaction::ManageTransaction; diff --git a/migra-cli/src/database/transaction.rs b/migra-cli/src/database/transaction.rs index a1a3fb0..a5945c4 100644 --- a/migra-cli/src/database/transaction.rs +++ b/migra-cli/src/database/transaction.rs @@ -48,14 +48,14 @@ where } pub fn maybe_with_transaction( - with: bool, + is_needed: bool, conn: &mut AnyConnection, trx_fn: &mut TrxFnMut, ) -> StdResult where TrxFnMut: FnMut(&mut AnyConnection) -> StdResult, { - if with { + if is_needed && conn.supports_transactional_ddl() { with_transaction(conn, trx_fn) } else { trx_fn(conn) diff --git a/migra-cli/tests/commands.rs b/migra-cli/tests/commands.rs index 7bb9e16..e9bed51 100644 --- a/migra-cli/tests/commands.rs +++ b/migra-cli/tests/commands.rs @@ -481,22 +481,6 @@ mod upgrade { Ok(()) })?; - #[cfg(feature = "mysql")] - inner("mysql_invalid", || { - use mysql::prelude::*; - - let pool = mysql::Pool::new(MYSQL_URL)?; - let mut conn = pool.get_conn()?; - - let articles_res = conn.query_drop("SELECT a.id FROM articles AS a"); - let persons_res = conn.query_drop("SELECT p.id FROM persons AS p"); - - assert!(articles_res.is_ok()); - assert!(persons_res.is_err()); - - Ok(()) - })?; - Ok(()) } @@ -533,23 +517,6 @@ mod upgrade { Ok(()) })?; - // TODO: Need to investigate how fix single transaction for Mysql - // #[cfg(feature = "mysql")] - // inner("mysql_invalid", || { - // use mysql::prelude::*; - - // let pool = mysql::Pool::new(MYSQL_URL)?; - // let mut conn = pool.get_conn()?; - - // let articles_res = conn.query_drop("SELECT a.id FROM articles AS a"); - // let persons_res = conn.query_drop("SELECT p.id FROM persons AS p"); - - // assert!(articles_res.is_err()); - // assert!(persons_res.is_err()); - - // Ok(()) - // })?; - Ok(()) } } diff --git a/migra-cli/tests/data/Migra_mysql_invalid.toml b/migra-cli/tests/data/Migra_mysql_invalid.toml deleted file mode 100644 index d388626..0000000 --- a/migra-cli/tests/data/Migra_mysql_invalid.toml +++ /dev/null @@ -1,4 +0,0 @@ -root = "./mysql_invalid" - -[database] -connection = "mysql://mysql:mysql@localhost:6001/migra_tests" diff --git a/migra-cli/tests/data/mysql_invalid/migrations/210218232851_create_articles/down.sql b/migra-cli/tests/data/mysql_invalid/migrations/210218232851_create_articles/down.sql deleted file mode 100644 index b625086..0000000 --- a/migra-cli/tests/data/mysql_invalid/migrations/210218232851_create_articles/down.sql +++ /dev/null @@ -1,3 +0,0 @@ --- This file should undo anything in `up.sql` - -DROP TABLE articles; diff --git a/migra-cli/tests/data/mysql_invalid/migrations/210218232851_create_articles/up.sql b/migra-cli/tests/data/mysql_invalid/migrations/210218232851_create_articles/up.sql deleted file mode 100644 index 2742046..0000000 --- a/migra-cli/tests/data/mysql_invalid/migrations/210218232851_create_articles/up.sql +++ /dev/null @@ -1,8 +0,0 @@ --- Your SQL goes here - -CREATE TABLE articles ( - id int AUTO_INCREMENT PRIMARY KEY, - title text NOT NULL CHECK (length(title) > 0), - content text NOT NULL, - created_at timestamp NOT NULL DEFAULT current_timestamp -); diff --git a/migra-cli/tests/data/mysql_invalid/migrations/210218233414_create_persons/down.sql b/migra-cli/tests/data/mysql_invalid/migrations/210218233414_create_persons/down.sql deleted file mode 100644 index c03d71a..0000000 --- a/migra-cli/tests/data/mysql_invalid/migrations/210218233414_create_persons/down.sql +++ /dev/null @@ -1,6 +0,0 @@ --- This file should undo anything in `up.sql` - -ALTER TABLE articles - DROP COLUMN author_person_id; - -DROP TABLE persons; diff --git a/migra-cli/tests/data/mysql_invalid/migrations/210218233414_create_persons/up.sql b/migra-cli/tests/data/mysql_invalid/migrations/210218233414_create_persons/up.sql deleted file mode 100644 index 073d809..0000000 --- a/migra-cli/tests/data/mysql_invalid/migrations/210218233414_create_persons/up.sql +++ /dev/null @@ -1,12 +0,0 @@ --- Your SQL goes here - -CREATE TABLE persons ( - id int AUTO_INCREMENT PRIMARY KEY - email varchar(256) NOT NULL UNIQUE - display_name text NOT NULL - created_at timestamp NOT NULL DEFAULT current_timestamp -); - -ALTER TABLE articles - ADD COLUMN author_person_id int NULL - REFERENCES persons (id) ON UPDATE CASCADE ON DELETE CASCADE; diff --git a/migra-cli/tests/data/postgres_invalid/migrations/210218233414_create_persons/up.sql b/migra-cli/tests/data/postgres_invalid/migrations/210218233414_create_persons/up.sql index 64ac772..ccae252 100644 --- a/migra-cli/tests/data/postgres_invalid/migrations/210218233414_create_persons/up.sql +++ b/migra-cli/tests/data/postgres_invalid/migrations/210218233414_create_persons/up.sql @@ -1,12 +1,14 @@ -- Your SQL goes here CREATE TABLE persons ( - id SERIAL PRIMARY KEY - email text NOT NULL UNIQUE - display_name text NOT NULL + id SERIAL PRIMARY KEY, + email text NOT NULL UNIQUE, + display_name text NOT NULL, created_at timestamp NOT NULL DEFAULT current_timestamp ); -ALTER TABLE articles +/* This table doesn't exist + ↓↓↓↓↓↓↓ */ +ALTER TABLE recipes ADD COLUMN author_person_id int NULL REFERENCES persons (id) ON UPDATE CASCADE ON DELETE CASCADE;