fix(sqlserver): fix unchanged FK false warning and identity column detection
* Fix: improve sidebar comment font rendering * Fix: 修复 SQL Server 表结构保存时未修改外键误报 --------- Co-authored-by: zipg <4047349+zipg@users.noreply.github.com>
This commit is contained in:
1 parent
dbc9072d30
commit
cd55c3d3ac
4 files changed
+282
-6
No files matched your search
@@ -85,6 +85,7 @@ pub fn build_single_column_alter_sql(options: SingleColumnAlterSqlOptions) -> Ta
|
|||||||
&options.column,
|
&options.column,
|
||||||
options.schema.as_deref(),
|
options.schema.as_deref(),
|
||||||
&options.table_name,
|
&options.table_name,
|
||||||
|
&mut warnings,
|
||||||
)),
|
)),
|
||||||
StructureDialect::Sqlite => {
|
StructureDialect::Sqlite => {
|
||||||
statements.extend(build_sqlite_existing_column_sql(&table, &options.column, &mut warnings))
|
statements.extend(build_sqlite_existing_column_sql(&table, &options.column, &mut warnings))
|
||||||
@@ -117,6 +118,66 @@ fn original_manticore_extra_flags(extra: &str) -> (bool, bool, bool, bool) {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn original_has_auto_increment(extra: &str) -> bool {
|
||||||
|
let lower = extra.to_lowercase();
|
||||||
|
lower.contains("auto_increment") || lower.contains("autoincrement")
|
||||||
|
}
|
||||||
|
|
||||||
|
fn original_has_identity(extra: &str) -> bool {
|
||||||
|
extra.to_lowercase().contains("identity")
|
||||||
|
}
|
||||||
|
|
||||||
|
fn sqlserver_identity_values(extra: &str) -> Option<(Option<i64>, Option<i64>)> {
|
||||||
|
let lower = extra.to_lowercase();
|
||||||
|
let identity_index = lower.find("identity")?;
|
||||||
|
let after_identity = extra.get(identity_index + "identity".len()..)?.trim_start();
|
||||||
|
if !after_identity.starts_with('(') {
|
||||||
|
return Some((None, None));
|
||||||
|
}
|
||||||
|
let close_index = after_identity.find(')')?;
|
||||||
|
let args = &after_identity[1..close_index];
|
||||||
|
let mut parts = args.split(',').map(|part| part.trim().parse::<i64>().ok());
|
||||||
|
Some((parts.next().flatten(), parts.next().flatten()))
|
||||||
|
}
|
||||||
|
|
||||||
|
fn sqlserver_identity_matches_original(identity: &super::types::ColumnIdentity, original_extra: &str) -> bool {
|
||||||
|
let Some((original_seed, original_increment)) = sqlserver_identity_values(original_extra) else {
|
||||||
|
return false;
|
||||||
|
};
|
||||||
|
identity.seed.unwrap_or(1) == original_seed.unwrap_or(1)
|
||||||
|
&& identity.increment.unwrap_or(1) == original_increment.unwrap_or(1)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn parse_i64_after_phrase(value: &str, lower: &str, phrase: &str) -> Option<i64> {
|
||||||
|
let start = lower.find(phrase)? + phrase.len();
|
||||||
|
let rest = value.get(start..)?.trim_start();
|
||||||
|
let token = rest.split(|ch: char| ch.is_whitespace() || ch == ')' || ch == ',').find(|part| !part.is_empty())?;
|
||||||
|
token.parse::<i64>().ok()
|
||||||
|
}
|
||||||
|
|
||||||
|
fn postgres_identity_values(extra: &str) -> Option<(String, Option<i64>, Option<i64>)> {
|
||||||
|
let lower = extra.to_lowercase();
|
||||||
|
let generation = if lower.contains("generated always as identity") {
|
||||||
|
"ALWAYS".to_string()
|
||||||
|
} else if lower.contains("generated by default as identity") {
|
||||||
|
"BY DEFAULT".to_string()
|
||||||
|
} else {
|
||||||
|
return None;
|
||||||
|
};
|
||||||
|
let seed = parse_i64_after_phrase(extra, &lower, "start with");
|
||||||
|
let increment = parse_i64_after_phrase(extra, &lower, "increment by");
|
||||||
|
Some((generation, seed, increment))
|
||||||
|
}
|
||||||
|
|
||||||
|
fn identity_matches_original(identity: &super::types::ColumnIdentity, original_extra: &str) -> bool {
|
||||||
|
if let Some((generation, original_seed, original_increment)) = postgres_identity_values(original_extra) {
|
||||||
|
return identity.generation.as_deref().unwrap_or("BY DEFAULT").eq_ignore_ascii_case(&generation)
|
||||||
|
&& identity.seed.unwrap_or(1) == original_seed.unwrap_or(1)
|
||||||
|
&& identity.increment.unwrap_or(1) == original_increment.unwrap_or(1);
|
||||||
|
}
|
||||||
|
sqlserver_identity_matches_original(identity, original_extra)
|
||||||
|
}
|
||||||
|
|
||||||
pub(super) fn has_column_extra_change(column: &EditableStructureColumn) -> bool {
|
pub(super) fn has_column_extra_change(column: &EditableStructureColumn) -> bool {
|
||||||
let Some(original) = &column.original else { return false };
|
let Some(original) = &column.original else { return false };
|
||||||
let current_extra = column.extra.as_ref();
|
let current_extra = column.extra.as_ref();
|
||||||
@@ -128,8 +189,9 @@ pub(super) fn has_column_extra_change(column: &EditableStructureColumn) -> bool
|
|||||||
(Some(curr), Some(orig)) if is_column_extra_empty(curr) => {
|
(Some(curr), Some(orig)) if is_column_extra_empty(curr) => {
|
||||||
let (indexed, stored, attribute, secondary_index) = original_manticore_extra_flags(orig);
|
let (indexed, stored, attribute, secondary_index) = original_manticore_extra_flags(orig);
|
||||||
let orig_lower = orig.to_lowercase();
|
let orig_lower = orig.to_lowercase();
|
||||||
orig_lower.contains("auto_increment")
|
original_has_auto_increment(orig)
|
||||||
|| orig_lower.contains("on update")
|
|| orig_lower.contains("on update")
|
||||||
|
|| original_has_identity(orig)
|
||||||
|| indexed
|
|| indexed
|
||||||
|| stored
|
|| stored
|
||||||
|| attribute
|
|| attribute
|
||||||
@@ -142,10 +204,16 @@ pub(super) fn has_column_extra_change(column: &EditableStructureColumn) -> bool
|
|||||||
(Some(curr), Some(orig)) => {
|
(Some(curr), Some(orig)) => {
|
||||||
let orig_lower = orig.to_lowercase();
|
let orig_lower = orig.to_lowercase();
|
||||||
let curr_has_ai = curr.auto_increment.unwrap_or(false);
|
let curr_has_ai = curr.auto_increment.unwrap_or(false);
|
||||||
let orig_has_ai = orig_lower.contains("auto_increment");
|
let orig_has_identity = original_has_identity(orig);
|
||||||
|
let orig_has_ai = original_has_auto_increment(orig) || (orig_has_identity && curr_has_ai);
|
||||||
let curr_has_on_update = curr.on_update_current_timestamp.unwrap_or(false);
|
let curr_has_on_update = curr.on_update_current_timestamp.unwrap_or(false);
|
||||||
let orig_has_on_update = orig_lower.contains("on update");
|
let orig_has_on_update = orig_lower.contains("on update");
|
||||||
let curr_has_identity = curr.identity.is_some();
|
let identity_changed = match (&curr.identity, orig_has_identity) {
|
||||||
|
(Some(identity), true) => !identity_matches_original(identity, orig),
|
||||||
|
(Some(_), false) => true,
|
||||||
|
(None, true) => !curr_has_ai,
|
||||||
|
(None, false) => false,
|
||||||
|
};
|
||||||
let curr_manticore = (
|
let curr_manticore = (
|
||||||
curr.manticore_indexed.unwrap_or(false),
|
curr.manticore_indexed.unwrap_or(false),
|
||||||
curr.manticore_stored.unwrap_or(false),
|
curr.manticore_stored.unwrap_or(false),
|
||||||
@@ -153,10 +221,9 @@ pub(super) fn has_column_extra_change(column: &EditableStructureColumn) -> bool
|
|||||||
curr.manticore_secondary_index.unwrap_or(false),
|
curr.manticore_secondary_index.unwrap_or(false),
|
||||||
);
|
);
|
||||||
let orig_manticore = original_manticore_extra_flags(orig);
|
let orig_manticore = original_manticore_extra_flags(orig);
|
||||||
// identity is harder to detect in free-form original.extra, so treat it as changed if present
|
|
||||||
curr_has_ai != orig_has_ai
|
curr_has_ai != orig_has_ai
|
||||||
|| curr_has_on_update != orig_has_on_update
|
|| curr_has_on_update != orig_has_on_update
|
||||||
|| curr_has_identity
|
|| identity_changed
|
||||||
|| curr_manticore != orig_manticore
|
|| curr_manticore != orig_manticore
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -322,6 +389,7 @@ pub(super) fn build_sqlserver_existing_column_sql(
|
|||||||
column: &EditableStructureColumn,
|
column: &EditableStructureColumn,
|
||||||
schema: Option<&str>,
|
schema: Option<&str>,
|
||||||
table_name: &str,
|
table_name: &str,
|
||||||
|
warnings: &mut Vec<String>,
|
||||||
) -> Vec<String> {
|
) -> Vec<String> {
|
||||||
let Some(original) = &column.original else {
|
let Some(original) = &column.original else {
|
||||||
return Vec::new();
|
return Vec::new();
|
||||||
@@ -330,6 +398,13 @@ pub(super) fn build_sqlserver_existing_column_sql(
|
|||||||
let mut statements = Vec::new();
|
let mut statements = Vec::new();
|
||||||
let mut current_name = original.name.clone();
|
let mut current_name = original.name.clone();
|
||||||
|
|
||||||
|
if has_sqlserver_identity_change(column) {
|
||||||
|
warnings.push(format!(
|
||||||
|
"Changing SQL Server IDENTITY for existing column \"{}\" is not supported from this editor.",
|
||||||
|
original.name
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
// Rename column via sp_rename
|
// Rename column via sp_rename
|
||||||
if column.name != original.name {
|
if column.name != original.name {
|
||||||
let full_obj_path = format!("{table}.{}", quote_ident(dialect, &original.name));
|
let full_obj_path = format!("{table}.{}", quote_ident(dialect, &original.name));
|
||||||
@@ -409,6 +484,28 @@ pub(super) fn build_sqlserver_existing_column_sql(
|
|||||||
statements
|
statements
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn has_sqlserver_identity_change(column: &EditableStructureColumn) -> bool {
|
||||||
|
let Some(original) = &column.original else {
|
||||||
|
return false;
|
||||||
|
};
|
||||||
|
let original_extra = original.extra.as_deref().unwrap_or("");
|
||||||
|
let original_identity = original_has_identity(original_extra);
|
||||||
|
let current_extra = column.extra.as_ref();
|
||||||
|
let current_identity =
|
||||||
|
current_extra.is_some_and(|extra| extra.auto_increment.unwrap_or(false) || extra.identity.is_some());
|
||||||
|
|
||||||
|
if current_identity != original_identity {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if !current_identity {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
current_extra
|
||||||
|
.and_then(|extra| extra.identity.as_ref())
|
||||||
|
.is_some_and(|identity| !sqlserver_identity_matches_original(identity, original_extra))
|
||||||
|
}
|
||||||
|
|
||||||
pub(super) fn build_h2_existing_column_sql(table: &str, column: &EditableStructureColumn) -> Vec<String> {
|
pub(super) fn build_h2_existing_column_sql(table: &str, column: &EditableStructureColumn) -> Vec<String> {
|
||||||
let Some(original) = &column.original else {
|
let Some(original) = &column.original else {
|
||||||
return Vec::new();
|
return Vec::new();
|
||||||
|
|||||||
@@ -127,6 +127,7 @@ pub(super) fn build_column_sql(options: &TableStructureSqlOptions, warnings: &mu
|
|||||||
column,
|
column,
|
||||||
options.schema.as_deref(),
|
options.schema.as_deref(),
|
||||||
&options.table_name,
|
&options.table_name,
|
||||||
|
warnings,
|
||||||
)),
|
)),
|
||||||
StructureDialect::Sqlite => statements.extend(build_sqlite_existing_column_sql(&table, column, warnings)),
|
StructureDialect::Sqlite => statements.extend(build_sqlite_existing_column_sql(&table, column, warnings)),
|
||||||
StructureDialect::Questdb => statements.extend(build_questdb_existing_column_sql(&table, column)),
|
StructureDialect::Questdb => statements.extend(build_questdb_existing_column_sql(&table, column)),
|
||||||
|
|||||||
@@ -11,7 +11,9 @@ pub(super) fn build_foreign_key_sql(options: &TableStructureSqlOptions, warnings
|
|||||||
let capabilities = super::dialect::capabilities_for(options.database_type);
|
let capabilities = super::dialect::capabilities_for(options.database_type);
|
||||||
let dialect = capabilities.dialect;
|
let dialect = capabilities.dialect;
|
||||||
if !capabilities.foreign_key {
|
if !capabilities.foreign_key {
|
||||||
warnings.push(format!("Editing foreign keys is not supported for {database_label} from this editor."));
|
if has_foreign_key_edits(&options.foreign_keys) {
|
||||||
|
warnings.push(format!("Editing foreign keys is not supported for {database_label} from this editor."));
|
||||||
|
}
|
||||||
return Vec::new();
|
return Vec::new();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -41,6 +43,29 @@ pub(super) fn build_foreign_key_sql(options: &TableStructureSqlOptions, warnings
|
|||||||
statements
|
statements
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn has_foreign_key_edits(foreign_keys: &[EditableStructureForeignKey]) -> bool {
|
||||||
|
foreign_keys.iter().any(|foreign_key| {
|
||||||
|
if foreign_key.marked_for_drop {
|
||||||
|
return foreign_key.original.is_some();
|
||||||
|
}
|
||||||
|
|
||||||
|
match &foreign_key.original {
|
||||||
|
Some(original) => has_foreign_key_change(foreign_key, original),
|
||||||
|
None => has_foreign_key_definition(foreign_key),
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
fn has_foreign_key_definition(foreign_key: &EditableStructureForeignKey) -> bool {
|
||||||
|
!clean(&foreign_key.name).is_empty()
|
||||||
|
|| !clean(&foreign_key.column).is_empty()
|
||||||
|
|| !clean(&foreign_key.ref_schema).is_empty()
|
||||||
|
|| !clean(&foreign_key.ref_table).is_empty()
|
||||||
|
|| !clean(&foreign_key.ref_column).is_empty()
|
||||||
|
|| !clean(&foreign_key.on_update).is_empty()
|
||||||
|
|| !clean(&foreign_key.on_delete).is_empty()
|
||||||
|
}
|
||||||
|
|
||||||
fn has_foreign_key_change(foreign_key: &EditableStructureForeignKey, original: &ForeignKeyInfo) -> bool {
|
fn has_foreign_key_change(foreign_key: &EditableStructureForeignKey, original: &ForeignKeyInfo) -> bool {
|
||||||
clean(&foreign_key.name) != clean(&original.name)
|
clean(&foreign_key.name) != clean(&original.name)
|
||||||
|| clean(&foreign_key.column) != clean(&original.column)
|
|| clean(&foreign_key.column) != clean(&original.column)
|
||||||
|
|||||||
@@ -1000,6 +1000,159 @@ fn builds_sql_server_quoted_column_and_index_statements() {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn sqlserver_unchanged_foreign_key_does_not_warn_when_saving_other_changes() {
|
||||||
|
let mut email = column("email");
|
||||||
|
email.data_type = "nvarchar(255)".to_string();
|
||||||
|
email.is_nullable = false;
|
||||||
|
|
||||||
|
let mut user_fk = foreign_key("fk_orders_user_id", "user_id", "users", "id");
|
||||||
|
user_fk.ref_schema = "dbo".to_string();
|
||||||
|
user_fk.original = Some(ForeignKeyInfo {
|
||||||
|
name: "fk_orders_user_id".to_string(),
|
||||||
|
column: "user_id".to_string(),
|
||||||
|
ref_schema: Some("dbo".to_string()),
|
||||||
|
ref_table: "users".to_string(),
|
||||||
|
ref_column: "id".to_string(),
|
||||||
|
on_update: None,
|
||||||
|
on_delete: None,
|
||||||
|
});
|
||||||
|
|
||||||
|
let result = build_table_structure_change_sql(TableStructureSqlOptions {
|
||||||
|
database_type: Some(DatabaseType::SqlServer),
|
||||||
|
schema: Some("dbo".to_string()),
|
||||||
|
table_name: "orders".to_string(),
|
||||||
|
columns: vec![email],
|
||||||
|
indexes: Vec::new(),
|
||||||
|
foreign_keys: vec![user_fk],
|
||||||
|
triggers: Vec::new(),
|
||||||
|
table_comment: None,
|
||||||
|
original_table_comment: None,
|
||||||
|
});
|
||||||
|
|
||||||
|
assert_eq!(result.warnings, Vec::<String>::new());
|
||||||
|
assert_eq!(result.statements, vec!["ALTER TABLE [dbo].[orders] ADD [email] nvarchar(255) NOT NULL;"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn sqlserver_changed_foreign_key_still_warns_as_unsupported() {
|
||||||
|
let mut user_fk = foreign_key("fk_orders_user_id", "user_id", "accounts", "id");
|
||||||
|
user_fk.ref_schema = "dbo".to_string();
|
||||||
|
user_fk.original = Some(ForeignKeyInfo {
|
||||||
|
name: "fk_orders_user_id".to_string(),
|
||||||
|
column: "user_id".to_string(),
|
||||||
|
ref_schema: Some("dbo".to_string()),
|
||||||
|
ref_table: "users".to_string(),
|
||||||
|
ref_column: "id".to_string(),
|
||||||
|
on_update: None,
|
||||||
|
on_delete: None,
|
||||||
|
});
|
||||||
|
|
||||||
|
let result = build_table_structure_change_sql(TableStructureSqlOptions {
|
||||||
|
database_type: Some(DatabaseType::SqlServer),
|
||||||
|
schema: Some("dbo".to_string()),
|
||||||
|
table_name: "orders".to_string(),
|
||||||
|
columns: Vec::new(),
|
||||||
|
indexes: Vec::new(),
|
||||||
|
foreign_keys: vec![user_fk],
|
||||||
|
triggers: Vec::new(),
|
||||||
|
table_comment: None,
|
||||||
|
original_table_comment: None,
|
||||||
|
});
|
||||||
|
|
||||||
|
assert_eq!(result.statements, Vec::<String>::new());
|
||||||
|
assert_eq!(result.warnings, vec!["Editing foreign keys is not supported for sqlserver from this editor."]);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn sqlserver_unchanged_identity_extra_does_not_mark_existing_column_changed() {
|
||||||
|
let mut id = column("id");
|
||||||
|
id.data_type = "int".to_string();
|
||||||
|
id.is_nullable = false;
|
||||||
|
id.is_primary_key = true;
|
||||||
|
id.extra = Some(ColumnExtra {
|
||||||
|
auto_increment: Some(true),
|
||||||
|
identity: Some(ColumnIdentity { generation: None, seed: Some(1), increment: Some(1) }),
|
||||||
|
..Default::default()
|
||||||
|
});
|
||||||
|
id.original = Some(ColumnInfo {
|
||||||
|
name: "id".to_string(),
|
||||||
|
data_type: "int".to_string(),
|
||||||
|
is_nullable: false,
|
||||||
|
column_default: None,
|
||||||
|
is_primary_key: true,
|
||||||
|
extra: Some("IDENTITY(1,1)".to_string()),
|
||||||
|
comment: None,
|
||||||
|
});
|
||||||
|
|
||||||
|
let result = build_table_structure_change_sql(TableStructureSqlOptions {
|
||||||
|
database_type: Some(DatabaseType::SqlServer),
|
||||||
|
schema: Some("dbo".to_string()),
|
||||||
|
table_name: "orders".to_string(),
|
||||||
|
columns: vec![id],
|
||||||
|
indexes: Vec::new(),
|
||||||
|
foreign_keys: Vec::new(),
|
||||||
|
triggers: Vec::new(),
|
||||||
|
table_comment: None,
|
||||||
|
original_table_comment: None,
|
||||||
|
});
|
||||||
|
|
||||||
|
assert_eq!(result.warnings, Vec::<String>::new());
|
||||||
|
assert_eq!(result.statements, Vec::<String>::new());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn sqlserver_existing_column_identity_change_warns_without_unchanged_foreign_key_warning() {
|
||||||
|
let mut id = column("id");
|
||||||
|
id.data_type = "int".to_string();
|
||||||
|
id.is_nullable = false;
|
||||||
|
id.is_primary_key = true;
|
||||||
|
id.extra = Some(ColumnExtra {
|
||||||
|
auto_increment: Some(true),
|
||||||
|
identity: Some(ColumnIdentity { generation: None, seed: Some(1), increment: Some(1) }),
|
||||||
|
..Default::default()
|
||||||
|
});
|
||||||
|
id.original = Some(ColumnInfo {
|
||||||
|
name: "id".to_string(),
|
||||||
|
data_type: "int".to_string(),
|
||||||
|
is_nullable: false,
|
||||||
|
column_default: None,
|
||||||
|
is_primary_key: true,
|
||||||
|
extra: None,
|
||||||
|
comment: None,
|
||||||
|
});
|
||||||
|
|
||||||
|
let mut user_fk = foreign_key("fk_orders_user_id", "user_id", "users", "id");
|
||||||
|
user_fk.ref_schema = "dbo".to_string();
|
||||||
|
user_fk.original = Some(ForeignKeyInfo {
|
||||||
|
name: "fk_orders_user_id".to_string(),
|
||||||
|
column: "user_id".to_string(),
|
||||||
|
ref_schema: Some("dbo".to_string()),
|
||||||
|
ref_table: "users".to_string(),
|
||||||
|
ref_column: "id".to_string(),
|
||||||
|
on_update: None,
|
||||||
|
on_delete: None,
|
||||||
|
});
|
||||||
|
|
||||||
|
let result = build_table_structure_change_sql(TableStructureSqlOptions {
|
||||||
|
database_type: Some(DatabaseType::SqlServer),
|
||||||
|
schema: Some("dbo".to_string()),
|
||||||
|
table_name: "orders".to_string(),
|
||||||
|
columns: vec![id],
|
||||||
|
indexes: Vec::new(),
|
||||||
|
foreign_keys: vec![user_fk],
|
||||||
|
triggers: Vec::new(),
|
||||||
|
table_comment: None,
|
||||||
|
original_table_comment: None,
|
||||||
|
});
|
||||||
|
|
||||||
|
assert_eq!(result.statements, Vec::<String>::new());
|
||||||
|
assert_eq!(
|
||||||
|
result.warnings,
|
||||||
|
vec!["Changing SQL Server IDENTITY for existing column \"id\" is not supported from this editor."]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(feature = "duckdb-bundled")]
|
#[cfg(feature = "duckdb-bundled")]
|
||||||
#[test]
|
#[test]
|
||||||
fn builds_duckdb_create_table_statements() {
|
fn builds_duckdb_create_table_statements() {
|
||||||
|
|||||||
Reference in new issue
Block a user