Feature/cargo sqlx migrate #171

JesperAxelsson · 2020-03-29T07:55:27Z

MVP migrator.
Can't create databases yet and no down migrations

mehcode · 2020-03-31T00:15:47Z

Thanks for the initial push @JesperAxelsson.

I plan to look get this initially merged in after we get a couple bugs squashed and push out v0.3.1 in a couple days.

D1plo1d · 2020-04-04T14:35:59Z

Excited to see this feature so I tried it out from your branch. I found that it works great for migrations that create a single table but trying to run a migration with multiple CREATE TABLE statements gives the following error:

Applying migration: '2020-04-04_14-29-08_create_initial_schema.sql'
thread 'main' panicked at 'Failed to run migration "2020-04-04_14-29-08_create_initial_schema.sql": Database(DatabaseError { message: "cannot insert multiple commands into a prepared statement", details: None, hint: None, table_name: None, column_name: None, constraint_name: None })', /home/[redacted]/.cargo/git/checkouts/sqlx-85e146549543ea70/fcd6ef4/cargo-sqlx/src/main.rs:159:9

JesperAxelsson · 2020-04-04T18:48:06Z

Good find! The issue is probably that I run the migration in a transaction, this causes issues with create database as well. Will take look at soon as I have some time over.

mehcode · 2020-04-07T20:30:36Z

cargo-sqlx/README.md

+
+
+##### Limitations
+- No down migrations! If you need down migrations, there are other more feature complete migrators to use.


I don't think down migrations should exist.

https://laraveldaily.com/still-need-migrations-taylor-says-no/

Generally I would recommend leap-frogging migrations as a migration should never be breaking on its own as that would equal downtime and make automatic migrations mostly pointless anyway.

If you made a mistake in a schema migration, make another to roll it back.

I agree. The only place where I use down migrations is during development. But that can be worked around if you have a good seed. The refresh feature mentioned in the podcast could be useful though.

I'll open an issue to talk about the commands but we should definitely have a refresh, recycle, or otherwise command that drops, creates, and then runs all migrations. Infinitely useful in development.

I'll open an issue to talk about the commands but we should definitely have a refresh, recycle, or otherwise command that drops, creates, and then runs all migrations. Infinitely useful in development.

I broadly agree but want to add a caveat here is around seed data.

But that can be worked around if you have a good seed

While this is an important point, good seed data typically targets the latest migration. If you specifically want to test and refine the migration against known heterogenous data (e.g. adding a column derived from data in an existing column), this becomes quite clumsy. Down migrations--even if they were only an option in dev somehow--are really a nice option for this kind of thing. In my experience working with larger and/or older datasets (1TB+ in my most recent case, larger before that) these kinds of migrations are common and laborious and having to blow away your entire DB as the only option for testing a migration in the presence of such data would be a total PITA.

mehcode · 2020-04-07T20:31:12Z

cargo-sqlx/README.md

+
+##### Limitations
+- No down migrations! If you need down migrations, there are other more feature complete migrators to use.
+- Only support postgres. Could be convinced to add other databases if there is need and easy to use database connection libs.


We need to lift the same style of database connection that is done in the macros crate:

https://github.com/launchbadge/sqlx/blob/master/sqlx-macros/src/lib.rs#L63

Is there a way to be general over a connection in sqlx?

No.. It wouldn't be easy to allow until lazy normalization gets finished in the Rust compiler due to our use of higher-kinded trait bounds.

See: rust-lang/rust#60471

mehcode · 2020-04-07T20:32:21Z

cargo-sqlx/src/main.rs

+// }
+
+fn add_migration_file(name: &str) {
+    use chrono::prelude::*;


I'd prefer to use time over chrono in here

Me too, unfortunately formatting is not something std::time does.

Stackoverflow: Format std::time output

If there is a way to do it with std::time I would be happy to change it.

I meant https://docs.rs/time/0.2.9/time/#formatting actually.

mehcode · 2020-04-07T20:33:47Z

cargo-sqlx/src/main.rs

+    use std::path::Path;
+    use std::path::PathBuf;
+
+    if !Path::new(MIGRATION_FOLDER).exists() {


You probably want to drop the exists check and just use fs::create_dir_all. This would support nested migration paths (once that is configurable) and remove the race condition here.

mehcode · 2020-04-07T20:34:31Z

cargo-sqlx/src/main.rs

+async fn main() {
+    let opt = Opt::from_args();
+
+    match opt {


commands should return anyhow::Result and we should catch and print a pretty error: ... message

mehcode · 2020-04-07T21:24:36Z

cargo-sqlx/src/main.rs

+    }
+
+    let dt = Utc::now();
+    let mut file_name = dt.format("%Y-%m-%d_%H-%M-%S").to_string();


We should probably have a larger discussion for file formats. I'd like some research into how other tools do it.

Sure, this is almost how ef core does it. They mush the numbers together like: %Y%m%d%H%M%S. I think knex does something similiar. Most other tools I have seen use some variation, like unix timestamps.

mehcode · 2020-04-07T21:28:12Z

cargo-sqlx/src/main.rs

+}
+
+fn load_migrations() -> Vec<Migration> {
+    let entries = fs::read_dir(&MIGRATION_FOLDER).expect("Could not find 'migrations' dir");


Maybe a glob crate? https://crates.io/crates/glob

I was thinking about it, but glob is a pretty big dependency that I would rather skip if it is not needed. If we start add more complicated rules I would be all for using a glob lib.

That's fair. The dependency tree is already pretty big because of async.

mehcode · 2020-04-07T21:28:38Z

cargo-sqlx/src/main.rs

+    migrations
+}
+
+async fn run_migrations() {


Use -> anyhow::Result everywhere with ? or .context("")? where needed

mehcode · 2020-04-07T21:28:53Z

cargo-sqlx/src/main.rs

+    dotenv().ok();
+    let db_url = env::var("DATABASE_URL").expect("Failed to find 'DATABASE_URL'");
+
+    let mut pool = PgPool::new(&db_url)


Use a single connection, not a pool. No need for a migrator.

mehcode · 2020-04-07T21:29:29Z

cargo-sqlx/src/main.rs

+        }
+        println!("Applying migration: '{}'", mig.name);
+
+        sqlx::query(&mig.sql)


You want to use raw or unprepared queries for this as they will allow batch execution.

tx.execute(&*mig.sql).await

mehcode · 2020-04-07T21:29:58Z

cargo-sqlx/src/main.rs

+}
+
+async fn create_migration_table(mut pool: &PgPool) {
+    sqlx::query(


same as above, use a raw query conn.execute(string) instead of `query(string).execute(conn)

mehcode · 2020-04-07T21:35:01Z

Thanks for the awesome start @JesperAxelsson. Let me know how much or how little you want to contribute moving forward. I know you had the repo listed as "passively maintained" before. I added you as a collaborator.

I left a ton of comments here if you or someone else wants to tackle them to iterate on this. I'm going to circle back to this likely within a week~ if the notes haven't been looked at yet.

JesperAxelsson · 2020-04-10T10:18:43Z

Thanks for awesome comments! Really learning a lot here. I will try to put some time in every now and then but. Of course feel free to make changes you feel is necessary.

How do you want to handle commits? Should I work via pull request from a fork or are you comfortable with me with in the main repo?

mehcode · 2020-04-10T16:56:03Z

How do you want to handle commits? Should I work via pull request from a fork or are you comfortable with me with in the main repo?

I want this to be Rust's SQL toolkit. Design stuff we should discuss. But feel free to push directly unless you want to formally talk about the changes.

JesperAxelsson added 2 commits March 29, 2020 09:39

Added migration tool to sqlx

e577358

Remove LICENSE and change homepage

fcd6ef4

mehcode reviewed Apr 7, 2020

View reviewed changes

mehcode merged commit 7038721 into launchbadge:master Apr 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/cargo sqlx migrate #171

Feature/cargo sqlx migrate #171

JesperAxelsson commented Mar 29, 2020

mehcode commented Mar 31, 2020

D1plo1d commented Apr 4, 2020

JesperAxelsson commented Apr 4, 2020

mehcode Apr 7, 2020

JesperAxelsson Apr 10, 2020

mehcode Apr 10, 2020

bjeanes May 24, 2020

mehcode Apr 7, 2020

JesperAxelsson Apr 10, 2020

mehcode Apr 10, 2020

mehcode Apr 7, 2020

JesperAxelsson Apr 10, 2020

mehcode Apr 10, 2020

mehcode Apr 7, 2020

mehcode Apr 7, 2020

mehcode Apr 7, 2020

JesperAxelsson Apr 10, 2020

mehcode Apr 7, 2020

JesperAxelsson Apr 10, 2020

mehcode Apr 10, 2020

mehcode Apr 7, 2020

mehcode Apr 7, 2020

mehcode Apr 7, 2020

mehcode Apr 7, 2020

mehcode commented Apr 7, 2020

JesperAxelsson commented Apr 10, 2020

mehcode commented Apr 10, 2020



		##### Limitations
		- No down migrations! If you need down migrations, there are other more feature complete migrators to use.

Feature/cargo sqlx migrate #171

Feature/cargo sqlx migrate #171

Conversation

JesperAxelsson commented Mar 29, 2020

mehcode commented Mar 31, 2020

D1plo1d commented Apr 4, 2020

JesperAxelsson commented Apr 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mehcode commented Apr 7, 2020

JesperAxelsson commented Apr 10, 2020

mehcode commented Apr 10, 2020