Skip to content

Commit 4920b18

Browse files
owjs3901owjs3901
committed
Update Removed value
1 parent b408036 commit 4920b18

6 files changed

Lines changed: 239 additions & 13 deletions

‎crates/vespertide-cli/src/commands/revision.rs‎

Lines changed: 61 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,14 @@ pub async fn cmd_revision(message: String, fill_with_args: Vec<String>) -> Resul
391391
)?;
392392

393393
// Handle any missing enum fill_with values (for removed enum values) interactively
394-
handle_missing_enum_fill_with(&mut plan, &baseline_schema, prompt_enum_value)?;
394+
handle_missing_enum_fill_with(&mut plan, &baseline_schema, |prompt, values| {
395+
let selected = prompt_enum_value(prompt, values)?;
396+
// Strip SQL quotes — BTreeMap stores bare enum names, SQL layer handles quoting
397+
Ok(selected
398+
.trim_start_matches('\'')
399+
.trim_end_matches('\'')
400+
.to_string())
401+
})?;
395402

396403
plan.id = uuid::Uuid::new_v4().to_string();
397404
plan.comment = Some(message);
@@ -1554,7 +1561,7 @@ mod tests {
15541561

15551562
// Mock prompt: always select first remaining value
15561563
let mock_enum =
1557-
|_prompt: &str, values: &[String]| -> Result<String> { Ok(format!("'{}'", values[0])) };
1564+
|_prompt: &str, values: &[String]| -> Result<String> { Ok(values[0].to_string()) };
15581565

15591566
let result = collect_enum_fill_with_values(&missing, mock_enum);
15601567
assert!(result.is_ok());
@@ -1563,7 +1570,7 @@ mod tests {
15631570
assert_eq!(collected[0].0, 0); // action_index
15641571
assert_eq!(
15651572
collected[0].1.get("cancelled"),
1566-
Some(&"'pending'".to_string())
1573+
Some(&"pending".to_string())
15671574
);
15681575
}
15691576

@@ -1581,17 +1588,17 @@ mod tests {
15811588

15821589
// Mock prompt: always select second remaining value
15831590
let mock_enum =
1584-
|_prompt: &str, values: &[String]| -> Result<String> { Ok(format!("'{}'", values[1])) };
1591+
|_prompt: &str, values: &[String]| -> Result<String> { Ok(values[1].to_string()) };
15851592

15861593
let result = collect_enum_fill_with_values(&missing, mock_enum);
15871594
assert!(result.is_ok());
15881595
let collected = result.unwrap();
15891596
assert_eq!(collected[0].1.len(), 2);
15901597
assert_eq!(
15911598
collected[0].1.get("cancelled"),
1592-
Some(&"'shipped'".to_string())
1599+
Some(&"shipped".to_string())
15931600
);
1594-
assert_eq!(collected[0].1.get("draft"), Some(&"'shipped'".to_string()));
1601+
assert_eq!(collected[0].1.get("draft"), Some(&"shipped".to_string()));
15951602
}
15961603

15971604
#[test]
@@ -1615,14 +1622,14 @@ mod tests {
16151622
};
16161623

16171624
let mut mappings = BTreeMap::new();
1618-
mappings.insert("cancelled".to_string(), "'pending'".to_string());
1625+
mappings.insert("cancelled".to_string(), "pending".to_string());
16191626
let collected = vec![(0usize, mappings)];
16201627

16211628
apply_enum_fill_with_to_plan(&mut plan, &collected);
16221629

16231630
if let MigrationAction::ModifyColumnType { fill_with, .. } = &plan.actions[0] {
16241631
let fw = fill_with.as_ref().expect("fill_with should be set");
1625-
assert_eq!(fw.get("cancelled"), Some(&"'pending'".to_string()));
1632+
assert_eq!(fw.get("cancelled"), Some(&"pending".to_string()));
16261633
} else {
16271634
panic!("Expected ModifyColumnType");
16281635
}
@@ -1674,14 +1681,14 @@ mod tests {
16741681

16751682
// Mock: always select first remaining value
16761683
let mock_enum =
1677-
|_prompt: &str, values: &[String]| -> Result<String> { Ok(format!("'{}'", values[0])) };
1684+
|_prompt: &str, values: &[String]| -> Result<String> { Ok(values[0].to_string()) };
16781685

16791686
let result = handle_missing_enum_fill_with(&mut plan, &baseline, mock_enum);
16801687
assert!(result.is_ok());
16811688

16821689
if let MigrationAction::ModifyColumnType { fill_with, .. } = &plan.actions[0] {
16831690
let fw = fill_with.as_ref().expect("fill_with should be populated");
1684-
assert_eq!(fw.get("cancelled"), Some(&"'pending'".to_string()));
1691+
assert_eq!(fw.get("cancelled"), Some(&"pending".to_string()));
16851692
} else {
16861693
panic!("Expected ModifyColumnType");
16871694
}
@@ -1704,4 +1711,48 @@ mod tests {
17041711
let result = handle_missing_enum_fill_with(&mut plan, &[], mock_enum);
17051712
assert!(result.is_ok());
17061713
}
1714+
1715+
#[test]
1716+
fn test_apply_enum_fill_with_to_plan_extends_existing() {
1717+
use vespertide_core::{ColumnType, ComplexColumnType, EnumValues};
1718+
1719+
// Start with a fill_with that already has one entry
1720+
let mut existing_fw = BTreeMap::new();
1721+
existing_fw.insert("draft".to_string(), "pending".to_string());
1722+
1723+
let mut plan = MigrationPlan {
1724+
id: String::new(),
1725+
comment: None,
1726+
created_at: None,
1727+
version: 2,
1728+
actions: vec![MigrationAction::ModifyColumnType {
1729+
table: "orders".into(),
1730+
column: "status".into(),
1731+
new_type: ColumnType::Complex(ComplexColumnType::Enum {
1732+
name: "order_status".into(),
1733+
values: EnumValues::String(vec!["pending".into(), "shipped".into()]),
1734+
}),
1735+
fill_with: Some(existing_fw),
1736+
}],
1737+
};
1738+
1739+
// Collect additional mappings
1740+
let mut new_mappings = BTreeMap::new();
1741+
new_mappings.insert("cancelled".to_string(), "shipped".to_string());
1742+
let collected = vec![(0usize, new_mappings)];
1743+
1744+
apply_enum_fill_with_to_plan(&mut plan, &collected);
1745+
1746+
if let MigrationAction::ModifyColumnType { fill_with, .. } = &plan.actions[0] {
1747+
let fw = fill_with.as_ref().expect("fill_with should be set");
1748+
// Original entry preserved
1749+
assert_eq!(fw.get("draft"), Some(&"pending".to_string()));
1750+
// New entry added
1751+
assert_eq!(fw.get("cancelled"), Some(&"shipped".to_string()));
1752+
// Total 2 entries
1753+
assert_eq!(fw.len(), 2);
1754+
} else {
1755+
panic!("Expected ModifyColumnType");
1756+
}
1757+
}
17071758
}

‎crates/vespertide-planner/src/validate.rs‎

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2254,7 +2254,7 @@ mod tests {
22542254
#[test]
22552255
fn find_missing_enum_fill_with_skips_already_covered() {
22562256
let mut fw = std::collections::BTreeMap::new();
2257-
fw.insert("cancelled".to_string(), "'pending'".to_string());
2257+
fw.insert("cancelled".to_string(), "pending".to_string());
22582258

22592259
let plan = MigrationPlan {
22602260
id: String::new(),
@@ -2287,7 +2287,7 @@ mod tests {
22872287
#[test]
22882288
fn find_missing_enum_fill_with_reports_partially_covered() {
22892289
let mut fw = std::collections::BTreeMap::new();
2290-
fw.insert("cancelled".to_string(), "'pending'".to_string());
2290+
fw.insert("cancelled".to_string(), "pending".to_string());
22912291

22922292
let plan = MigrationPlan {
22932293
id: String::new(),
@@ -2389,4 +2389,64 @@ mod tests {
23892389
"Non-enum type changes should not trigger fill_with"
23902390
);
23912391
}
2392+
2393+
#[test]
2394+
fn validate_modify_column_type_fill_with_invalid_replacement() {
2395+
let mut fw = std::collections::BTreeMap::new();
2396+
fw.insert("cancelled".to_string(), "nonexistent_value".to_string());
2397+
2398+
let plan = MigrationPlan {
2399+
id: String::new(),
2400+
comment: None,
2401+
created_at: None,
2402+
version: 2,
2403+
actions: vec![MigrationAction::ModifyColumnType {
2404+
table: "orders".into(),
2405+
column: "status".into(),
2406+
new_type: ColumnType::Complex(ComplexColumnType::Enum {
2407+
name: "order_status".into(),
2408+
values: EnumValues::String(vec!["pending".into(), "shipped".into()]),
2409+
}),
2410+
fill_with: Some(fw),
2411+
}],
2412+
};
2413+
2414+
let result = validate_migration_plan(&plan);
2415+
assert!(result.is_err());
2416+
match result.unwrap_err() {
2417+
PlannerError::InvalidEnumDefault(err) => {
2418+
assert_eq!(err.enum_name, "order_status");
2419+
assert_eq!(err.table_name, "orders");
2420+
assert_eq!(err.column_name, "status");
2421+
assert_eq!(err.value_type, "fill_with");
2422+
assert_eq!(err.value, "nonexistent_value");
2423+
}
2424+
err => panic!("expected InvalidEnumDefault error, got {:?}", err),
2425+
}
2426+
}
2427+
2428+
#[test]
2429+
fn validate_modify_column_type_fill_with_valid_replacement() {
2430+
let mut fw = std::collections::BTreeMap::new();
2431+
fw.insert("cancelled".to_string(), "pending".to_string());
2432+
2433+
let plan = MigrationPlan {
2434+
id: String::new(),
2435+
comment: None,
2436+
created_at: None,
2437+
version: 2,
2438+
actions: vec![MigrationAction::ModifyColumnType {
2439+
table: "orders".into(),
2440+
column: "status".into(),
2441+
new_type: ColumnType::Complex(ComplexColumnType::Enum {
2442+
name: "order_status".into(),
2443+
values: EnumValues::String(vec!["pending".into(), "shipped".into()]),
2444+
}),
2445+
fill_with: Some(fw),
2446+
}],
2447+
};
2448+
2449+
let result = validate_migration_plan(&plan);
2450+
assert!(result.is_ok());
2451+
}
23922452
}

‎crates/vespertide-query/src/sql/modify_column_type.rs‎

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ fn build_fill_with_updates(
2424
.map(|(removed_value, replacement)| {
2525
let update_stmt = Query::update()
2626
.table(Alias::new(table))
27-
.value(Alias::new(column), Expr::cust(replacement))
27+
.value(Alias::new(column), Expr::val(replacement.as_str()))
2828
.and_where(Expr::col(Alias::new(column)).eq(removed_value.as_str()))
2929
.to_owned();
3030
BuiltQuery::Update(Box::new(update_stmt))
@@ -942,4 +942,95 @@ mod tests {
942942
assert!(sql.contains("status_type"));
943943
assert!(sql.contains("ALTER TABLE"));
944944
}
945+
946+
#[rstest]
947+
#[case::fill_with_enum_change_postgres(
948+
"fill_with_enum_change_postgres",
949+
DatabaseBackend::Postgres,
950+
ColumnType::Complex(ComplexColumnType::Enum {
951+
name: "status".into(),
952+
values: EnumValues::String(vec!["active".into(), "inactive".into(), "banned".into()]),
953+
}),
954+
ColumnType::Complex(ComplexColumnType::Enum {
955+
name: "status".into(),
956+
values: EnumValues::String(vec!["active".into(), "inactive".into()]),
957+
})
958+
)]
959+
#[case::fill_with_enum_change_sqlite(
960+
"fill_with_enum_change_sqlite",
961+
DatabaseBackend::Sqlite,
962+
ColumnType::Complex(ComplexColumnType::Enum {
963+
name: "status".into(),
964+
values: EnumValues::String(vec!["active".into(), "inactive".into(), "banned".into()]),
965+
}),
966+
ColumnType::Complex(ComplexColumnType::Enum {
967+
name: "status".into(),
968+
values: EnumValues::String(vec!["active".into(), "inactive".into()]),
969+
})
970+
)]
971+
#[case::fill_with_enum_change_mysql(
972+
"fill_with_enum_change_mysql",
973+
DatabaseBackend::MySql,
974+
ColumnType::Complex(ComplexColumnType::Enum {
975+
name: "status".into(),
976+
values: EnumValues::String(vec!["active".into(), "inactive".into(), "banned".into()]),
977+
}),
978+
ColumnType::Complex(ComplexColumnType::Enum {
979+
name: "status".into(),
980+
values: EnumValues::String(vec!["active".into(), "inactive".into()]),
981+
})
982+
)]
983+
fn test_modify_column_type_with_fill_with(
984+
#[case] title: &str,
985+
#[case] backend: DatabaseBackend,
986+
#[case] old_type: ColumnType,
987+
#[case] new_type: ColumnType,
988+
) {
989+
let mut fill_with_map = std::collections::BTreeMap::new();
990+
fill_with_map.insert("banned".to_string(), "inactive".to_string());
991+
992+
let current_schema = vec![TableDef {
993+
name: "users".into(),
994+
description: None,
995+
columns: vec![ColumnDef {
996+
name: "status".into(),
997+
r#type: old_type,
998+
nullable: true,
999+
default: None,
1000+
comment: None,
1001+
primary_key: None,
1002+
unique: None,
1003+
index: None,
1004+
foreign_key: None,
1005+
}],
1006+
constraints: vec![],
1007+
}];
1008+
1009+
let result = build_modify_column_type(
1010+
&backend,
1011+
"users",
1012+
"status",
1013+
&new_type,
1014+
Some(&fill_with_map),
1015+
&current_schema,
1016+
)
1017+
.unwrap();
1018+
1019+
let sql = result
1020+
.iter()
1021+
.map(|q| q.build(backend))
1022+
.collect::<Vec<_>>()
1023+
.join(";\n");
1024+
1025+
// All backends should include the UPDATE statement for fill_with
1026+
assert!(
1027+
sql.contains("UPDATE"),
1028+
"Expected UPDATE for fill_with mapping, got: {}",
1029+
sql
1030+
);
1031+
1032+
with_settings!({ snapshot_suffix => format!("modify_column_type_with_fill_with_{}", title) }, {
1033+
assert_snapshot!(sql);
1034+
});
1035+
}
9451036
}

‎crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_type__tests__modify_column_type_with_fill_with@modify_column_type_with_fill_with_fill_with_enum_change_mysql.snap‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
source: crates/vespertide-query/src/sql/modify_column_type.rs
3+
expression: sql
4+
---
5+
UPDATE `users` SET `status` = 'inactive' WHERE `status` = 'banned';
6+
ALTER TABLE `users` MODIFY COLUMN `status` ENUM('active', 'inactive')

‎crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_type__tests__modify_column_type_with_fill_with@modify_column_type_with_fill_with_fill_with_enum_change_postgres.snap‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
source: crates/vespertide-query/src/sql/modify_column_type.rs
3+
expression: sql
4+
---
5+
UPDATE "users" SET "status" = 'inactive' WHERE "status" = 'banned';
6+
CREATE TYPE "users_status_new" AS ENUM ('active', 'inactive');
7+
ALTER TABLE "users" ALTER COLUMN "status" TYPE "users_status_new" USING "status"::text::"users_status_new";
8+
DROP TYPE "users_status";
9+
ALTER TYPE "users_status_new" RENAME TO "users_status"

‎crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_type__tests__modify_column_type_with_fill_with@modify_column_type_with_fill_with_fill_with_enum_change_sqlite.snap‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
source: crates/vespertide-query/src/sql/modify_column_type.rs
3+
expression: sql
4+
---
5+
UPDATE "users" SET "status" = 'inactive' WHERE "status" = 'banned';
6+
CREATE TABLE "users_temp" ( "status" enum_text , CONSTRAINT "chk_users__status" CHECK ("status" IN ('active', 'inactive')));
7+
INSERT INTO "users_temp" ("status") SELECT "status" FROM "users";
8+
DROP TABLE "users";
9+
ALTER TABLE "users_temp" RENAME TO "users"

0 commit comments

Comments
 (0)