Option for changing the default migration table name from __diesel_schema_migrations

[kriswuollett]

Running different applications, or multiple copies of them, in the same database instance, or even the same schema if one is restricted to it, it would be great if diesel supported changing the migration table name used, e.g., the request in #1728 may be satisfied by this.

Different roles with privileges granted by a higher-level admin allow such a structure to be safely set up so different migrations will not clobber each other. Not too much different than managing infrastructure with Terraform with different modules and devops cloud IAM accounts. A library like Diesel isn't directly responsible for database access security, so the anti-clobber argument against a feature request like this really isn't relevant IMO.

If it is reasonably straightforward, I'd be completely willing to do a PR to get this to work. See launchbadge/sqlx#2725 for example. It's just a matter if project is willing to take on this feature and accept a PR (if it doesn't require a lot of refactoring).

[weiznich]

I think I would be open for such a change, as long as:

• It's backward compatible (no breaking change)
• It doesn't require duplicating large amounts of the API.

From a technical point of view that likely just means that you need to provide another type that implements MigrationHarness in such a way that it allows passing the table as part of that struct. I'm happy to accept a PR (including tests + documentation) for that, although you can likely implement that today as third party outside of diesel.

[kriswuollett]

OK. I'll take a look at that.

[kriswuollett]

I'm going to take a pass on making a PR since migrations relies upon the core libraries instead of raw SQL for what should be simple two column table, which could just get the table name inlined after a regular expression check:

diesel/diesel_migrations/src/migration_harness.rs

Lines 19 to 24 in cf77884

	diesel::table! {
	__diesel_schema_migrations (version) {
	version -> VarChar,
	run_on -> Timestamp,
	}
	}

I'd believe that is theoretically possible now, especially since there is the sql_name mechanism to avoid Rust reserved words, but its too much to understand the internals of how things work, for a relatively new Rust developer, since it isn't the advertised happy path of using diesel with the macros.

Using core libraries themselves for such a thing usually is a great idea. However the issue I see with this and other similar Rust compile-time/DSL query builders is that they suffer which I'd consider to be in the class of the robot legs problem and so they can't support the use case of database sharding. For example in a trading system you may want to shard in two dimensions: date and instrument name range (which itself could change over time). Also think of the multi-tenant SaaS use case, what if you need to join the same type of tables across schemas? Even implementing search path on the connection mentioned back in #1045 won't even help. Also related is the issue that SQLite doesn't support schemas. Maybe a goal of diesel v3.0 to easier support "table schema instances" at runtime?

[weiznich]

I'd believe that is theoretically possible now, especially since there is the sql_name mechanism to avoid Rust reserved words, but its too much to understand the internals of how things work, for a relatively new Rust developer, since it isn't the advertised happy path of using diesel with the macros.

I think we might have a misunderstanding here of what the desired outcome of such a change would be. To write my idea again in more clear words:

You do not need to touch any of the existing code. This explicitly includes the linked table definition.
What needs to be done is to provide some new type that implements MigrationHarness. Lets just name it CustomTableNameHarness for now. As thats a new type you have complete control around how to implement the three required functions of the linked trait, which essentially means you can just ignore the existing table definition and do whatever fits the requirements there. That would include using diesel::sql_query to implement the necessary queries or (better) use the types provided by diesel_dynamic_schema in combination with the query builder. I think I personally would prefer the later variant as it completely removes the possibility of having "bad" identifiers by handling all the necessary escaping internally.
I think we might end up with the following definition:

struct CustomTableNameHarness<C> {
     conn: & C, // thats the connection,
     table: Table,
     version: Column,
     run_on: Column,
}

• a matching implementation of MigrationHarness which essentially closely mirrors the queries of the existing default implementation.

Also think of the multi-tenant SaaS use case, what if you need to join the same type of tables across schemas? Even implementing search path on the connection mentioned back in #1045 won't even help. Also related is the issue that SQLite doesn't support schemas. Maybe a goal of diesel v3.0 to easier support "table schema instances" at runtime?

I'm happy to discuss design suggestions for that. Other than that to get expectations clear: This is not something that has priority for me as I personally need that, neither do the projects I care for need it. In addition it seems like an use case that almost entirely seems to occurs in commercial environments, which is on its own nothing bad. But that means for me that I likely wont spend my free time on priorizing it over other stuff, nor that I will release a new major diesel version to just support this feature. (Additionally the mentioned use case is already somewhat possible by either providing all table definitions of the different schemas at compile time, or by using diesel_dynamic_schema or by providing your own custom dsl (extension).

[kriswuollett]

Understood regarding the project being volunteer-run and develops mostly on as features needed basis.

I was suggesting, more realistically hoping, that it would have been an easy change that would be useful to potential new users of diesel when they stop by to check it out.

I do think it may be possible if the migrator was mostly an actual implementation and not a trait so it could store migration metadata, instead of inside a migration step itself, and had a had a reference to a connection, it could then use more of a dynamic schema like you point out for the later use case I mentioned. But I'd think migration implementation details would need to be moved out of diesel and diesel-cli packages as well as seen in my draft pr (just for exploration purposes). So that sort of large refactoring change is really unrealistic time and process-wise for an outside contributor.

In any case, I've just been looking for something Rust-based for embedded SQLite migrations that has configurable migration table name that may already exist. I already use alembic for server-based databases.

[weiznich]

At this point I'm honestly not sure if you read what I wrote above. Your draft PR does exactly what my text above told you (several times!) not to do. Especially it:

• Changes the existing API
• Tries to somehow force that into the existing implementation.

Both points are a no-go for the suggested feature as pointed out several times before!

I do think it may be possible if the migrator was mostly an actual implementation and not a trait so it could store migration metadata, instead of inside a migration step itself, and had a had a reference to a connection,

The main point is: Given the trait instead of an concrete implementation it's even easier to do that as you don't need to touch any of the existing implementations! You literally just need to implement a trait with 3 methods for a new migration harness type. That's all, all other migration functionality will just work afterwards. You don't even need to do that in diesel_migrations itself you can do that in whatever crate you want. I would guess it's something like 50 lines of code in diesel_migrations. I'm happy to help with figuring out the details if you ever spend the time updating your PR to what I've actually suggested.

[Sytten]

I came here since I wanted to a build a seeding migrations system separate from the schema migrations. The MigrationHarness trait is honestly very good and extendable. I am not even sure a custom harness really needs to live in diesel, a guide on how to build one would probably be enough.

For reference this is my custom harness:

use diesel::migration::{Migration, MigrationVersion, Result};
use diesel::sqlite::Sqlite;
use diesel::{Connection, ExpressionMethods, QueryDsl, RunQueryDsl, SqliteConnection};
use diesel_migrations::MigrationHarness;

diesel::table! {
    __caido_seed_migrations (version) {
        version -> VarChar,
        run_on -> Timestamp,
    }
}

pub struct SeedHarness<'a> {
    connection: &'a mut SqliteConnection,
}

impl<'a> SeedHarness<'a> {
    pub fn new(connection: &'a mut SqliteConnection) -> Self {
        Self { connection }
    }
}

impl<'a> MigrationHarness<Sqlite> for SeedHarness<'a> {
    fn run_migration(
        &mut self,
        migration: &dyn Migration<Sqlite>,
    ) -> Result<MigrationVersion<'static>> {
        let apply_migration = |conn: &mut SqliteConnection| -> Result<()> {
            migration.run(conn)?;
            diesel::insert_into(__caido_seed_migrations::table)
                .values(__caido_seed_migrations::version.eq(migration.name().version().as_owned()))
                .execute(conn)?;
            Ok(())
        };

        if migration.metadata().run_in_transaction() {
            self.connection.transaction(apply_migration)?;
        } else {
            apply_migration(self.connection)?;
        }
        Ok(migration.name().version().as_owned())
    }

    fn revert_migration(
        &mut self,
        _migration: &dyn Migration<Sqlite>,
    ) -> Result<MigrationVersion<'static>> {
        Err("Seeds are not revertable".into())
    }

    fn applied_migrations(&mut self) -> Result<Vec<MigrationVersion<'static>>> {
        Ok(__caido_seed_migrations::table
            .select(__caido_seed_migrations::version)
            .order(__caido_seed_migrations::version.desc())
            .load(self.connection)?)
    }
}

[weiznich]

Thanks for providing this example 👍
Mind submitting a PR that adds it to the documentation?

[Sytten]

Will do!