fix(sqlserver): restrict identity to compatible column types
Co-authored-by: staff <staff@qimaos-MacBook-Pro.local>
This commit is contained in:
1 parent
a224a6747c
commit
5d2ce9f189
13 files changed
+144
-12
No files matched your search
@@ -63,6 +63,16 @@ pub(super) fn build_column_sql(options: &TableStructureSqlOptions, warnings: &mu
|
||||
warnings.push(format!("Adding columns is not supported for {database_label} from this editor."));
|
||||
continue;
|
||||
}
|
||||
if dialect == StructureDialect::SqlServer
|
||||
&& has_sqlserver_identity(column)
|
||||
&& !is_sqlserver_identity_compatible_type(&column.data_type)
|
||||
{
|
||||
warnings.push(format!(
|
||||
"SQL Server identity column \"{}\" must use tinyint, smallint, int, bigint, or decimal/numeric with scale 0.",
|
||||
column.name
|
||||
));
|
||||
continue;
|
||||
}
|
||||
statements.extend(build_add_column_sql(
|
||||
dialect,
|
||||
options.database_type,
|
||||
@@ -199,6 +209,38 @@ pub(super) fn build_primary_key_sql(
|
||||
statements
|
||||
}
|
||||
|
||||
fn has_sqlserver_identity(column: &EditableStructureColumn) -> bool {
|
||||
column.extra.as_ref().is_some_and(|extra| extra.auto_increment.unwrap_or(false) || extra.identity.is_some())
|
||||
}
|
||||
|
||||
fn is_sqlserver_identity_compatible_type(data_type: &str) -> bool {
|
||||
let trimmed = data_type.trim();
|
||||
let (base_type, params) = match trimmed.find('(') {
|
||||
Some(open_index) => {
|
||||
let close_index = trimmed.rfind(')').unwrap_or(trimmed.len());
|
||||
(&trimmed[..open_index], trimmed.get(open_index + 1..close_index).unwrap_or(""))
|
||||
}
|
||||
None => (trimmed, ""),
|
||||
};
|
||||
let normalized = base_type.split_whitespace().collect::<Vec<_>>().join(" ").to_ascii_lowercase();
|
||||
if matches!(normalized.as_str(), "tinyint" | "smallint" | "int" | "integer" | "bigint") {
|
||||
return true;
|
||||
}
|
||||
if !matches!(normalized.as_str(), "decimal" | "numeric") {
|
||||
return false;
|
||||
}
|
||||
let normalized_params = params.split_whitespace().collect::<String>();
|
||||
if normalized_params.is_empty() {
|
||||
return true;
|
||||
}
|
||||
let parts = normalized_params.split(',').collect::<Vec<_>>();
|
||||
match parts.as_slice() {
|
||||
[precision] => precision.parse::<u32>().is_ok(),
|
||||
[precision, scale] => precision.parse::<u32>().is_ok() && *scale == "0",
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn build_add_column_sql(
|
||||
dialect: StructureDialect,
|
||||
database_type: Option<crate::models::connection::DatabaseType>,
|
||||
|
||||
@@ -1208,6 +1208,36 @@ fn sqlserver_add_column_with_identity() {
|
||||
assert_eq!(result.statements, vec!["ALTER TABLE [dbo].[orders] ADD [id] int NOT NULL IDENTITY(10, 2);"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sqlserver_rejects_identity_on_incompatible_type() {
|
||||
let mut column = column("test");
|
||||
column.data_type = "varchar(255)".to_string();
|
||||
column.is_nullable = false;
|
||||
column.extra = Some(ColumnExtra {
|
||||
auto_increment: Some(true),
|
||||
identity: Some(ColumnIdentity { generation: None, seed: Some(1), increment: Some(1) }),
|
||||
..Default::default()
|
||||
});
|
||||
|
||||
let result = build_table_structure_change_sql(TableStructureSqlOptions {
|
||||
database_type: Some(DatabaseType::SqlServer),
|
||||
schema: Some("core".to_string()),
|
||||
table_name: "products".to_string(),
|
||||
columns: vec![column],
|
||||
indexes: Vec::new(),
|
||||
foreign_keys: Vec::new(),
|
||||
triggers: Vec::new(),
|
||||
table_comment: None,
|
||||
original_table_comment: None,
|
||||
});
|
||||
|
||||
assert_eq!(result.statements, Vec::<String>::new());
|
||||
assert_eq!(
|
||||
result.warnings,
|
||||
vec!["SQL Server identity column \"test\" must use tinyint, smallint, int, bigint, or decimal/numeric with scale 0."]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sqlserver_changed_foreign_key_still_warns_as_unsupported() {
|
||||
let mut user_fk = foreign_key("fk_orders_user_id", "user_id", "accounts", "id");
|
||||
|
||||
Reference in new issue
Block a user