Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrap embedded migrations with enums #303

Open
superstator opened this issue Dec 29, 2023 · 8 comments
Open

Wrap embedded migrations with enums #303

superstator opened this issue Dec 29, 2023 · 8 comments
Labels
enhancement New feature or request

Comments

@superstator
Copy link
Contributor

Now that I have a way to iterate over migrations as I run them, I'd like to be able to match strongly against them. Something like:

let mut conn = Connection::open("test.db").unwrap();
    
for migration in embedded::migrations::runner().run_iter(&mut conn) {
    match migration?.try_into()? {
        EmbeddedMigration::Initial(_) => println!("Initialized database"),
        EmbeddedMigration::AddCarsAndMotosTable(m) => println!("Now we have cars: {m:?}"),
        _ => ()
    }
}

I think this could be done without any changes to Migration or Runner, but just by emitting an enum and impl TryFrom<Migration> with embed_migrations!. Is this something you'd be open to another PR for?

@jxs
Copy link
Member

jxs commented Jan 3, 2024

Interesting, can you disclose you usecase for this? And why comparing the name would not be enough?
Thanks!

@superstator
Copy link
Contributor Author

Sure, it's just an extension of the usecase for iteration. For example, if my app stores metadata in sqlite but also stores complex binary data on disk, I might need to initialize a directory structure after the initial migration, and then reshuffle a bunch of files during a later migration.

fn main() -> Result<(), Error> {
    let mut conn = Connection::open("test.db").unwrap();

    for migration in embedded::migrations::runner().run_iter(&mut conn) {
        match migration?.try_into()? {
            EmbeddedMigration::Initial(_) => init_filesystem(),
            EmbeddedMigration::AddCarsAndMotosTable(_) => migrate_v2(),
            _ => ()
        }
    }

    Ok(())
}

fn init_filesystem() -> Result<(), Error> {
    // create fs junk
    Ok(())
}

fn migrate_v2() -> Result<(), Error> {
    // update fs junk folowing db changes
    Ok(())
}

I can absolutely achieve this just matching names, but enums would allow for compile-time checking. My thought was to use the migration version as the enum discriminant so strictly speaking I'd be matching an i32 instead of a string which would be more efficient and much less error prone.

@jxs
Copy link
Member

jxs commented Feb 15, 2024

The idea seems interesting, but if it's a TryInto it means there's still going to be a runtime check, which I assume is converting the migration name from string into the matching enum right?

@jxs jxs added the enhancement New feature or request label Feb 15, 2024
@superstator
Copy link
Contributor Author

My thought was to have the macro output a TryInto impl that maps migration.version to the enum discriminant, so no runtime string comparisons. That way the only failure mode for TryInto would be somehow instantiating a Migration struct with an invalid version number, and I think the only obvious way for that to happen would be to monkey with the schema history table or delete applied migrations.

@jxs
Copy link
Member

jxs commented Feb 22, 2024

then I'd say it can be an Into right?

@superstator
Copy link
Contributor Author

It would have to panic on a bad migration version but otherwise sure

@jxs
Copy link
Member

jxs commented Feb 22, 2024

and isn't that most probably unreachable!?

@superstator
Copy link
Contributor Author

Off the top of my head the obvious way to get there (short of unsafe) would be to have two sets of embedded migrations for two databases, and then try to convert one into the wrapping enum for the other. I'm fine with that being a panic if you are, and it would eliminate the need to make changes in error.rs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants