Skip to content

Commit 1171e96

Browse files
authored
fix(schema-engine): report rolled back migrations as unapplied (#5817)
## Summary Fixes prisma/prisma#29601. `diagnoseMigrationHistory` was matching a migration from the filesystem with any migration table row that had the same name. That included rows marked with `rolled_back_at`, even though those rows are ignored by migrate when they are not null. This changes the history comparison to ignore rolled-back rows when deciding which migrations are already present in the database. A rolled-back migration that still exists on disk is now reported as unapplied, which lets `migrate status` return a non-clean result instead of reporting the schema as up to date. ## Tests ```sh cargo fmt --check git diff --check make dev-sqlite set -a; . ./.test_database_urls/sqlite; set +a CARGO_BUILD_JOBS=2 RUST_TEST_THREADS=1 cargo test -p sql-migration-tests diagnose_migrations_history_reports_rolled_back_migration_as_unapplied -- --test-threads 1 CARGO_BUILD_JOBS=2 RUST_TEST_THREADS=1 cargo test -p sql-migration-tests diagnose_migration_history_tests -- --test-threads 1 CARGO_BUILD_JOBS=2 cargo check -p schema-core -p schema-commands -p sql-migration-tests ```
1 parent 3c6e192 commit 1171e96

3 files changed

Lines changed: 79 additions & 10 deletions

File tree

schema-engine/commands/src/commands/diagnose_migration_history.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,9 @@ pub async fn diagnose_migration_history(
9090

9191
// Check filesystem history against database history.
9292
for (index, fs_migration) in migrations_from_filesystem.migration_directories.iter().enumerate() {
93-
let corresponding_db_migration = migrations_from_database
94-
.iter()
95-
.find(|db_migration| db_migration.migration_name == fs_migration.migration_name());
93+
let corresponding_db_migration = migrations_from_database.iter().find(|db_migration| {
94+
db_migration.migration_name == fs_migration.migration_name() && db_migration.rolled_back_at.is_none()
95+
});
9696

9797
match corresponding_db_migration {
9898
Some(db_migration)
@@ -107,13 +107,17 @@ pub async fn diagnose_migration_history(
107107
}
108108
}
109109

110-
for (index, db_migration) in migrations_from_database.iter().enumerate() {
110+
let active_migrations_from_database = migrations_from_database
111+
.iter()
112+
.filter(|migration| migration.rolled_back_at.is_none());
113+
114+
for (index, db_migration) in active_migrations_from_database.enumerate() {
111115
let corresponding_fs_migration = migrations_from_filesystem
112116
.migration_directories
113117
.iter()
114118
.find(|fs_migration| db_migration.migration_name == fs_migration.migration_name());
115119

116-
if db_migration.finished_at.is_none() && db_migration.rolled_back_at.is_none() {
120+
if db_migration.finished_at.is_none() {
117121
diagnostics.failed_migrations.push(db_migration);
118122
}
119123

schema-engine/core/src/commands/diagnose_migration_history_cli.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ pub async fn diagnose_migration_history_cli(
3030

3131
// Check filesystem history against database history.
3232
for (index, fs_migration) in migrations_from_filesystem.migration_directories.iter().enumerate() {
33-
let corresponding_db_migration = migrations_from_database
34-
.iter()
35-
.find(|db_migration| db_migration.migration_name == fs_migration.migration_name());
33+
let corresponding_db_migration = migrations_from_database.iter().find(|db_migration| {
34+
db_migration.migration_name == fs_migration.migration_name() && db_migration.rolled_back_at.is_none()
35+
});
3636

3737
match corresponding_db_migration {
3838
Some(db_migration)
@@ -47,13 +47,17 @@ pub async fn diagnose_migration_history_cli(
4747
}
4848
}
4949

50-
for (index, db_migration) in migrations_from_database.iter().enumerate() {
50+
let active_migrations_from_database = migrations_from_database
51+
.iter()
52+
.filter(|migration| migration.rolled_back_at.is_none());
53+
54+
for (index, db_migration) in active_migrations_from_database.enumerate() {
5155
let corresponding_fs_migration = migrations_from_filesystem
5256
.migration_directories
5357
.iter()
5458
.find(|fs_migration| db_migration.migration_name == fs_migration.migration_name());
5559

56-
if db_migration.finished_at.is_none() && db_migration.rolled_back_at.is_none() {
60+
if db_migration.finished_at.is_none() {
5761
diagnostics.failed_migrations.push(db_migration);
5862
}
5963

schema-engine/sql-migration-tests/tests/migrations/diagnose_migration_history_tests.rs

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,67 @@ fn diagnose_migrations_history_can_detect_when_the_database_is_behind(api: TestA
282282
assert!(error_in_unapplied_migration.is_none());
283283
}
284284

285+
#[test_connector]
286+
fn diagnose_migrations_history_reports_rolled_back_migration_as_unapplied(api: TestApi) {
287+
let directory = api.create_migrations_directory();
288+
289+
let dm1 = api.datamodel_with_provider(
290+
r#"
291+
model Cat {
292+
id Int @id
293+
name String
294+
}
295+
"#,
296+
);
297+
298+
api.create_migration("initial", &dm1, &directory).send_sync();
299+
api.apply_migrations(&directory).send_sync();
300+
301+
let dm2 = api.datamodel_with_provider(
302+
r#"
303+
model Cat {
304+
id Int @id
305+
name String
306+
fluffiness Float
307+
}
308+
"#,
309+
);
310+
311+
let rolled_back_migration_name = api
312+
.create_migration("02_add_fluffiness", &dm2, &directory)
313+
.send_sync()
314+
.modify_migration(|script| {
315+
script.clear();
316+
script.push_str("SELECT YOLO;");
317+
})
318+
.into_output()
319+
.generated_migration_name;
320+
321+
api.apply_migrations(&directory).send_unwrap_err();
322+
api.mark_migration_rolled_back(&rolled_back_migration_name).send();
323+
324+
let DiagnoseMigrationHistoryOutput {
325+
drift,
326+
history,
327+
failed_migration_names,
328+
edited_migration_names,
329+
has_migrations_table,
330+
error_in_unapplied_migration,
331+
} = api.diagnose_migration_history(&directory).send_sync().into_output();
332+
333+
assert!(drift.is_none());
334+
assert!(failed_migration_names.is_empty());
335+
assert!(edited_migration_names.is_empty());
336+
assert_eq!(
337+
history,
338+
Some(HistoryDiagnostic::DatabaseIsBehind {
339+
unapplied_migration_names: vec![rolled_back_migration_name],
340+
})
341+
);
342+
assert!(has_migrations_table);
343+
assert!(error_in_unapplied_migration.is_none());
344+
}
345+
285346
#[test_connector]
286347
fn diagnose_migrations_history_can_detect_when_the_folder_is_behind(api: TestApi) {
287348
let directory = api.create_migrations_directory();

0 commit comments

Comments
 (0)