diff --git a/crates/dbx-core/src/table_structure_sql/column_alter.rs b/crates/dbx-core/src/table_structure_sql/column_alter.rs index 6d6e320d9d..1d8cc301d0 100644 --- a/crates/dbx-core/src/table_structure_sql/column_alter.rs +++ b/crates/dbx-core/src/table_structure_sql/column_alter.rs @@ -475,6 +475,14 @@ pub(super) fn build_sqlserver_existing_column_sql( let dialect = StructureDialect::SqlServer; let mut statements = Vec::new(); let mut current_name = original.name.clone(); + let has_column_definition_change = + column.data_type.trim() != original.data_type.trim() || column.is_nullable != original.is_nullable; + let default_value = normalize_default(Some(&column.default_value)); + let has_default_change = default_value != original_default(column); + let has_old_default = original + .column_default + .as_deref() + .is_some_and(|value| !value.trim().is_empty() && !value.trim().eq_ignore_ascii_case("null")); if has_sqlserver_identity_change(column) { warnings.push(format!( @@ -494,44 +502,28 @@ pub(super) fn build_sqlserver_existing_column_sql( current_name = column.name.clone(); } - // Build the ALTER COLUMN clause for type + nullability changes - if column.data_type.trim() != original.data_type.trim() || column.is_nullable != original.is_nullable { - let null_clause = if column.is_nullable { "NULL" } else { "NOT NULL" }; - statements.push(format!( - "ALTER TABLE {table} ALTER COLUMN {} {} {null_clause};", - quote_ident(dialect, ¤t_name), - column_data_type(dialect, column) - )); + // SQL Server default constraints are separate objects that can block ALTER COLUMN. + // Preserve the exact constraint name and expression when the default itself is unchanged. + if has_column_definition_change && has_old_default && !has_default_change { + statements.push(build_sqlserver_alter_column_preserving_default_sql(table, ¤t_name, column)); + } else { + if has_column_definition_change && has_old_default { + statements.push(build_sqlserver_drop_default_constraint_sql(table, ¤t_name)); + } + if has_column_definition_change { + let null_clause = if column.is_nullable { "NULL" } else { "NOT NULL" }; + statements.push(format!( + "ALTER TABLE {table} ALTER COLUMN {} {} {null_clause};", + quote_ident(dialect, ¤t_name), + column_data_type(dialect, column) + )); + } } - // Default value changes via ADD/DROP CONSTRAINT - if normalize_default(Some(&column.default_value)) != original_default(column) { - let default_value = normalize_default(Some(&column.default_value)); - let has_old_default = !column - .original - .as_ref() - .and_then(|o| o.column_default.as_ref()) - .unwrap_or(&String::new()) - .trim() - .is_empty() - && !column - .original - .as_ref() - .and_then(|o| o.column_default.as_ref()) - .map(|d| d.trim().eq_ignore_ascii_case("null")) - .unwrap_or(false); - - if has_old_default { - let sql_var = sqlserver_default_constraint_sql_var(table, ¤t_name); - statements.push(format!( - "DECLARE {sql_var} NVARCHAR(MAX); \ - SELECT TOP (1) {sql_var} = N'ALTER TABLE {table} DROP CONSTRAINT ' + QUOTENAME(dc.name) \ - FROM sys.default_constraints AS dc \ - WHERE dc.parent_object_id = OBJECT_ID(N'{table}') AND dc.parent_column_id = COLUMNPROPERTY(OBJECT_ID(N'{table}'), N'{col_name}', 'ColumnId'); \ - IF {sql_var} IS NOT NULL EXEC sp_executesql {sql_var};", - table = table.replace('\'', "''"), - col_name = current_name.replace('\'', "''") - )); + // Default value changes via ADD/DROP CONSTRAINT. + if has_default_change { + if has_old_default && !has_column_definition_change { + statements.push(build_sqlserver_drop_default_constraint_sql(table, ¤t_name)); } if !default_value.is_empty() { let short_table = @@ -563,6 +555,47 @@ pub(super) fn build_sqlserver_existing_column_sql( statements } +fn build_sqlserver_alter_column_preserving_default_sql( + table: &str, + column_name: &str, + column: &EditableStructureColumn, +) -> String { + let dialect = StructureDialect::SqlServer; + let sql_var = sqlserver_default_constraint_sql_var(table, column_name); + let name_var = format!("{sql_var}_name"); + let definition_var = format!("{sql_var}_definition"); + let table_literal = table.replace('\'', "''"); + let column_literal = column_name.replace('\'', "''"); + let quoted_column = quote_ident(dialect, column_name); + let quoted_column_literal = quoted_column.replace('\'', "''"); + let null_clause = if column.is_nullable { "NULL" } else { "NOT NULL" }; + + format!( + "DECLARE {sql_var} NVARCHAR(MAX), {name_var} sysname, {definition_var} NVARCHAR(MAX); \ + SELECT TOP (1) {name_var} = dc.name, {definition_var} = dc.definition \ + FROM sys.default_constraints AS dc \ + WHERE dc.parent_object_id = OBJECT_ID(N'{table_literal}') AND dc.parent_column_id = COLUMNPROPERTY(OBJECT_ID(N'{table_literal}'), N'{column_literal}', 'ColumnId'); \ + IF {name_var} IS NOT NULL BEGIN SET {sql_var} = N'ALTER TABLE {table_literal} DROP CONSTRAINT ' + QUOTENAME({name_var}); EXEC sp_executesql {sql_var}; END; \ + ALTER TABLE {table} ALTER COLUMN {quoted_column} {data_type} {null_clause}; \ + IF {name_var} IS NOT NULL BEGIN SET {sql_var} = N'ALTER TABLE {table_literal} ADD CONSTRAINT ' + QUOTENAME({name_var}) + N' DEFAULT ' + {definition_var} + N' FOR {quoted_column_literal}'; EXEC sp_executesql {sql_var}; END;", + data_type = column_data_type(dialect, column), + ) +} + +fn build_sqlserver_drop_default_constraint_sql(table: &str, column_name: &str) -> String { + let sql_var = sqlserver_default_constraint_sql_var(table, column_name); + let table_literal = table.replace('\'', "''"); + let column_literal = column_name.replace('\'', "''"); + + format!( + "DECLARE {sql_var} NVARCHAR(MAX); \ + SELECT TOP (1) {sql_var} = N'ALTER TABLE {table_literal} DROP CONSTRAINT ' + QUOTENAME(dc.name) \ + FROM sys.default_constraints AS dc \ + WHERE dc.parent_object_id = OBJECT_ID(N'{table_literal}') AND dc.parent_column_id = COLUMNPROPERTY(OBJECT_ID(N'{table_literal}'), N'{column_literal}', 'ColumnId'); \ + IF {sql_var} IS NOT NULL EXEC sp_executesql {sql_var};" + ) +} + fn sqlserver_default_constraint_sql_var(table: &str, column_name: &str) -> String { let mut hash = 0x811c_9dc5u32; for byte in table.bytes().chain([0]).chain(column_name.bytes()) { diff --git a/crates/dbx-core/src/table_structure_sql/tests.rs b/crates/dbx-core/src/table_structure_sql/tests.rs index d753c865de..e9e349de78 100644 --- a/crates/dbx-core/src/table_structure_sql/tests.rs +++ b/crates/dbx-core/src/table_structure_sql/tests.rs @@ -1703,6 +1703,125 @@ fn sqlserver_default_changes_drop_old_constraints_with_isolated_batches() { ); } +#[test] +fn sqlserver_type_change_preserves_existing_default_constraint() { + let mut check_value = column("check_value"); + check_value.data_type = "decimal(18,2)".to_string(); + check_value.is_nullable = false; + check_value.default_value = "0".to_string(); + check_value.original = Some(ColumnInfo { + name: "check_value".to_string(), + data_type: "int".to_string(), + is_nullable: false, + column_default: Some("0".to_string()), + ..Default::default() + }); + + let result = build_single_column_alter_sql(SingleColumnAlterSqlOptions { + database_type: Some(DatabaseType::SqlServer), + schema: Some("dbo".to_string()), + table_name: "issue_3714".to_string(), + column: check_value, + }); + + assert_eq!(result.warnings, Vec::::new()); + assert_eq!(result.statements.len(), 1); + let sql = &result.statements[0]; + let capture = sql.find("= dc.name").unwrap(); + let drop = sql.find("DROP CONSTRAINT").unwrap(); + let alter = sql.find("ALTER COLUMN [check_value] decimal(18,2) NOT NULL").unwrap(); + let restore = sql.rfind("ADD CONSTRAINT").unwrap(); + assert!(capture < drop && drop < alter && alter < restore); + assert!(sql.contains("= dc.definition")); + assert!(sql.contains("QUOTENAME(@dbx_default_sql_")); + assert!(sql.contains("+ N' DEFAULT ' + @dbx_default_sql_")); + assert!(sql.contains("+ N' FOR [check_value]'")); +} + +#[test] +fn sqlserver_type_and_default_change_drops_before_alter_and_adds_new_default() { + let mut quantity = column("quantity"); + quantity.data_type = "decimal(12,3)".to_string(); + quantity.is_nullable = false; + quantity.default_value = "1.5".to_string(); + quantity.original = Some(ColumnInfo { + name: "quantity".to_string(), + data_type: "int".to_string(), + is_nullable: false, + column_default: Some("0".to_string()), + ..Default::default() + }); + + let result = build_single_column_alter_sql(SingleColumnAlterSqlOptions { + database_type: Some(DatabaseType::SqlServer), + schema: Some("dbo".to_string()), + table_name: "inventory".to_string(), + column: quantity, + }); + + assert_eq!(result.warnings, Vec::::new()); + assert_eq!(result.statements.len(), 3); + assert!(result.statements[0].contains("DROP CONSTRAINT")); + assert_eq!(result.statements[1], "ALTER TABLE [dbo].[inventory] ALTER COLUMN [quantity] decimal(12,3) NOT NULL;"); + assert_eq!( + result.statements[2], + "ALTER TABLE [dbo].[inventory] ADD CONSTRAINT [DF_inventory_quantity] DEFAULT 1.5 FOR [quantity];" + ); +} + +#[test] +fn sqlserver_rename_and_nullability_change_restores_default_on_new_column_name() { + let mut renamed = column("is_enabled"); + renamed.data_type = "bit".to_string(); + renamed.is_nullable = false; + renamed.default_value = "1".to_string(); + renamed.original = Some(ColumnInfo { + name: "enabled".to_string(), + data_type: "bit".to_string(), + is_nullable: true, + column_default: Some("1".to_string()), + ..Default::default() + }); + + let result = build_single_column_alter_sql(SingleColumnAlterSqlOptions { + database_type: Some(DatabaseType::SqlServer), + schema: Some("dbo".to_string()), + table_name: "settings".to_string(), + column: renamed, + }); + + assert_eq!(result.warnings, Vec::::new()); + assert_eq!(result.statements.len(), 2); + assert_eq!(result.statements[0], "EXEC sp_rename '[dbo].[settings].[enabled]', 'is_enabled', 'COLUMN';"); + assert!(result.statements[1].contains("N'is_enabled', 'ColumnId'")); + assert!(result.statements[1].contains("ALTER COLUMN [is_enabled] bit NOT NULL")); + assert!(result.statements[1].contains("FOR [is_enabled]")); +} + +#[test] +fn sqlserver_type_change_without_default_keeps_direct_alter_behavior() { + let mut value = column("value"); + value.data_type = "bigint".to_string(); + value.is_nullable = false; + value.original = Some(ColumnInfo { + name: "value".to_string(), + data_type: "int".to_string(), + is_nullable: false, + column_default: None, + ..Default::default() + }); + + let result = build_single_column_alter_sql(SingleColumnAlterSqlOptions { + database_type: Some(DatabaseType::SqlServer), + schema: Some("dbo".to_string()), + table_name: "metrics".to_string(), + column: value, + }); + + assert_eq!(result.warnings, Vec::::new()); + assert_eq!(result.statements, vec!["ALTER TABLE [dbo].[metrics] ALTER COLUMN [value] bigint NOT NULL;"]); +} + #[test] fn sqlserver_unchanged_foreign_key_does_not_warn_when_saving_other_changes() { let mut email = column("email");