diff --git a/schema-engine/connectors/sql-schema-connector/src/sql_schema_differ.rs b/schema-engine/connectors/sql-schema-connector/src/sql_schema_differ.rs index 31d237fd99b..4e1b9c52a85 100644 --- a/schema-engine/connectors/sql-schema-connector/src/sql_schema_differ.rs +++ b/schema-engine/connectors/sql-schema-connector/src/sql_schema_differ.rs @@ -40,7 +40,7 @@ pub(crate) fn calculate_steps( flavour.push_enum_steps(&mut steps, &db); flavour.push_alter_sequence_steps(&mut steps, &db); - steps.sort(); + sort_migration_steps(&mut steps, &db); steps } @@ -541,3 +541,81 @@ fn is_prisma_implicit_m2m_fk(fk: ForeignKeyWalker<'_>) -> bool { fn all_match(a: &mut dyn ExactSizeIterator, b: &mut dyn ExactSizeIterator) -> bool { a.len() == b.len() && a.zip(b).all(|(a, b)| a == b) } + +fn sort_migration_steps(steps: &mut [SqlMigrationStep], db: &DifferDatabase<'_>) { + dbg!(&steps); + steps.sort_by(|a, b| match (a, b) { + // TODO: does this define a total order??? + (SqlMigrationStep::DropIndex { index_id }, SqlMigrationStep::AlterTable(alter_table)) => { + if move_drop_unique_index_after_creating_pk(db, *index_id, alter_table) { + std::cmp::Ordering::Greater + } else { + a.cmp(b) + } + } + _ => a.cmp(b), + }); +} + +fn move_drop_unique_index_after_creating_pk( + db: &DifferDatabase<'_>, + index_id: IndexId, + alter_table: &AlterTable, +) -> bool { + let index = db.schemas.previous.describer_schema.walk(index_id); + dbg!(index.column_names().collect::>()); + + if !index.is_unique() { + return false; + } + + if alter_table.table_ids.previous != index.table().id { + return false; + } + + if db + .schemas + .previous + .describer_schema + .walk(alter_table.table_ids.previous) + .primary_key() + .is_some() + { + return false; + } + + let Some(pk_columns) = db + .schemas + .next + .describer_schema + .walk(alter_table.table_ids.next) + .primary_key_columns() + else { + return false; + }; + + if !all_match(&mut index.column_names(), &mut pk_columns.map(|col| col.name())) { + return false; + } + + let added_pk_in_this_alter_stmt = alter_table + .changes + .iter() + .any(|change| matches!(change, TableChange::AddPrimaryKey)); + + if !added_pk_in_this_alter_stmt { + return false; + } + + let dropped_index_column_in_this_alter_stmt = alter_table.changes.iter().any(|change| match change { + TableChange::DropColumn { column_id } => index.contains_column(*column_id), + TableChange::DropAndRecreateColumn { column_id, .. } => index.contains_column(column_id.previous), + _ => false, + }); + + if dropped_index_column_in_this_alter_stmt { + return false; + } + + true +} diff --git a/schema-engine/sql-migration-tests/tests/migrations/diff.rs b/schema-engine/sql-migration-tests/tests/migrations/diff.rs index ed9c15bc019..7d3df3f19de 100644 --- a/schema-engine/sql-migration-tests/tests/migrations/diff.rs +++ b/schema-engine/sql-migration-tests/tests/migrations/diff.rs @@ -96,7 +96,7 @@ fn from_unique_index_to_without(mut api: TestApi) { expected_printed_messages.assert_debug_eq(&host.printed_messages.lock().unwrap()); } -#[test_connector(tags(Sqlite, Mysql, Postgres, CockroachDb, Mssql))] +#[test_connector] fn from_unique_index_to_pk(mut api: TestApi) { let tempdir = tempfile::tempdir().unwrap(); let host = Arc::new(TestConnectorHost::default()); diff --git a/schema-engine/sql-migration-tests/tests/migrations/indexes.rs b/schema-engine/sql-migration-tests/tests/migrations/indexes.rs index 9b534349c37..200fcede04d 100644 --- a/schema-engine/sql-migration-tests/tests/migrations/indexes.rs +++ b/schema-engine/sql-migration-tests/tests/migrations/indexes.rs @@ -975,3 +975,81 @@ fn changing_normal_index_to_a_fulltext_index(api: TestApi) { table.assert_index_on_columns(&["a", "b"], |index| index.assert_is_fulltext()) }); } + +#[test_connector] +fn changing_unique_to_pk_works(api: TestApi) { + let dm1 = indoc! {r#" + model A { + id Int @unique + name String? + links C[] + } + + model B { + id Int @unique + name String? + links C[] + } + + model C { + a_id Int + b_id Int + a A @relation(fields: [a_id], references: [id]) + b B @relation(fields: [b_id], references: [id]) + + @@unique([a_id, b_id]) + } + "#}; + + api.schema_push_w_datasource(dm1).send().assert_green(); + + api.assert_schema() + .assert_table("A", |table| { + table.assert_index_on_columns(&["id"], |index| index.assert_is_unique()) + }) + .assert_table("B", |table| { + table.assert_index_on_columns(&["id"], |index| index.assert_is_unique()) + }) + .assert_table("C", |table| { + table.assert_index_on_columns(&["a_id", "b_id"], |index| index.assert_is_unique()) + }); + + let dm2 = indoc! {r#" + model A { + id Int @id + name String? + links C[] + } + + model B { + id Int @id + name String? + links C[] + } + + model C { + a_id Int + b_id Int + a A @relation(fields: [a_id], references: [id]) + b B @relation(fields: [b_id], references: [id]) + + @@id([a_id, b_id]) + } + "#}; + + api.schema_push_w_datasource(dm2).send().assert_green(); + + api.assert_schema() + .assert_table("A", |table| { + table.assert_indexes_count(1); + table.assert_pk(|pk| pk.assert_columns(&["id"])) + }) + .assert_table("B", |table| { + table.assert_indexes_count(1); + table.assert_pk(|pk| pk.assert_columns(&["id"])) + }) + .assert_table("C", |table| { + table.assert_indexes_count(1); + table.assert_pk(|pk| pk.assert_columns(&["a_id", "b_id"])) + }); +}