diff --git a/crates/dbx-core/src/schema_diff.rs b/crates/dbx-core/src/schema_diff.rs index c711f3cee..3ec0c0c14 100644 --- a/crates/dbx-core/src/schema_diff.rs +++ b/crates/dbx-core/src/schema_diff.rs @@ -366,8 +366,11 @@ fn diff_schema(options: &SchemaDiffPreparationOptions) -> Vec { let column_diffs = diff_columns_with_options( &source.columns, &target.columns, - options.ignore_comments, - options.compare_column_order, + ColumnDiffOptions { + ignore_comments: options.ignore_comments, + compare_column_order: options.compare_column_order, + case_insensitive_names: column_names_are_case_insensitive(options.database_type), + }, ); let index_diffs = diff_indexes(&source.indexes, &target.indexes); let foreign_key_diffs = diff_foreign_keys(&source.foreign_keys, &target.foreign_keys); @@ -415,27 +418,50 @@ fn diff_names(source: &[String], target: &[String]) -> (Vec, Vec } pub fn diff_columns(source: &[ColumnInfo], target: &[ColumnInfo]) -> Vec { - diff_columns_with_options(source, target, false, false) + diff_columns_with_options(source, target, ColumnDiffOptions::default()) +} + +#[derive(Debug, Clone, Copy, Default)] +struct ColumnDiffOptions { + ignore_comments: bool, + compare_column_order: bool, + case_insensitive_names: bool, } fn diff_columns_with_options( source: &[ColumnInfo], target: &[ColumnInfo], - ignore_comments: bool, - compare_column_order: bool, + options: ColumnDiffOptions, ) -> Vec { let mut diffs = Vec::new(); - let target_map: HashMap<&str, &ColumnInfo> = target.iter().map(|column| (column.name.as_str(), column)).collect(); - let source_map: HashMap<&str, &ColumnInfo> = source.iter().map(|column| (column.name.as_str(), column)).collect(); + let target_map: HashMap = target + .iter() + .map(|column| (column_name_match_key(&column.name, options.case_insensitive_names), column)) + .collect(); + let source_map: HashMap = source + .iter() + .map(|column| (column_name_match_key(&column.name, options.case_insensitive_names), column)) + .collect(); let target_position_map: HashMap<&str, usize> = target.iter().enumerate().map(|(index, column)| (column.name.as_str(), index)).collect(); - let can_compare_order = compare_column_order + let target_normalized_position_map: HashMap = target + .iter() + .enumerate() + .map(|(index, column)| (column_name_match_key(&column.name, options.case_insensitive_names), index)) + .collect(); + let can_compare_order = options.compare_column_order && source.len() == target.len() - && source.iter().all(|column| target_map.contains_key(column.name.as_str())); + && source.iter().all(|column| { + target_map.contains_key(&column_name_match_key(&column.name, options.case_insensitive_names)) + }); for (source_index, source_column) in source.iter().enumerate() { - if let Some(target_column) = target_map.get(source_column.name.as_str()) { + let source_name_key = column_name_match_key(&source_column.name, options.case_insensitive_names); + if let Some(target_column) = target_map.get(&source_name_key) { let mut changes = Vec::new(); + if options.case_insensitive_names && source_column.name != target_column.name { + changes.push(format!("name: {} → {}", target_column.name, source_column.name)); + } if source_column.data_type.to_lowercase() != target_column.data_type.to_lowercase() { changes.push(format!("type: {} → {}", target_column.data_type, source_column.data_type)); } @@ -455,7 +481,7 @@ fn diff_columns_with_options( source_column.column_default.as_deref().unwrap_or("NULL") )); } - if !ignore_comments + if !options.ignore_comments && source_column.comment.as_deref().unwrap_or_default() != target_column.comment.as_deref().unwrap_or_default() { @@ -466,7 +492,12 @@ fn diff_columns_with_options( )); } if can_compare_order { - if let Some(target_index) = target_position_map.get(source_column.name.as_str()) { + let target_index = if options.case_insensitive_names { + target_normalized_position_map.get(&source_name_key) + } else { + target_position_map.get(source_column.name.as_str()) + }; + if let Some(target_index) = target_index { if source_index != *target_index { changes.push(format!("order: {} → {}", *target_index + 1, source_index + 1)); } @@ -493,7 +524,7 @@ fn diff_columns_with_options( } for target_column in target { - if !source_map.contains_key(target_column.name.as_str()) { + if !source_map.contains_key(&column_name_match_key(&target_column.name, options.case_insensitive_names)) { diffs.push(ColumnDiff { diff_type: "removed".to_string(), name: target_column.name.clone(), @@ -507,6 +538,20 @@ fn diff_columns_with_options( diffs } +fn column_name_match_key(name: &str, case_insensitive: bool) -> String { + if case_insensitive { + name.to_ascii_lowercase() + } else { + name.to_string() + } +} + +fn column_names_are_case_insensitive(db_type: DatabaseType) -> bool { + // MySQL documents column identifiers as case-insensitive on every platform; exact matching would turn + // a case-only rename into a destructive add/drop plan. + matches!(db_type, DatabaseType::Mysql) +} + pub fn diff_indexes(source: &[IndexInfo], target: &[IndexInfo]) -> Vec { let mut diffs = Vec::new(); let target_map: HashMap<&str, &IndexInfo> = target.iter().map(|index| (index.name.as_str(), index)).collect(); @@ -1143,7 +1188,16 @@ pub fn generate_schema_sync_sql( "modified" => { if let Some(source) = &column.source { if is_mysql { - if column.changes.iter().any(|change| !change.starts_with("order:")) { + let renames_column = column.changes.iter().any(|change| change.starts_with("name:")); + if renames_column { + if let Some(target) = &column.target { + parts.push(format!( + " CHANGE COLUMN {} {}", + quote_id(&target.name, db_type), + column_def(source, db_type) + )); + } + } else if column.changes.iter().any(|change| !change.starts_with("order:")) { parts.push(format!(" MODIFY COLUMN {}", column_def(source, db_type))); } } else { @@ -1463,8 +1517,7 @@ mod tests { let diffs = diff_columns_with_options( &[column("id", "int", None), column("name", "varchar(64)", None), column("status", "varchar(16)", None)], &[column("status", "varchar(16)", None), column("id", "int", None), column("name", "varchar(64)", None)], - false, - false, + ColumnDiffOptions::default(), ); assert!(diffs.is_empty()); @@ -1475,14 +1528,69 @@ mod tests { let diffs = diff_columns_with_options( &[column("id", "int", None), column("name", "varchar(64)", None), column("status", "varchar(16)", None)], &[column("status", "varchar(16)", None), column("id", "int", None), column("name", "varchar(64)", None)], - false, - true, + ColumnDiffOptions { compare_column_order: true, ..ColumnDiffOptions::default() }, ); assert_eq!(diffs.len(), 3); assert_eq!(diffs[0].changes, vec!["order: 2 → 1"]); } + #[test] + fn keeps_case_sensitive_column_names_distinct_by_default() { + let diffs = diff_columns_with_options( + &[column("FREEDATE", "datetime", None)], + &[column("freedate", "datetime", None)], + ColumnDiffOptions::default(), + ); + + assert_eq!(diffs.len(), 2); + assert_eq!(diffs[0].diff_type, "added"); + assert_eq!(diffs[1].diff_type, "removed"); + } + + #[test] + fn matches_mysql_column_names_case_insensitively() { + let diffs = diff_columns_with_options( + &[column("FREEDATE", "datetime", None), column("DATETYPE", "char(6)", Some("holiday type"))], + &[column("freedate", "datetime", None), column("datetype", "char(6)", Some("holiday type"))], + ColumnDiffOptions { case_insensitive_names: true, ..ColumnDiffOptions::default() }, + ); + + assert_eq!(diffs.len(), 2); + assert!(diffs.iter().all(|diff| diff.diff_type == "modified")); + assert_eq!(diffs[0].changes, vec!["name: freedate → FREEDATE"]); + assert_eq!(diffs[1].changes, vec!["name: datetype → DATETYPE"]); + + let sql = generate_schema_sync_sql( + &[TableDiff { + diff_type: "modified".to_string(), + object_type: Some("table".to_string()), + name: "c_freedate".to_string(), + columns: Some(diffs), + indexes: None, + foreign_keys: None, + triggers: None, + ddl: None, + target_ddl: None, + source_table_comment: None, + target_table_comment: None, + sync_sql: None, + }], + &[], + &[], + &[], + &[], + DatabaseType::Mysql, + None, + false, + ); + + assert!(sql.contains("CHANGE COLUMN `freedate` `FREEDATE` datetime NOT NULL")); + assert!(sql.contains("CHANGE COLUMN `datetype` `DATETYPE` char(6) NOT NULL COMMENT 'holiday type'")); + assert!(!sql.contains("ADD COLUMN")); + assert!(!sql.contains("DROP COLUMN")); + } + #[test] fn detects_modified_indexes_not_only_added_or_removed_indexes() { let diffs = diff_indexes(