From cd426f0db0e602346a18251a94664edbeacd08df Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Fri, 6 Mar 2026 17:23:41 +0900 Subject: [PATCH 1/9] Feat prompt when add fk with required --- .../vespertide-cli/src/commands/revision.rs | 415 ++++++++++++++++-- 1 file changed, 385 insertions(+), 30 deletions(-) diff --git a/crates/vespertide-cli/src/commands/revision.rs b/crates/vespertide-cli/src/commands/revision.rs index fae37da..f8659d9 100644 --- a/crates/vespertide-cli/src/commands/revision.rs +++ b/crates/vespertide-cli/src/commands/revision.rs @@ -4,7 +4,7 @@ use std::path::Path; use anyhow::{Context, Result}; use chrono::Utc; use colored::Colorize; -use dialoguer::{Input, Select}; +use dialoguer::{Confirm, Input, Select}; use serde_json::Value; use tokio::fs; use vespertide_config::FileFormat; @@ -330,9 +330,36 @@ where Ok(()) } -/// Check that no AddColumn action adds a non-nullable FK column without a default. -/// This is logically impossible: existing rows can't satisfy the FK constraint. -fn check_non_nullable_fk_add_columns(plan: &MigrationPlan) -> Result<()> { +/// Reason why a table needs to be recreated. +#[derive(Debug, Clone, PartialEq, Eq)] +enum RecreateReason { + /// A new non-nullable FK column is being added. + AddColumnWithFk, + /// A FK constraint is being added to an existing non-nullable column. + AddFkToExistingColumn, +} + +/// A table that needs to be recreated because of a non-nullable FK constraint issue. +#[derive(Debug, Clone, PartialEq, Eq)] +struct RecreateTableRequired { + table: String, + column: String, + reason: RecreateReason, +} + +/// Find actions that require table recreation due to non-nullable FK constraints. +/// +/// Two cases are detected: +/// 1. **AddColumn with FK**: A new non-nullable FK column is being added (no default). +/// 2. **AddConstraint(FK) on existing column**: A FK constraint is being added to an +/// existing non-nullable column without a default. +/// +/// In both cases, existing rows cannot satisfy the foreign key constraint, +/// so the table must be recreated (DeleteTable + CreateTable). +fn find_non_nullable_fk_add_columns( + plan: &MigrationPlan, + current_models: &[TableDef], +) -> Vec { use std::collections::HashSet; // Collect FK columns from AddConstraint actions @@ -349,22 +376,156 @@ fn check_non_nullable_fk_add_columns(plan: &MigrationPlan) -> Result<()> { } } + // Collect columns being added in this migration (to distinguish new vs existing) + let mut added_columns: HashSet<(String, String)> = HashSet::new(); + for action in &plan.actions { + if let MigrationAction::AddColumn { table, column, .. } = action { + added_columns.insert((table.clone(), column.name.clone())); + } + } + + let mut result = Vec::new(); + + // Case 1: AddColumn with FK (new non-nullable FK column) for action in &plan.actions { if let MigrationAction::AddColumn { table, column, .. } = action { let has_fk = column.foreign_key.is_some() || fk_columns.contains(&(table.clone(), column.name.to_string())); if has_fk && !column.nullable && column.default.is_none() { - anyhow::bail!( - "Cannot add non-nullable foreign key column '{}' to existing table '{}': \ - existing rows cannot satisfy the foreign key constraint. \ - Make the column nullable, or add it with a default value that references an existing row.", - column.name, - table - ); + result.push(RecreateTableRequired { + table: table.clone(), + column: column.name.clone(), + reason: RecreateReason::AddColumnWithFk, + }); } } } - Ok(()) + + // Case 2: AddConstraint(FK) on existing non-nullable column + for action in &plan.actions { + if let MigrationAction::AddConstraint { + table, + constraint: TableConstraint::ForeignKey { columns, .. }, + } = action + { + for col_name in columns { + // Skip if this column is being added in this migration (handled by Case 1) + if added_columns.contains(&(table.clone(), col_name.to_string())) { + continue; + } + // Look up column in current models to check nullability + if let Some(model) = current_models + .iter() + .find(|m| m.name.as_str() == table.as_str()) + { + if let Some(col_def) = model + .columns + .iter() + .find(|c| c.name.as_str() == col_name.as_str()) + { + if !col_def.nullable && col_def.default.is_none() { + result.push(RecreateTableRequired { + table: table.clone(), + column: col_name.clone(), + reason: RecreateReason::AddFkToExistingColumn, + }); + } + } + } + } + } + } + + result +} + +/// Prompt the user to confirm table recreation. +/// Returns true if the user confirms, false otherwise. +#[cfg(not(tarpaulin_include))] +fn prompt_recreate_tables(tables: &[RecreateTableRequired]) -> Result { + println!( + "\n{} {}", + "\u{26a0}".bright_yellow(), + "The following tables need to be RECREATED:".bright_yellow() + ); + println!("{}", "\u{2500}".repeat(60).bright_black()); + + for item in tables { + let reason_msg = match item.reason { + RecreateReason::AddColumnWithFk => "adding required FK column", + RecreateReason::AddFkToExistingColumn => "adding FK to existing required column", + }; + println!( + " {} Table {} \u{2014} {} {}", + "\u{2022}".bright_cyan(), + item.table.bright_white(), + reason_msg, + item.column.bright_green() + ); + } + + println!("{}", "\u{2500}".repeat(60).bright_black()); + println!( + " {} {}", + "\u{26a0}".bright_red(), + "ALL DATA in these tables will be DELETED.".bright_red() + ); + + let confirmed = Confirm::new() + .with_prompt(" Proceed with table recreation?") + .default(false) + .interact() + .context("failed to read confirmation")?; + + Ok(confirmed) +} + +/// Rewrite the migration plan to recreate tables instead of adding columns. +/// Removes all column/constraint actions targeting the recreated tables and replaces +/// them with DeleteTable + CreateTable using the full target model. +fn rewrite_plan_for_recreation( + plan: &mut MigrationPlan, + recreate_tables: &[RecreateTableRequired], + current_models: &[TableDef], +) { + use std::collections::HashSet; + + let tables_to_recreate: HashSet<&str> = + recreate_tables.iter().map(|r| r.table.as_str()).collect(); + + // Remove all column/constraint actions targeting recreated tables + plan.actions.retain(|action| { + let table = match action { + MigrationAction::AddColumn { table, .. } + | MigrationAction::DeleteColumn { table, .. } + | MigrationAction::RenameColumn { table, .. } + | MigrationAction::ModifyColumnType { table, .. } + | MigrationAction::ModifyColumnNullable { table, .. } + | MigrationAction::ModifyColumnDefault { table, .. } + | MigrationAction::ModifyColumnComment { table, .. } + | MigrationAction::AddConstraint { table, .. } + | MigrationAction::RemoveConstraint { table, .. } => Some(table.as_str()), + _ => None, + }; + table.map_or(true, |t| !tables_to_recreate.contains(t)) + }); + + // Add DeleteTable + CreateTable for each recreated table + for table_name in &tables_to_recreate { + if let Some(model) = current_models + .iter() + .find(|m| m.name.as_str() == *table_name) + { + plan.actions.push(MigrationAction::DeleteTable { + table: table_name.to_string(), + }); + plan.actions.push(MigrationAction::CreateTable { + table: model.name.clone(), + columns: model.columns.clone(), + constraints: model.constraints.clone(), + }); + } + } } pub async fn cmd_revision(message: String, fill_with_args: Vec) -> Result<()> { @@ -384,9 +545,28 @@ pub async fn cmd_revision(message: String, fill_with_args: Vec) -> Resul return Ok(()); } - // Fail early: non-nullable FK column cannot be added to an existing table. - // Even with fill_with, there's no way to guarantee the value references a valid row. - check_non_nullable_fk_add_columns(&plan)?; + // Check for non-nullable FK columns being added to existing tables. + // These require table recreation because existing rows can't satisfy the FK constraint. + let recreate_tables = find_non_nullable_fk_add_columns(&plan, ¤t_models); + if !recreate_tables.is_empty() { + if !prompt_recreate_tables(&recreate_tables)? { + anyhow::bail!( + "Migration cancelled. To proceed without recreation, make the column nullable \ + or add it with a default value that references an existing row." + ); + } + rewrite_plan_for_recreation(&mut plan, &recreate_tables, ¤t_models); + + // Re-check: if plan is now empty after recreation rewrite, nothing to do + if plan.actions.is_empty() { + println!( + "{} {}", + "No changes detected.".bright_yellow(), + "Nothing to migrate.".bright_white() + ); + return Ok(()); + } + } // Reconstruct baseline schema for column type lookups let baseline_schema = schema_from_plans(&applied_plans) @@ -626,7 +806,7 @@ mod tests { } #[test] - fn check_non_nullable_fk_add_column_fails() { + fn find_non_nullable_fk_add_column_detects_recreate() { use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; let plan = MigrationPlan { id: String::new(), @@ -662,18 +842,15 @@ mod tests { }, ], }; - let result = check_non_nullable_fk_add_columns(&plan); - assert!(result.is_err()); - let msg = result.unwrap_err().to_string(); - assert!( - msg.contains("user_id"), - "error should mention column: {msg}" - ); - assert!(msg.contains("post"), "error should mention table: {msg}"); + let result = find_non_nullable_fk_add_columns(&plan, &[]); + assert_eq!(result.len(), 1); + assert_eq!(result[0].table, "post"); + assert_eq!(result[0].column, "user_id"); + assert_eq!(result[0].reason, RecreateReason::AddColumnWithFk); } #[test] - fn check_nullable_fk_add_column_ok() { + fn find_nullable_fk_add_column_returns_empty() { use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; let plan = MigrationPlan { id: String::new(), @@ -709,12 +886,12 @@ mod tests { }, ], }; - assert!(check_non_nullable_fk_add_columns(&plan).is_ok()); + assert!(find_non_nullable_fk_add_columns(&plan, &[]).is_empty()); } #[test] - fn check_non_nullable_no_fk_passes() { - // Regular non-nullable column without FK should NOT be blocked + fn find_non_nullable_no_fk_returns_empty() { + // Regular non-nullable column without FK should NOT trigger recreation use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; let plan = MigrationPlan { id: String::new(), @@ -737,8 +914,186 @@ mod tests { fill_with: None, }], }; - // Should pass — this column needs fill_with but that's handled separately - assert!(check_non_nullable_fk_add_columns(&plan).is_ok()); + // Should return empty — this column needs fill_with but that's handled separately + assert!(find_non_nullable_fk_add_columns(&plan, &[]).is_empty()); + } + + #[test] + fn find_fk_on_existing_non_nullable_column_detects_recreate() { + // Adding FK constraint to an existing non-nullable column should trigger recreation + use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; + let plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 2, + actions: vec![MigrationAction::AddConstraint { + table: "post".into(), + constraint: TableConstraint::ForeignKey { + name: None, + columns: vec!["user_id".into()], + ref_table: "user".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }, + }], + }; + let models = vec![TableDef { + name: "post".into(), + description: None, + columns: vec![ + ColumnDef { + name: "id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Integer), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }, + ColumnDef { + name: "user_id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Uuid), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }, + ], + constraints: vec![], + }]; + let result = find_non_nullable_fk_add_columns(&plan, &models); + assert_eq!(result.len(), 1); + assert_eq!(result[0].table, "post"); + assert_eq!(result[0].column, "user_id"); + assert_eq!(result[0].reason, RecreateReason::AddFkToExistingColumn); + } + + #[test] + fn find_fk_on_existing_nullable_column_returns_empty() { + // Adding FK constraint to an existing nullable column should NOT trigger recreation + use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; + let plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 2, + actions: vec![MigrationAction::AddConstraint { + table: "post".into(), + constraint: TableConstraint::ForeignKey { + name: None, + columns: vec!["user_id".into()], + ref_table: "user".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }, + }], + }; + let models = vec![TableDef { + name: "post".into(), + description: None, + columns: vec![ColumnDef { + name: "user_id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Uuid), + nullable: true, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }], + constraints: vec![], + }]; + assert!(find_non_nullable_fk_add_columns(&plan, &models).is_empty()); + } + + #[test] + fn rewrite_plan_replaces_actions_with_recreate() { + use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; + let mut plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 2, + actions: vec![ + MigrationAction::AddColumn { + table: "post".into(), + column: Box::new(ColumnDef { + name: "user_id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Uuid), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }), + fill_with: None, + }, + MigrationAction::AddConstraint { + table: "post".into(), + constraint: TableConstraint::ForeignKey { + name: None, + columns: vec!["user_id".into()], + ref_table: "user".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }, + }, + ], + }; + + let recreate = vec![RecreateTableRequired { + table: "post".into(), + column: "user_id".into(), + reason: RecreateReason::AddColumnWithFk, + }]; + + let models = vec![TableDef { + name: "post".into(), + description: None, + columns: vec![ + ColumnDef { + name: "id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Integer), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }, + ColumnDef { + name: "user_id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Uuid), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }, + ], + constraints: vec![], + }]; + + rewrite_plan_for_recreation(&mut plan, &recreate, &models); + + assert_eq!(plan.actions.len(), 2); + assert!(matches!(&plan.actions[0], MigrationAction::DeleteTable { table } if table == "post")); + assert!(matches!(&plan.actions[1], MigrationAction::CreateTable { table, .. } if table == "post")); } #[test] From 05221e84820e5e9cfb2383e3a9b92c554b2d5ea0 Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Fri, 6 Mar 2026 17:24:02 +0900 Subject: [PATCH 2/9] Feat prompt when add fk with required --- .changepacks/changepack_log_dlsNQpHXG2aZaDAah4qh0.json | 1 + 1 file changed, 1 insertion(+) create mode 100644 .changepacks/changepack_log_dlsNQpHXG2aZaDAah4qh0.json diff --git a/.changepacks/changepack_log_dlsNQpHXG2aZaDAah4qh0.json b/.changepacks/changepack_log_dlsNQpHXG2aZaDAah4qh0.json new file mode 100644 index 0000000..fbba7ff --- /dev/null +++ b/.changepacks/changepack_log_dlsNQpHXG2aZaDAah4qh0.json @@ -0,0 +1 @@ +{"changes":{"crates/vespertide-planner/Cargo.toml":"Patch","crates/vespertide-core/Cargo.toml":"Patch","crates/vespertide-naming/Cargo.toml":"Patch","crates/vespertide-query/Cargo.toml":"Patch","crates/vespertide-cli/Cargo.toml":"Patch","crates/vespertide-loader/Cargo.toml":"Patch","crates/vespertide-config/Cargo.toml":"Patch","crates/vespertide-exporter/Cargo.toml":"Patch","crates/vespertide/Cargo.toml":"Patch","crates/vespertide-macro/Cargo.toml":"Patch"},"note":"Add prompt for recreate table","date":"2026-03-06T08:24:00.184098300Z"} \ No newline at end of file From 7d513ef151bac5c7ef54413da2d5614b79172d72 Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Fri, 6 Mar 2026 17:24:45 +0900 Subject: [PATCH 3/9] Feat prompt when add fk with required --- crates/vespertide-cli/src/commands/revision.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/vespertide-cli/src/commands/revision.rs b/crates/vespertide-cli/src/commands/revision.rs index f8659d9..bfd0d99 100644 --- a/crates/vespertide-cli/src/commands/revision.rs +++ b/crates/vespertide-cli/src/commands/revision.rs @@ -417,21 +417,17 @@ fn find_non_nullable_fk_add_columns( if let Some(model) = current_models .iter() .find(|m| m.name.as_str() == table.as_str()) - { - if let Some(col_def) = model + && let Some(col_def) = model .columns .iter() .find(|c| c.name.as_str() == col_name.as_str()) - { - if !col_def.nullable && col_def.default.is_none() { + && !col_def.nullable && col_def.default.is_none() { result.push(RecreateTableRequired { table: table.clone(), column: col_name.clone(), reason: RecreateReason::AddFkToExistingColumn, }); } - } - } } } } @@ -507,7 +503,7 @@ fn rewrite_plan_for_recreation( | MigrationAction::RemoveConstraint { table, .. } => Some(table.as_str()), _ => None, }; - table.map_or(true, |t| !tables_to_recreate.contains(t)) + table.is_none_or(|t| !tables_to_recreate.contains(t)) }); // Add DeleteTable + CreateTable for each recreated table @@ -1092,8 +1088,12 @@ mod tests { rewrite_plan_for_recreation(&mut plan, &recreate, &models); assert_eq!(plan.actions.len(), 2); - assert!(matches!(&plan.actions[0], MigrationAction::DeleteTable { table } if table == "post")); - assert!(matches!(&plan.actions[1], MigrationAction::CreateTable { table, .. } if table == "post")); + assert!( + matches!(&plan.actions[0], MigrationAction::DeleteTable { table } if table == "post") + ); + assert!( + matches!(&plan.actions[1], MigrationAction::CreateTable { table, .. } if table == "post") + ); } #[test] From 2729faa2b1cf74c267883bafd1d115eba8670a27 Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Fri, 6 Mar 2026 19:27:38 +0900 Subject: [PATCH 4/9] Fix fmt --- crates/vespertide-cli/src/commands/revision.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/crates/vespertide-cli/src/commands/revision.rs b/crates/vespertide-cli/src/commands/revision.rs index bfd0d99..44ef429 100644 --- a/crates/vespertide-cli/src/commands/revision.rs +++ b/crates/vespertide-cli/src/commands/revision.rs @@ -421,13 +421,15 @@ fn find_non_nullable_fk_add_columns( .columns .iter() .find(|c| c.name.as_str() == col_name.as_str()) - && !col_def.nullable && col_def.default.is_none() { - result.push(RecreateTableRequired { - table: table.clone(), - column: col_name.clone(), - reason: RecreateReason::AddFkToExistingColumn, - }); - } + && !col_def.nullable + && col_def.default.is_none() + { + result.push(RecreateTableRequired { + table: table.clone(), + column: col_name.clone(), + reason: RecreateReason::AddFkToExistingColumn, + }); + } } } } From b95351a647ed92073af4f9593b5d22df29c5186a Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Fri, 6 Mar 2026 19:44:29 +0900 Subject: [PATCH 5/9] Add testcase --- .../vespertide-cli/src/commands/revision.rs | 229 ++++++++++++++++-- .../src/sql/modify_column_type.rs | 9 +- 2 files changed, 216 insertions(+), 22 deletions(-) diff --git a/crates/vespertide-cli/src/commands/revision.rs b/crates/vespertide-cli/src/commands/revision.rs index 44ef429..436073d 100644 --- a/crates/vespertide-cli/src/commands/revision.rs +++ b/crates/vespertide-cli/src/commands/revision.rs @@ -526,6 +526,42 @@ fn rewrite_plan_for_recreation( } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum RecreateHandling { + NotNeeded, + Rewritten, + PlanEmptied, +} + +fn handle_recreate_requirements( + plan: &mut MigrationPlan, + current_models: &[TableDef], + prompt_fn: F, +) -> Result +where + F: Fn(&[RecreateTableRequired]) -> Result, +{ + let recreate_tables = find_non_nullable_fk_add_columns(plan, current_models); + if recreate_tables.is_empty() { + return Ok(RecreateHandling::NotNeeded); + } + + if !prompt_fn(&recreate_tables)? { + anyhow::bail!( + "Migration cancelled. To proceed without recreation, make the column nullable \ + or add it with a default value that references an existing row." + ); + } + + rewrite_plan_for_recreation(plan, &recreate_tables, current_models); + + if plan.actions.is_empty() { + return Ok(RecreateHandling::PlanEmptied); + } + + Ok(RecreateHandling::Rewritten) +} + pub async fn cmd_revision(message: String, fill_with_args: Vec) -> Result<()> { let config = load_config()?; let current_models = load_models(&config)?; @@ -543,20 +579,10 @@ pub async fn cmd_revision(message: String, fill_with_args: Vec) -> Resul return Ok(()); } - // Check for non-nullable FK columns being added to existing tables. - // These require table recreation because existing rows can't satisfy the FK constraint. - let recreate_tables = find_non_nullable_fk_add_columns(&plan, ¤t_models); - if !recreate_tables.is_empty() { - if !prompt_recreate_tables(&recreate_tables)? { - anyhow::bail!( - "Migration cancelled. To proceed without recreation, make the column nullable \ - or add it with a default value that references an existing row." - ); - } - rewrite_plan_for_recreation(&mut plan, &recreate_tables, ¤t_models); - - // Re-check: if plan is now empty after recreation rewrite, nothing to do - if plan.actions.is_empty() { + // Check for non-nullable FK changes that require table recreation. + match handle_recreate_requirements(&mut plan, ¤t_models, prompt_recreate_tables)? { + RecreateHandling::NotNeeded | RecreateHandling::Rewritten => {} + RecreateHandling::PlanEmptied => { println!( "{} {}", "No changes detected.".bright_yellow(), @@ -1098,6 +1124,181 @@ mod tests { ); } + #[test] + fn rewrite_plan_keeps_non_table_actions() { + use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; + + let mut plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 2, + actions: vec![ + MigrationAction::RawSql { + sql: "select 1".into(), + }, + MigrationAction::AddColumn { + table: "post".into(), + column: Box::new(ColumnDef { + name: "user_id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Uuid), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }), + fill_with: None, + }, + ], + }; + + let recreate = vec![RecreateTableRequired { + table: "post".into(), + column: "user_id".into(), + reason: RecreateReason::AddColumnWithFk, + }]; + + let models = vec![TableDef { + name: "post".into(), + description: None, + columns: vec![ColumnDef { + name: "user_id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Uuid), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }], + constraints: vec![], + }]; + + rewrite_plan_for_recreation(&mut plan, &recreate, &models); + + assert!(matches!(&plan.actions[0], MigrationAction::RawSql { sql } if sql == "select 1")); + assert!( + matches!(&plan.actions[1], MigrationAction::DeleteTable { table } if table == "post") + ); + assert!( + matches!(&plan.actions[2], MigrationAction::CreateTable { table, .. } if table == "post") + ); + } + + #[test] + fn handle_recreate_requirements_returns_not_needed() { + let mut plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 1, + actions: vec![MigrationAction::RawSql { + sql: "select 1".into(), + }], + }; + + let result = handle_recreate_requirements(&mut plan, &[], |_| Ok(true)).unwrap(); + + assert_eq!(result, RecreateHandling::NotNeeded); + assert_eq!(plan.actions.len(), 1); + } + + #[test] + fn handle_recreate_requirements_bails_when_prompt_rejected() { + use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; + + let mut plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 1, + actions: vec![ + MigrationAction::AddColumn { + table: "post".into(), + column: Box::new(ColumnDef { + name: "user_id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Uuid), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }), + fill_with: None, + }, + MigrationAction::AddConstraint { + table: "post".into(), + constraint: TableConstraint::ForeignKey { + name: None, + columns: vec!["user_id".into()], + ref_table: "user".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }, + }, + ], + }; + + let err = handle_recreate_requirements(&mut plan, &[], |_| Ok(false)).unwrap_err(); + + assert!( + err.to_string() + .contains("Migration cancelled. To proceed without recreation") + ); + } + + #[test] + fn handle_recreate_requirements_returns_plan_emptied_when_model_missing() { + use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; + + let mut plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 1, + actions: vec![ + MigrationAction::AddColumn { + table: "post".into(), + column: Box::new(ColumnDef { + name: "user_id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Uuid), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }), + fill_with: None, + }, + MigrationAction::AddConstraint { + table: "post".into(), + constraint: TableConstraint::ForeignKey { + name: None, + columns: vec!["user_id".into()], + ref_table: "user".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }, + }, + ], + }; + + let result = handle_recreate_requirements(&mut plan, &[], |_| Ok(true)).unwrap(); + + assert_eq!(result, RecreateHandling::PlanEmptied); + assert!(plan.actions.is_empty()); + } + #[test] fn test_parse_fill_with_args() { let args = vec![ diff --git a/crates/vespertide-query/src/sql/modify_column_type.rs b/crates/vespertide-query/src/sql/modify_column_type.rs index ebe102c..c0601c5 100644 --- a/crates/vespertide-query/src/sql/modify_column_type.rs +++ b/crates/vespertide-query/src/sql/modify_column_type.rs @@ -193,14 +193,7 @@ pub fn build_modify_column_type( } // 3. ALTER TABLE ... ALTER COLUMN ... TYPE target_type USING col::text::target_type - queries.push(BuiltQuery::Raw(super::types::RawSql::per_backend( - format!( - "ALTER TABLE \"{}\" ALTER COLUMN \"{}\" TYPE \"{}\" USING \"{}\"::text::\"{}\"", - table, column, target_type_name, column, target_type_name - ), - String::new(), - String::new(), - ))); + queries.push(BuiltQuery::Raw(super::types::RawSql::per_backend(format!("ALTER TABLE \"{}\" ALTER COLUMN \"{}\" TYPE \"{}\" USING \"{}\"::text::\"{}\"", table, column, target_type_name, column, target_type_name), String::new(), String::new()))); // 4. DROP old enum type queries.push(BuiltQuery::Raw(super::types::RawSql::per_backend( From c118803fe4c931a83ffaf43980f7086cc6621375 Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Fri, 6 Mar 2026 19:55:46 +0900 Subject: [PATCH 6/9] Add testcase --- .../vespertide-cli/src/commands/revision.rs | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/crates/vespertide-cli/src/commands/revision.rs b/crates/vespertide-cli/src/commands/revision.rs index 436073d..60fca51 100644 --- a/crates/vespertide-cli/src/commands/revision.rs +++ b/crates/vespertide-cli/src/commands/revision.rs @@ -873,6 +873,45 @@ mod tests { assert_eq!(result[0].reason, RecreateReason::AddColumnWithFk); } + #[test] + fn find_non_nullable_inline_fk_add_column_detects_recreate() { + use vespertide_core::{ColumnDef, ColumnType, ReferenceAction, SimpleColumnType}; + use vespertide_core::schema::foreign_key::{ForeignKeyDef, ForeignKeySyntax}; + + let plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 2, + actions: vec![MigrationAction::AddColumn { + table: "post".into(), + column: Box::new(ColumnDef { + name: "user_id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Uuid), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: Some(ForeignKeySyntax::Object(ForeignKeyDef { + ref_table: "user".into(), + ref_columns: vec!["id".into()], + on_delete: Some(ReferenceAction::Cascade), + on_update: None, + })), + }), + fill_with: None, + }], + }; + + let result = find_non_nullable_fk_add_columns(&plan, &[]); + assert_eq!(result.len(), 1); + assert_eq!(result[0].table, "post"); + assert_eq!(result[0].column, "user_id"); + assert_eq!(result[0].reason, RecreateReason::AddColumnWithFk); + } + #[test] fn find_nullable_fk_add_column_returns_empty() { use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; @@ -1039,6 +1078,88 @@ mod tests { assert!(find_non_nullable_fk_add_columns(&plan, &models).is_empty()); } + #[test] + fn find_fk_on_existing_column_with_default_returns_empty() { + use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; + + let plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 2, + actions: vec![MigrationAction::AddConstraint { + table: "post".into(), + constraint: TableConstraint::ForeignKey { + name: None, + columns: vec!["user_id".into()], + ref_table: "user".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }, + }], + }; + let models = vec![TableDef { + name: "post".into(), + description: None, + columns: vec![ColumnDef { + name: "user_id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Uuid), + nullable: false, + default: Some(true.into()), + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }], + constraints: vec![], + }]; + + assert!(find_non_nullable_fk_add_columns(&plan, &models).is_empty()); + } + + #[test] + fn find_fk_on_existing_column_missing_from_model_returns_empty() { + use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; + + let plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 2, + actions: vec![MigrationAction::AddConstraint { + table: "post".into(), + constraint: TableConstraint::ForeignKey { + name: None, + columns: vec!["user_id".into()], + ref_table: "user".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }, + }], + }; + let models = vec![TableDef { + name: "post".into(), + description: None, + columns: vec![ColumnDef { + name: "other_id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Uuid), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }], + constraints: vec![], + }]; + + assert!(find_non_nullable_fk_add_columns(&plan, &models).is_empty()); + } + #[test] fn rewrite_plan_replaces_actions_with_recreate() { use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; From 84b46fccd84fb7ca935126874d6798932f7ac47a Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Fri, 6 Mar 2026 19:55:56 +0900 Subject: [PATCH 7/9] Add testcase --- crates/vespertide-cli/src/commands/revision.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/vespertide-cli/src/commands/revision.rs b/crates/vespertide-cli/src/commands/revision.rs index 60fca51..b1a54af 100644 --- a/crates/vespertide-cli/src/commands/revision.rs +++ b/crates/vespertide-cli/src/commands/revision.rs @@ -875,8 +875,8 @@ mod tests { #[test] fn find_non_nullable_inline_fk_add_column_detects_recreate() { - use vespertide_core::{ColumnDef, ColumnType, ReferenceAction, SimpleColumnType}; use vespertide_core::schema::foreign_key::{ForeignKeyDef, ForeignKeySyntax}; + use vespertide_core::{ColumnDef, ColumnType, ReferenceAction, SimpleColumnType}; let plan = MigrationPlan { id: String::new(), From 187862ddc4f9e67c12b98210b2aa7b71eca66bea Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Fri, 6 Mar 2026 20:07:19 +0900 Subject: [PATCH 8/9] Add testcase --- .../vespertide-cli/src/commands/revision.rs | 130 ++++++++++++++---- 1 file changed, 106 insertions(+), 24 deletions(-) diff --git a/crates/vespertide-cli/src/commands/revision.rs b/crates/vespertide-cli/src/commands/revision.rs index b1a54af..95245cf 100644 --- a/crates/vespertide-cli/src/commands/revision.rs +++ b/crates/vespertide-cli/src/commands/revision.rs @@ -526,40 +526,46 @@ fn rewrite_plan_for_recreation( } } +/// Whether the caller should continue processing the plan or return early. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum RecreateHandling { - NotNeeded, - Rewritten, - PlanEmptied, +enum RecreateOutcome { + /// No recreation was needed, or recreation succeeded. Continue processing. + Continue, + /// Plan is empty after recreation rewrite. Caller should return early. + Done, } fn handle_recreate_requirements( plan: &mut MigrationPlan, current_models: &[TableDef], prompt_fn: F, -) -> Result +) -> Result where F: Fn(&[RecreateTableRequired]) -> Result, { let recreate_tables = find_non_nullable_fk_add_columns(plan, current_models); if recreate_tables.is_empty() { - return Ok(RecreateHandling::NotNeeded); + return Ok(RecreateOutcome::Continue); } if !prompt_fn(&recreate_tables)? { anyhow::bail!( - "Migration cancelled. To proceed without recreation, make the column nullable \ - or add it with a default value that references an existing row." + "Migration cancelled. To proceed without recreation, make the column nullable or add it with a default value that references an existing row." ); } rewrite_plan_for_recreation(plan, &recreate_tables, current_models); if plan.actions.is_empty() { - return Ok(RecreateHandling::PlanEmptied); + println!( + "{} {}", + "No changes detected.".bright_yellow(), + "Nothing to migrate.".bright_white() + ); + return Ok(RecreateOutcome::Done); } - Ok(RecreateHandling::Rewritten) + Ok(RecreateOutcome::Continue) } pub async fn cmd_revision(message: String, fill_with_args: Vec) -> Result<()> { @@ -580,16 +586,11 @@ pub async fn cmd_revision(message: String, fill_with_args: Vec) -> Resul } // Check for non-nullable FK changes that require table recreation. - match handle_recreate_requirements(&mut plan, ¤t_models, prompt_recreate_tables)? { - RecreateHandling::NotNeeded | RecreateHandling::Rewritten => {} - RecreateHandling::PlanEmptied => { - println!( - "{} {}", - "No changes detected.".bright_yellow(), - "Nothing to migrate.".bright_white() - ); - return Ok(()); - } + if matches!( + handle_recreate_requirements(&mut plan, ¤t_models, prompt_recreate_tables)?, + RecreateOutcome::Done + ) { + return Ok(()); } // Reconstruct baseline schema for column type lookups @@ -1311,7 +1312,7 @@ mod tests { } #[test] - fn handle_recreate_requirements_returns_not_needed() { + fn handle_recreate_requirements_returns_continue_when_no_fk() { let mut plan = MigrationPlan { id: String::new(), comment: None, @@ -1324,7 +1325,7 @@ mod tests { let result = handle_recreate_requirements(&mut plan, &[], |_| Ok(true)).unwrap(); - assert_eq!(result, RecreateHandling::NotNeeded); + assert_eq!(result, RecreateOutcome::Continue); assert_eq!(plan.actions.len(), 1); } @@ -1376,7 +1377,7 @@ mod tests { } #[test] - fn handle_recreate_requirements_returns_plan_emptied_when_model_missing() { + fn handle_recreate_requirements_returns_done_when_model_missing() { use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; let mut plan = MigrationPlan { @@ -1416,10 +1417,91 @@ mod tests { let result = handle_recreate_requirements(&mut plan, &[], |_| Ok(true)).unwrap(); - assert_eq!(result, RecreateHandling::PlanEmptied); + assert_eq!(result, RecreateOutcome::Done); assert!(plan.actions.is_empty()); } + #[test] + fn handle_recreate_requirements_returns_continue_after_rewrite() { + use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; + + let mut plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 1, + actions: vec![ + MigrationAction::AddColumn { + table: "post".into(), + column: Box::new(ColumnDef { + name: "user_id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Uuid), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }), + fill_with: None, + }, + MigrationAction::AddConstraint { + table: "post".into(), + constraint: TableConstraint::ForeignKey { + name: None, + columns: vec!["user_id".into()], + ref_table: "user".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }, + }, + ], + }; + + let models = vec![TableDef { + name: "post".into(), + description: None, + columns: vec![ + ColumnDef { + name: "id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Integer), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }, + ColumnDef { + name: "user_id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Uuid), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }, + ], + constraints: vec![], + }]; + + let result = handle_recreate_requirements(&mut plan, &models, |_| Ok(true)).unwrap(); + + assert_eq!(result, RecreateOutcome::Continue); + assert_eq!(plan.actions.len(), 2); + assert!( + matches!(&plan.actions[0], MigrationAction::DeleteTable { table } if table == "post") + ); + assert!( + matches!(&plan.actions[1], MigrationAction::CreateTable { table, .. } if table == "post") + ); + } + #[test] fn test_parse_fill_with_args() { let args = vec![ From 7b4d91fe51585f3e486744a0e153043836468eec Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Fri, 6 Mar 2026 20:18:48 +0900 Subject: [PATCH 9/9] Fix case --- .../vespertide-cli/src/commands/revision.rs | 51 +++++-------------- 1 file changed, 12 insertions(+), 39 deletions(-) diff --git a/crates/vespertide-cli/src/commands/revision.rs b/crates/vespertide-cli/src/commands/revision.rs index 95245cf..33be2d4 100644 --- a/crates/vespertide-cli/src/commands/revision.rs +++ b/crates/vespertide-cli/src/commands/revision.rs @@ -526,26 +526,17 @@ fn rewrite_plan_for_recreation( } } -/// Whether the caller should continue processing the plan or return early. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum RecreateOutcome { - /// No recreation was needed, or recreation succeeded. Continue processing. - Continue, - /// Plan is empty after recreation rewrite. Caller should return early. - Done, -} - fn handle_recreate_requirements( plan: &mut MigrationPlan, current_models: &[TableDef], prompt_fn: F, -) -> Result +) -> Result<()> where F: Fn(&[RecreateTableRequired]) -> Result, { let recreate_tables = find_non_nullable_fk_add_columns(plan, current_models); if recreate_tables.is_empty() { - return Ok(RecreateOutcome::Continue); + return Ok(()); } if !prompt_fn(&recreate_tables)? { @@ -555,17 +546,7 @@ where } rewrite_plan_for_recreation(plan, &recreate_tables, current_models); - - if plan.actions.is_empty() { - println!( - "{} {}", - "No changes detected.".bright_yellow(), - "Nothing to migrate.".bright_white() - ); - return Ok(RecreateOutcome::Done); - } - - Ok(RecreateOutcome::Continue) + Ok(()) } pub async fn cmd_revision(message: String, fill_with_args: Vec) -> Result<()> { @@ -576,6 +557,9 @@ pub async fn cmd_revision(message: String, fill_with_args: Vec) -> Resul let mut plan = plan_next_migration(¤t_models, &applied_plans) .map_err(|e| anyhow::anyhow!("planning error: {}", e))?; + // Check for non-nullable FK changes that require table recreation. + handle_recreate_requirements(&mut plan, ¤t_models, prompt_recreate_tables)?; + if plan.actions.is_empty() { println!( "{} {}", @@ -585,14 +569,6 @@ pub async fn cmd_revision(message: String, fill_with_args: Vec) -> Resul return Ok(()); } - // Check for non-nullable FK changes that require table recreation. - if matches!( - handle_recreate_requirements(&mut plan, ¤t_models, prompt_recreate_tables)?, - RecreateOutcome::Done - ) { - return Ok(()); - } - // Reconstruct baseline schema for column type lookups let baseline_schema = schema_from_plans(&applied_plans) .map_err(|e| anyhow::anyhow!("schema reconstruction error: {}", e))?; @@ -1312,7 +1288,7 @@ mod tests { } #[test] - fn handle_recreate_requirements_returns_continue_when_no_fk() { + fn handle_recreate_requirements_returns_ok_when_no_fk() { let mut plan = MigrationPlan { id: String::new(), comment: None, @@ -1323,9 +1299,8 @@ mod tests { }], }; - let result = handle_recreate_requirements(&mut plan, &[], |_| Ok(true)).unwrap(); + handle_recreate_requirements(&mut plan, &[], |_| Ok(true)).unwrap(); - assert_eq!(result, RecreateOutcome::Continue); assert_eq!(plan.actions.len(), 1); } @@ -1377,7 +1352,7 @@ mod tests { } #[test] - fn handle_recreate_requirements_returns_done_when_model_missing() { + fn handle_recreate_requirements_empties_plan_when_model_missing() { use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; let mut plan = MigrationPlan { @@ -1415,14 +1390,13 @@ mod tests { ], }; - let result = handle_recreate_requirements(&mut plan, &[], |_| Ok(true)).unwrap(); + handle_recreate_requirements(&mut plan, &[], |_| Ok(true)).unwrap(); - assert_eq!(result, RecreateOutcome::Done); assert!(plan.actions.is_empty()); } #[test] - fn handle_recreate_requirements_returns_continue_after_rewrite() { + fn handle_recreate_requirements_rewrites_plan_when_model_exists() { use vespertide_core::{ColumnDef, ColumnType, SimpleColumnType}; let mut plan = MigrationPlan { @@ -1490,9 +1464,8 @@ mod tests { constraints: vec![], }]; - let result = handle_recreate_requirements(&mut plan, &models, |_| Ok(true)).unwrap(); + handle_recreate_requirements(&mut plan, &models, |_| Ok(true)).unwrap(); - assert_eq!(result, RecreateOutcome::Continue); assert_eq!(plan.actions.len(), 2); assert!( matches!(&plan.actions[0], MigrationAction::DeleteTable { table } if table == "post")