From f071520b28a718478ef0134dfb62e2dd2b22ae46 Mon Sep 17 00:00:00 2001 From: Patrick Sullivan Date: Mon, 23 Dec 2024 11:18:26 -0600 Subject: [PATCH] fix: broken MySQL migration for dropping index (#5436) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The migration to drop an index on the `folders` table fails for MySQL. The `DROP INDEX` statement in MySQL requires specifying the table name but including the table name is not allowed for PostgreSQL and Sqlite. ⚠️ **Migration change:** This PR modifies an existing migration, so we should be very careful not to alter the behavior of a migration that has already been applied to users' databases. In this case, this PR only changes the behavior of the `aler_folder_table_idx` migration when the database backend is MySQL. Prior to this PR that migration would panic when ran on a MySQL database, so no user with a MySQL database will have been able to successfully run this migration yet. Therefore there should be not risk of creating diverging migration behavior in this PR. ## Summary by CodeRabbit - **Chores** - Updated migration logic for the `folders` table to enhance indexing clarity. - Improved index drop operations for the dashboards, alerts, and search queue tables by specifying the associated tables. - Introduced a new CLI command `seaorm-rollback` for rolling back migration steps, with options to rollback all or the last N migrations. - Added an asynchronous function to facilitate migration rollbacks with optional step specification. --- src/cli/basic/cli.rs | 30 +++++++++++++++++++ .../m20241114_000001_create_folders_table.rs | 14 +++++++-- ...20241119_000001_create_dashboards_table.rs | 1 + ...241204_143100_create_table_search_queue.rs | 14 +++++++-- .../m20241209_120000_create_alerts_table.rs | 8 ++++- ...20241217_154900_alter_folders_table_idx.rs | 8 ++++- src/infra/src/table/mod.rs | 6 ++++ 7 files changed, 75 insertions(+), 6 deletions(-) diff --git a/src/cli/basic/cli.rs b/src/cli/basic/cli.rs index cffec1c9bf..cc78a6f7d6 100644 --- a/src/cli/basic/cli.rs +++ b/src/cli/basic/cli.rs @@ -117,6 +117,16 @@ pub async fn cli() -> Result { .help("the parquet file name"), ), clap::Command::new("migrate-schemas").about("migrate from single row to row per schema version"), + clap::Command::new("seaorm-rollback").about("rollback SeaORM migration steps") + .subcommand( + clap::Command::new("all") + .about("rollback all SeaORM migration steps") + ) + .subcommand( + clap::Command::new("last") + .about("rollback last N SeaORM migration steps") + .arg(clap::Arg::new("N").help("number of migration steps to rollback (default is 1)").value_parser(clap::value_parser!(u32))) + ), ]) .get_matches(); @@ -274,6 +284,26 @@ pub async fn cli() -> Result { #[allow(deprecated)] migration::schema::run().await? } + "seaorm-rollback" => match command.subcommand() { + Some(("all", _)) => { + println!("Rolling back all"); + infra::table::down(None).await? + } + Some(("last", sub_matches)) => { + let n = sub_matches + .get_one::("N") + .map(|n| n.to_owned()) + .unwrap_or(1); + println!("Rolling back {n}"); + infra::table::down(Some(n)).await? + } + Some((name, _)) => { + return Err(anyhow::anyhow!("unsupported sub command: {name}")); + } + None => { + return Err(anyhow::anyhow!("missing sub command")); + } + }, _ => { return Err(anyhow::anyhow!("unsupported sub command: {name}")); } diff --git a/src/infra/src/table/migration/m20241114_000001_create_folders_table.rs b/src/infra/src/table/migration/m20241114_000001_create_folders_table.rs index 2cc5ccc936..15d4b0faac 100644 --- a/src/infra/src/table/migration/m20241114_000001_create_folders_table.rs +++ b/src/infra/src/table/migration/m20241114_000001_create_folders_table.rs @@ -36,10 +36,20 @@ impl MigrationTrait for Migration { async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { manager - .drop_index(Index::drop().name(FOLDERS_ORG_FOLDER_ID_IDX).to_owned()) + .drop_index( + Index::drop() + .name(FOLDERS_ORG_FOLDER_ID_IDX) + .table(Folders::Table) + .to_owned(), + ) .await?; manager - .drop_index(Index::drop().name(FOLDERS_ORG_IDX).to_owned()) + .drop_index( + Index::drop() + .name(FOLDERS_ORG_IDX) + .table(Folders::Table) + .to_owned(), + ) .await?; manager .drop_table(Table::drop().table(Folders::Table).to_owned()) diff --git a/src/infra/src/table/migration/m20241119_000001_create_dashboards_table.rs b/src/infra/src/table/migration/m20241119_000001_create_dashboards_table.rs index d19197f967..7a436a43ff 100644 --- a/src/infra/src/table/migration/m20241119_000001_create_dashboards_table.rs +++ b/src/infra/src/table/migration/m20241119_000001_create_dashboards_table.rs @@ -38,6 +38,7 @@ impl MigrationTrait for Migration { .drop_index( Index::drop() .name(DASHBOARDS_FOLDER_ID_DASHBOARD_ID_IDX) + .table(Dashboards::Table) .to_owned(), ) .await?; diff --git a/src/infra/src/table/migration/m20241204_143100_create_table_search_queue.rs b/src/infra/src/table/migration/m20241204_143100_create_table_search_queue.rs index e5f83b4878..25ce41de1c 100644 --- a/src/infra/src/table/migration/m20241204_143100_create_table_search_queue.rs +++ b/src/infra/src/table/migration/m20241204_143100_create_table_search_queue.rs @@ -17,10 +17,20 @@ impl MigrationTrait for Migration { async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { manager - .drop_index(Index::drop().name(SEARCH_QUEUE_WORK_GROUP_IDX).to_owned()) + .drop_index( + Index::drop() + .name(SEARCH_QUEUE_WORK_GROUP_IDX) + .table(SearchQueue::Table) + .to_owned(), + ) .await?; manager - .drop_index(Index::drop().name(SEARCH_QUEUE_CREATED_AT_IDX).to_owned()) + .drop_index( + Index::drop() + .name(SEARCH_QUEUE_CREATED_AT_IDX) + .table(SearchQueue::Table) + .to_owned(), + ) .await?; manager .drop_table(Table::drop().table(SearchQueue::Table).to_owned()) diff --git a/src/infra/src/table/migration/m20241209_120000_create_alerts_table.rs b/src/infra/src/table/migration/m20241209_120000_create_alerts_table.rs index 43b8144792..ee77c490a1 100644 --- a/src/infra/src/table/migration/m20241209_120000_create_alerts_table.rs +++ b/src/infra/src/table/migration/m20241209_120000_create_alerts_table.rs @@ -40,12 +40,18 @@ impl MigrationTrait for Migration { async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { manager - .drop_index(Index::drop().name(ALERTS_FOLDER_ID_IDX).to_owned()) + .drop_index( + Index::drop() + .name(ALERTS_FOLDER_ID_IDX) + .table(Alerts::Table) + .to_owned(), + ) .await?; manager .drop_index( Index::drop() .name(ALERTS_ORG_STREAM_TYPE_STREAM_NAME_NAME_IDX) + .table(Alerts::Table) .to_owned(), ) .await?; diff --git a/src/infra/src/table/migration/m20241217_154900_alter_folders_table_idx.rs b/src/infra/src/table/migration/m20241217_154900_alter_folders_table_idx.rs index 275d4c2f92..5334931fa7 100644 --- a/src/infra/src/table/migration/m20241217_154900_alter_folders_table_idx.rs +++ b/src/infra/src/table/migration/m20241217_154900_alter_folders_table_idx.rs @@ -31,7 +31,12 @@ const NEW_FOLDERS_ORG_TYPE_FOLDER_ID_IDX: &str = "folders_org_type_folder_id_idx impl MigrationTrait for Migration { async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { manager - .drop_index(Index::drop().name(OLD_FOLDERS_ORG_FOLDER_ID_IDX).to_owned()) + .drop_index( + Index::drop() + .name(OLD_FOLDERS_ORG_FOLDER_ID_IDX) + .table(Folders::Table) + .to_owned(), + ) .await?; manager .create_index(create_new_folders_org_type_folder_id_idx_stmnt()) @@ -44,6 +49,7 @@ impl MigrationTrait for Migration { .drop_index( Index::drop() .name(NEW_FOLDERS_ORG_TYPE_FOLDER_ID_IDX) + .table(Folders::Table) .to_owned(), ) .await?; diff --git a/src/infra/src/table/mod.rs b/src/infra/src/table/mod.rs index 8002c45f95..3ba0d34eac 100644 --- a/src/infra/src/table/mod.rs +++ b/src/infra/src/table/mod.rs @@ -41,6 +41,12 @@ pub async fn migrate() -> Result<(), anyhow::Error> { Ok(()) } +pub async fn down(steps: Option) -> Result<(), anyhow::Error> { + let client = ORM_CLIENT.get_or_init(connect_to_orm).await; + Migrator::down(client, steps).await?; + Ok(()) +} + /// Acquires a lock on the SQLite client if SQLite is configured as the meta store. /// /// # Returns