From 905e9e7690ad81aabcb630e8ee77434cac99d9a2 Mon Sep 17 00:00:00 2001 From: emilpriver Date: Sat, 10 Feb 2024 12:06:12 +0100 Subject: [PATCH 1/5] Added transactions --- src/lib/database_drivers/libsql.rs | 1 + src/lib/database_drivers/maria.rs | 23 ++++++++++++++--------- src/lib/database_drivers/mod.rs | 1 + src/lib/database_drivers/mysql.rs | 23 ++++++++++++++--------- src/lib/database_drivers/postgres.rs | 23 ++++++++++++++--------- src/lib/database_drivers/sqlite.rs | 21 +++++++++++++++++++-- src/lib/migrate.rs | 18 +++++++++++++++--- 7 files changed, 78 insertions(+), 32 deletions(-) diff --git a/src/lib/database_drivers/libsql.rs b/src/lib/database_drivers/libsql.rs index 104c31b..2e59176 100644 --- a/src/lib/database_drivers/libsql.rs +++ b/src/lib/database_drivers/libsql.rs @@ -44,6 +44,7 @@ impl DatabaseDriver for LibSQLDriver { fn execute<'a>( &'a mut self, query: &'a str, + _run_in_transaction: bool, ) -> Pin> + '_>> { let fut = async move { let queries = query diff --git a/src/lib/database_drivers/maria.rs b/src/lib/database_drivers/maria.rs index 79a768b..5100cb5 100644 --- a/src/lib/database_drivers/maria.rs +++ b/src/lib/database_drivers/maria.rs @@ -79,18 +79,23 @@ impl DatabaseDriver for MariaDBDriver { fn execute<'a>( &'a mut self, query: &'a str, + run_in_transaction: bool, ) -> Pin> + '_>> { let fut = async move { - let mut tx = self.db.begin().await?; - - match tx.execute(query).await { - Ok(_) => { - tx.commit().await?; - } - Err(e) => { - error!("Error executing query: {}", e); - tx.rollback().await?; + if run_in_transaction { + let mut tx = self.db.begin().await?; + match tx.execute(query).await { + Ok(_) => { + tx.commit().await?; + } + Err(e) => { + error!("Error executing query: {}", e); + tx.rollback().await?; + } } + return Ok(()); + } else { + self.db.execute(query).await?; } Ok(()) diff --git a/src/lib/database_drivers/mod.rs b/src/lib/database_drivers/mod.rs index 8a2d711..e788db0 100644 --- a/src/lib/database_drivers/mod.rs +++ b/src/lib/database_drivers/mod.rs @@ -23,6 +23,7 @@ pub trait DatabaseDriver { fn execute<'a>( &'a mut self, query: &'a str, + run_in_transaction: bool, ) -> Pin> + '_>>; // create database with the specific driver diff --git a/src/lib/database_drivers/mysql.rs b/src/lib/database_drivers/mysql.rs index 50772f8..4150341 100644 --- a/src/lib/database_drivers/mysql.rs +++ b/src/lib/database_drivers/mysql.rs @@ -78,18 +78,23 @@ impl DatabaseDriver for MySQLDriver { fn execute<'a>( &'a mut self, query: &'a str, + run_in_transaction: bool, ) -> Pin> + '_>> { let fut = async move { - let mut tx = self.db.begin().await?; - - match tx.execute(query).await { - Ok(_) => { - tx.commit().await?; - } - Err(e) => { - error!("Error executing query: {}", e); - tx.rollback().await?; + if run_in_transaction { + let mut tx = self.db.begin().await?; + match tx.execute(query).await { + Ok(_) => { + tx.commit().await?; + } + Err(e) => { + error!("Error executing query: {}", e); + tx.rollback().await?; + } } + return Ok(()); + } else { + self.db.execute(query).await?; } Ok(()) diff --git a/src/lib/database_drivers/postgres.rs b/src/lib/database_drivers/postgres.rs index 70cea4b..c26ca07 100644 --- a/src/lib/database_drivers/postgres.rs +++ b/src/lib/database_drivers/postgres.rs @@ -71,18 +71,23 @@ impl DatabaseDriver for PostgresDriver { fn execute<'a>( &'a mut self, query: &'a str, + run_in_transaction: bool, ) -> Pin> + '_>> { let fut = async move { - let mut tx = self.db.begin().await?; - - match tx.execute(query).await { - Ok(_) => { - tx.commit().await?; - } - Err(e) => { - error!("Error executing query: {}", e); - tx.rollback().await?; + if run_in_transaction { + let mut tx = self.db.begin().await?; + match tx.execute(query).await { + Ok(_) => { + tx.commit().await?; + } + Err(e) => { + error!("Error executing query: {}", e); + tx.rollback().await?; + } } + return Ok(()); + } else { + self.db.execute(query).await?; } Ok(()) diff --git a/src/lib/database_drivers/sqlite.rs b/src/lib/database_drivers/sqlite.rs index 8886098..c452eb6 100644 --- a/src/lib/database_drivers/sqlite.rs +++ b/src/lib/database_drivers/sqlite.rs @@ -1,5 +1,5 @@ use crate::database_drivers::DatabaseDriver; -use anyhow::Result; +use anyhow::{bail, Result}; use libsql_client::{de, local::Client}; use std::fs::{self, File}; use std::future::Future; @@ -48,15 +48,32 @@ impl DatabaseDriver for SqliteDriver { fn execute<'a>( &'a mut self, query: &'a str, + run_in_transaction: bool, ) -> Pin> + '_>> { let fut = async move { + if run_in_transaction { + self.db.execute("BEGIN;")?; + } + let queries = query .split(';') .map(|x| x.trim()) .filter(|x| !x.is_empty()) .collect::>(); for query in queries { - self.db.execute(query)?; + match self.db.execute(query) { + Ok(_) => {} + Err(e) => { + if run_in_transaction { + self.db.execute("ROLLBACK;")?; + } + bail!("{:?}", e); + } + } + } + + if run_in_transaction { + self.db.execute("COMMIT;")?; } Ok(()) }; diff --git a/src/lib/migrate.rs b/src/lib/migrate.rs index 2ce461e..731dc13 100644 --- a/src/lib/migrate.rs +++ b/src/lib/migrate.rs @@ -55,8 +55,14 @@ pub async fn up( if !migrations.contains(&id) { info!("Running migration {}", id); let query = read_file_content(&f.1); + let run_in_transaction = query + .split_once('\n') + // by default do we want to run in a transaction + .unwrap_or(("transaction: yes", "")) + .0 + .contains("transaction: yes"); - database.execute(&query).await?; + database.execute(&query, run_in_transaction).await?; database.insert_schema_migration(&id).await?; } @@ -124,8 +130,14 @@ pub async fn down( Some(f) => { info!("Running rollback for {}", migration); let query = read_file_content(&f.1); - - database.execute(&query).await?; + let run_in_transaction = query + .split_once('\n') + // by default do we want to run in a transaction + .unwrap_or(("transaction: yes", "")) + .0 + .contains("transaction: yes"); + + database.execute(&query, run_in_transaction).await?; database .remove_schema_migration(migration.to_string().as_str()) From f0cb379d781c9a6632eaf7347e1d8d0c2e215b13 Mon Sep 17 00:00:00 2001 From: emilpriver Date: Sat, 10 Feb 2024 12:13:27 +0100 Subject: [PATCH 2/5] added test for transactions --- src/lib/migrate.rs | 16 +++------------- src/lib/utils.rs | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/src/lib/migrate.rs b/src/lib/migrate.rs index 731dc13..b970801 100644 --- a/src/lib/migrate.rs +++ b/src/lib/migrate.rs @@ -1,5 +1,5 @@ -use crate::database_drivers; use crate::utils::{get_local_migrations, read_file_content}; +use crate::{database_drivers, utils}; use anyhow::{bail, Result}; use log::info; use std::path::PathBuf; @@ -55,12 +55,7 @@ pub async fn up( if !migrations.contains(&id) { info!("Running migration {}", id); let query = read_file_content(&f.1); - let run_in_transaction = query - .split_once('\n') - // by default do we want to run in a transaction - .unwrap_or(("transaction: yes", "")) - .0 - .contains("transaction: yes"); + let run_in_transaction = utils::should_run_in_transaction(&query); database.execute(&query, run_in_transaction).await?; @@ -130,12 +125,7 @@ pub async fn down( Some(f) => { info!("Running rollback for {}", migration); let query = read_file_content(&f.1); - let run_in_transaction = query - .split_once('\n') - // by default do we want to run in a transaction - .unwrap_or(("transaction: yes", "")) - .0 - .contains("transaction: yes"); + let run_in_transaction = utils::should_run_in_transaction(&query); database.execute(&query, run_in_transaction).await?; diff --git a/src/lib/utils.rs b/src/lib/utils.rs index c3cb310..e58d96d 100644 --- a/src/lib/utils.rs +++ b/src/lib/utils.rs @@ -42,3 +42,40 @@ pub fn get_local_migrations(folder: &PathBuf, ending: &str) -> Result String { fs::read_to_string(path).unwrap() } + +pub fn should_run_in_transaction(query: &str) -> bool { + !query + .split_once('\n') + .unwrap_or(("", "")) + .0 + .contains("transaction: no") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_without_transaction_no_in_first_line() { + let query = "something else\ntransaction: no"; + assert_eq!(should_run_in_transaction(query), true); + } + + #[test] + fn test_with_empty_line() { + let query = ""; + assert_eq!(should_run_in_transaction(query), true); + } + + #[test] + fn test_with_transaction_yes_in_first_line() { + let query = "transaction: yes\nSELECT * FROM users"; + assert_eq!(should_run_in_transaction(query), true); + } + + #[test] + fn test_with_transaction_no_in_first_line() { + let query = "transaction: no\nSELECT * FROM users"; + assert_eq!(should_run_in_transaction(query), false); + } +} From 93f81c1a8e9407ba37c55dccbb48570c44058711 Mon Sep 17 00:00:00 2001 From: emilpriver Date: Sat, 10 Feb 2024 12:17:06 +0100 Subject: [PATCH 3/5] added space checking --- README.md | 6 ++++++ src/lib/utils.rs | 21 ++++++++++++++++----- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index c7a446c..878a840 100644 --- a/README.md +++ b/README.md @@ -146,6 +146,12 @@ DROP TABLE Persons; in the generated `.down.sql` file as this code would revert the creation of the table `Persons` +### Transactions + +Geni defaults to always run in transactions but if you want to prevent usage of transactions, add `transaction: no` as the first line of the migration file. +Then Geni won't use transactions for the specific migration. +This works for both up and down + ### Running migration Running migration can be done using diff --git a/src/lib/utils.rs b/src/lib/utils.rs index e58d96d..0979d15 100644 --- a/src/lib/utils.rs +++ b/src/lib/utils.rs @@ -44,11 +44,17 @@ pub fn read_file_content(path: &PathBuf) -> String { } pub fn should_run_in_transaction(query: &str) -> bool { - !query - .split_once('\n') - .unwrap_or(("", "")) - .0 - .contains("transaction: no") + let first_line = query.split_once('\n').unwrap_or(("", "")).0; + + if first_line.contains("transaction: no") { + return false; + } + + if first_line.contains("transaction:no") { + return false; + } + + return true; } #[cfg(test)] @@ -78,4 +84,9 @@ mod tests { let query = "transaction: no\nSELECT * FROM users"; assert_eq!(should_run_in_transaction(query), false); } + #[test] + fn test_with_transaction_no_in_first_line_without_space() { + let query = "transaction:no\nSELECT * FROM users"; + assert_eq!(should_run_in_transaction(query), false); + } } From 7b67e6558ae3183cc888c203aa31bacf96973c32 Mon Sep 17 00:00:00 2001 From: emilpriver Date: Sat, 10 Feb 2024 12:18:01 +0100 Subject: [PATCH 4/5] updated readme --- README.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/README.md b/README.md index 878a840..a8ea8a1 100644 --- a/README.md +++ b/README.md @@ -152,6 +152,17 @@ Geni defaults to always run in transactions but if you want to prevent usage of Then Geni won't use transactions for the specific migration. This works for both up and down +Example: + +```sql +-- transaction:no +CREATE TABLE table_2 ( + id INT PRIMARY KEY, + name VARCHAR(255) NOT NULL, + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP +); +``` + ### Running migration Running migration can be done using From 072bd9b848ce23c1a40ff600c1ba2b63484c42f4 Mon Sep 17 00:00:00 2001 From: emilpriver Date: Sat, 10 Feb 2024 12:31:53 +0100 Subject: [PATCH 5/5] changed libsql to use transactions if needed --- src/lib/database_drivers/libsql.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/lib/database_drivers/libsql.rs b/src/lib/database_drivers/libsql.rs index 2e59176..2ca4736 100644 --- a/src/lib/database_drivers/libsql.rs +++ b/src/lib/database_drivers/libsql.rs @@ -44,7 +44,7 @@ impl DatabaseDriver for LibSQLDriver { fn execute<'a>( &'a mut self, query: &'a str, - _run_in_transaction: bool, + run_in_transaction: bool, ) -> Pin> + '_>> { let fut = async move { let queries = query @@ -53,7 +53,18 @@ impl DatabaseDriver for LibSQLDriver { .map(Statement::new) .collect::>(); - self.db.batch(queries).await?; + if run_in_transaction { + self.db.batch(queries).await?; + } else { + for query in queries { + match self.db.execute(query).await { + Ok(_) => {} + Err(e) => { + bail!("{:?}", e); + } + } + } + } Ok(()) };