fix(table-editor): clear stale auto-increment flag on PK change
This commit is contained in:
1 parent
203eab3dec
commit
cd8300c3f5
2 files changed
+204
-4
No files matched your search
@@ -12,6 +12,7 @@ use super::column_format::{
|
|||||||
};
|
};
|
||||||
use super::comments::build_sqlserver_column_comment_sql;
|
use super::comments::build_sqlserver_column_comment_sql;
|
||||||
use super::dialect::{capabilities_for, database_label, is_oracle_like, StructureDialect};
|
use super::dialect::{capabilities_for, database_label, is_oracle_like, StructureDialect};
|
||||||
|
use super::indexes::has_existing_index_change;
|
||||||
use super::types::{EditableStructureColumn, TableStructureSqlOptions};
|
use super::types::{EditableStructureColumn, TableStructureSqlOptions};
|
||||||
use super::util::{
|
use super::util::{
|
||||||
clean, is_protected_manticore_id_column, normalize_default, original_comment, original_default, qualified_table,
|
clean, is_protected_manticore_id_column, normalize_default, original_comment, original_default, qualified_table,
|
||||||
@@ -176,7 +177,18 @@ pub(super) fn build_column_sql(options: &TableStructureSqlOptions, warnings: &mu
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
if !has_existing_column_attribute_change(column) && !has_column_extra_change(column) && !has_position_change {
|
// A column handing its primary key membership over to another column keeps its
|
||||||
|
// AUTO_INCREMENT flag in the submitted draft, which MySQL rejects (ERROR 1075) once
|
||||||
|
// the column is no longer keyed. Rewrite it inside the same coalesced ALTER even
|
||||||
|
// though the draft reports no change on the column itself.
|
||||||
|
let clears_orphaned_auto_increment = mysql_coalesced_primary_key_change
|
||||||
|
.is_some_and(|change| mysql_orphans_auto_increment_column(options, column, change));
|
||||||
|
|
||||||
|
if !has_existing_column_attribute_change(column)
|
||||||
|
&& !has_column_extra_change(column)
|
||||||
|
&& !has_position_change
|
||||||
|
&& !clears_orphaned_auto_increment
|
||||||
|
{
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
let original = column.original.as_ref().unwrap();
|
let original = column.original.as_ref().unwrap();
|
||||||
@@ -221,14 +233,23 @@ pub(super) fn build_column_sql(options: &TableStructureSqlOptions, warnings: &mu
|
|||||||
));
|
));
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if !has_rename && !has_attribute_change && !has_position_change {
|
if !has_rename && !has_attribute_change && !has_position_change && !clears_orphaned_auto_increment {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
match dialect {
|
match dialect {
|
||||||
StructureDialect::Mysql => {
|
StructureDialect::Mysql => {
|
||||||
let clause =
|
let rewritten;
|
||||||
build_mysql_existing_column_clause(column, if has_position_change { &position_clause } else { "" });
|
let effective_column = if clears_orphaned_auto_increment {
|
||||||
|
rewritten = without_auto_increment(column);
|
||||||
|
&rewritten
|
||||||
|
} else {
|
||||||
|
column
|
||||||
|
};
|
||||||
|
let clause = build_mysql_existing_column_clause(
|
||||||
|
effective_column,
|
||||||
|
if has_position_change { &position_clause } else { "" },
|
||||||
|
);
|
||||||
if mysql_coalesced_primary_key_change
|
if mysql_coalesced_primary_key_change
|
||||||
.is_some_and(|change| mysql_auto_increment_touches_primary_key(column, change))
|
.is_some_and(|change| mysql_auto_increment_touches_primary_key(column, change))
|
||||||
{
|
{
|
||||||
@@ -377,6 +398,54 @@ fn mysql_auto_increment_touches_primary_key(column: &EditableStructureColumn, ch
|
|||||||
&& (change.old_ids.contains(column.id.as_str()) || change.new_ids.contains(column.id.as_str()))
|
&& (change.old_ids.contains(column.id.as_str()) || change.new_ids.contains(column.id.as_str()))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// MySQL allows AUTO_INCREMENT only on a column that leads some key, so a column handing the
|
||||||
|
/// primary key over to another column must lose the flag in the same statement — unless an
|
||||||
|
/// already existing index that survives this change still keys it.
|
||||||
|
fn mysql_orphans_auto_increment_column(
|
||||||
|
options: &TableStructureSqlOptions,
|
||||||
|
column: &EditableStructureColumn,
|
||||||
|
change: &PrimaryKeyChange<'_>,
|
||||||
|
) -> bool {
|
||||||
|
column.original.is_some()
|
||||||
|
&& column.extra.as_ref().is_some_and(|extra| extra.auto_increment.unwrap_or(false))
|
||||||
|
&& change.old_ids.contains(column.id.as_str())
|
||||||
|
&& !change.new_ids.contains(column.id.as_str())
|
||||||
|
&& !mysql_column_leads_kept_index(options, column)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Whether a persisted, non-primary index keys this column for the whole change.
|
||||||
|
///
|
||||||
|
/// Only an index the draft leaves completely untouched counts. One created by this draft is
|
||||||
|
/// added after the column DDL, and an edited one is rebuilt as DROP + CREATE after it, so
|
||||||
|
/// neither covers the column while the coalesced ALTER runs — and the DROP would hit the very
|
||||||
|
/// same ERROR 1075. Matching is against the persisted names on both sides, so a draft that
|
||||||
|
/// swaps two column names cannot credit one column's index to the other.
|
||||||
|
///
|
||||||
|
/// This applies the InnoDB rule that the auto column must *lead* an index. MyISAM also accepts
|
||||||
|
/// it in a later position of a multi-column index; DBX has no engine information here, so such
|
||||||
|
/// a table loses AUTO_INCREMENT rather than every InnoDB table keeping an invalid one.
|
||||||
|
fn mysql_column_leads_kept_index(options: &TableStructureSqlOptions, column: &EditableStructureColumn) -> bool {
|
||||||
|
let Some(original_name) = column.original.as_ref().map(|original| original.name.as_str()) else {
|
||||||
|
return false;
|
||||||
|
};
|
||||||
|
options.indexes.iter().any(|index| {
|
||||||
|
!index.marked_for_drop
|
||||||
|
&& !has_existing_index_change(index)
|
||||||
|
&& index.original.as_ref().is_some_and(|original| {
|
||||||
|
!original.is_primary
|
||||||
|
&& original.columns.first().is_some_and(|first| clean(first).eq_ignore_ascii_case(original_name))
|
||||||
|
})
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
fn without_auto_increment(column: &EditableStructureColumn) -> EditableStructureColumn {
|
||||||
|
let mut cleared = column.clone();
|
||||||
|
if let Some(extra) = cleared.extra.as_mut() {
|
||||||
|
extra.auto_increment = Some(false);
|
||||||
|
}
|
||||||
|
cleared
|
||||||
|
}
|
||||||
|
|
||||||
pub(super) fn build_primary_key_sql(
|
pub(super) fn build_primary_key_sql(
|
||||||
options: &TableStructureSqlOptions,
|
options: &TableStructureSqlOptions,
|
||||||
dialect: StructureDialect,
|
dialect: StructureDialect,
|
||||||
|
|||||||
@@ -4242,10 +4242,141 @@ fn mysql_coalesces_migration_away_from_existing_auto_increment_primary_key() {
|
|||||||
vec![old_pk, new_pk],
|
vec![old_pk, new_pk],
|
||||||
));
|
));
|
||||||
|
|
||||||
|
assert_eq!(result.warnings, Vec::<String>::new());
|
||||||
|
// The old key column keeps its AUTO_INCREMENT checkbox in the draft, but MySQL refuses an
|
||||||
|
// auto column that no longer leads a key, so the flag is cleared in the same statement.
|
||||||
|
assert_eq!(
|
||||||
|
result.statements,
|
||||||
|
vec![
|
||||||
|
"ALTER TABLE `users` DROP PRIMARY KEY, MODIFY COLUMN `id` bigint NOT NULL, ADD PRIMARY KEY (`external_id`);"
|
||||||
|
]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Regression for #7973: swapping the primary key onto another column left the previous
|
||||||
|
/// AUTO_INCREMENT key column untouched, so the coalesced ALTER failed with
|
||||||
|
/// `ERROR 1075 Incorrect table definition; there can be only one auto column ...`.
|
||||||
|
#[test]
|
||||||
|
fn mysql_clears_auto_increment_on_column_replaced_by_new_auto_increment_primary_key() {
|
||||||
|
let mut old_pk = existing_pk_column("ID", "bigint unsigned", true, false);
|
||||||
|
old_pk.id = "old_id".to_string();
|
||||||
|
old_pk.comment = "自增ID".to_string();
|
||||||
|
old_pk.extra = Some(ColumnExtra { auto_increment: Some(true), ..Default::default() });
|
||||||
|
let old_original = old_pk.original.as_mut().unwrap();
|
||||||
|
old_original.extra = Some("auto_increment".to_string());
|
||||||
|
old_original.comment = Some("自增ID".to_string());
|
||||||
|
|
||||||
|
let mut new_pk = existing_pk_column("ProjectID", "bigint", false, true);
|
||||||
|
new_pk.id = "project_id".to_string();
|
||||||
|
new_pk.comment = "对应project表的主键ID".to_string();
|
||||||
|
new_pk.extra = Some(ColumnExtra { auto_increment: Some(true), ..Default::default() });
|
||||||
|
new_pk.original.as_mut().unwrap().comment = Some("对应project表的主键ID".to_string());
|
||||||
|
|
||||||
|
let result = build_table_structure_change_sql(structure_change_options(
|
||||||
|
DatabaseType::Mysql,
|
||||||
|
None,
|
||||||
|
"issue7973_repro",
|
||||||
|
vec![old_pk, new_pk],
|
||||||
|
));
|
||||||
|
|
||||||
|
assert_eq!(result.warnings, Vec::<String>::new());
|
||||||
|
assert_eq!(
|
||||||
|
result.statements,
|
||||||
|
vec![
|
||||||
|
"ALTER TABLE `issue7973_repro` DROP PRIMARY KEY, MODIFY COLUMN `ID` bigint unsigned NOT NULL COMMENT '自增ID', MODIFY COLUMN `ProjectID` bigint NOT NULL AUTO_INCREMENT COMMENT '对应project表的主键ID', ADD PRIMARY KEY (`ProjectID`);"
|
||||||
|
]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A surviving secondary index still keys the column, so AUTO_INCREMENT stays legal there
|
||||||
|
/// and must not be stripped just because the primary key moved elsewhere.
|
||||||
|
#[test]
|
||||||
|
fn mysql_keeps_auto_increment_when_a_kept_index_still_leads_with_the_column() {
|
||||||
|
let mut old_pk = existing_pk_column("id", "bigint", true, false);
|
||||||
|
old_pk.id = "old_id".to_string();
|
||||||
|
old_pk.extra = Some(ColumnExtra { auto_increment: Some(true), ..Default::default() });
|
||||||
|
old_pk.original.as_mut().unwrap().extra = Some("auto_increment".to_string());
|
||||||
|
|
||||||
|
let mut new_pk = existing_pk_column("external_id", "varchar(64)", false, true);
|
||||||
|
new_pk.id = "new_external_id".to_string();
|
||||||
|
|
||||||
|
let mut options = structure_change_options(DatabaseType::Mysql, None, "users", vec![old_pk, new_pk]);
|
||||||
|
options.indexes = vec![existing_index("idx_users_id", &["id"], false)];
|
||||||
|
|
||||||
|
let result = build_table_structure_change_sql(options);
|
||||||
|
|
||||||
assert_eq!(result.warnings, Vec::<String>::new());
|
assert_eq!(result.warnings, Vec::<String>::new());
|
||||||
assert_eq!(result.statements, vec!["ALTER TABLE `users` DROP PRIMARY KEY, ADD PRIMARY KEY (`external_id`);"]);
|
assert_eq!(result.statements, vec!["ALTER TABLE `users` DROP PRIMARY KEY, ADD PRIMARY KEY (`external_id`);"]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// An index the same draft edits is rebuilt as DROP + CREATE *after* the column DDL, so it
|
||||||
|
/// cannot excuse the AUTO_INCREMENT flag: the DROP INDEX would hit ERROR 1075 itself.
|
||||||
|
#[test]
|
||||||
|
fn mysql_clears_auto_increment_when_the_only_covering_index_is_rebuilt_by_the_same_draft() {
|
||||||
|
let mut old_pk = existing_pk_column("id", "bigint", true, false);
|
||||||
|
old_pk.id = "old_id".to_string();
|
||||||
|
old_pk.extra = Some(ColumnExtra { auto_increment: Some(true), ..Default::default() });
|
||||||
|
old_pk.original.as_mut().unwrap().extra = Some("auto_increment".to_string());
|
||||||
|
|
||||||
|
let mut new_pk = existing_pk_column("external_id", "varchar(64)", false, true);
|
||||||
|
new_pk.id = "new_external_id".to_string();
|
||||||
|
|
||||||
|
let mut rebuilt_index = existing_index("idx_users_id", &["id"], false);
|
||||||
|
rebuilt_index.is_unique = true;
|
||||||
|
|
||||||
|
let mut options = structure_change_options(DatabaseType::Mysql, None, "users", vec![old_pk, new_pk]);
|
||||||
|
options.indexes = vec![rebuilt_index];
|
||||||
|
|
||||||
|
let result = build_table_structure_change_sql(options);
|
||||||
|
|
||||||
|
assert_eq!(result.warnings, Vec::<String>::new());
|
||||||
|
assert_eq!(
|
||||||
|
result.statements,
|
||||||
|
vec![
|
||||||
|
"ALTER TABLE `users` DROP PRIMARY KEY, MODIFY COLUMN `id` bigint NOT NULL, ADD PRIMARY KEY (`external_id`);",
|
||||||
|
"DROP INDEX `idx_users_id` ON `users`;",
|
||||||
|
"CREATE UNIQUE INDEX `idx_users_id` ON `users` (`id`);",
|
||||||
|
]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The covering index is matched by persisted names on both sides, so a draft that swaps two
|
||||||
|
/// column names cannot credit one column's index to the other.
|
||||||
|
#[test]
|
||||||
|
fn mysql_index_cover_does_not_follow_a_column_name_swap() {
|
||||||
|
let mut old_pk = existing_pk_column("id", "bigint", true, false);
|
||||||
|
old_pk.id = "old_id".to_string();
|
||||||
|
old_pk.name = "legacy_id".to_string();
|
||||||
|
old_pk.extra = Some(ColumnExtra { auto_increment: Some(true), ..Default::default() });
|
||||||
|
old_pk.original.as_mut().unwrap().extra = Some("auto_increment".to_string());
|
||||||
|
|
||||||
|
// Takes over the name the surviving index was built on, but not the index itself.
|
||||||
|
let mut renamed = existing_pk_column("tenant_id", "bigint", false, false);
|
||||||
|
renamed.id = "tenant".to_string();
|
||||||
|
renamed.name = "id".to_string();
|
||||||
|
|
||||||
|
let mut new_pk = existing_pk_column("external_id", "varchar(64)", false, true);
|
||||||
|
new_pk.id = "new_external_id".to_string();
|
||||||
|
|
||||||
|
let mut options = structure_change_options(DatabaseType::Mysql, None, "users", vec![old_pk, renamed, new_pk]);
|
||||||
|
options.indexes = vec![existing_index("idx_users_tenant_id", &["tenant_id"], false)];
|
||||||
|
|
||||||
|
let result = build_table_structure_change_sql(options);
|
||||||
|
|
||||||
|
assert_eq!(result.warnings, Vec::<String>::new());
|
||||||
|
// Only the second statement is what this test is about: the auto column lost AUTO_INCREMENT
|
||||||
|
// even though `idx_users_tenant_id` leads with a column *named* `id` in the draft. The
|
||||||
|
// separate rename statement, and the order the two renames run in, are pre-existing
|
||||||
|
// name-swap behavior unrelated to this fix.
|
||||||
|
assert_eq!(
|
||||||
|
result.statements,
|
||||||
|
vec![
|
||||||
|
"ALTER TABLE `users` CHANGE COLUMN `tenant_id` `id` bigint NOT NULL;",
|
||||||
|
"ALTER TABLE `users` DROP PRIMARY KEY, CHANGE COLUMN `id` `legacy_id` bigint NOT NULL, ADD PRIMARY KEY (`external_id`);",
|
||||||
|
]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn mysql_coalesces_renamed_auto_increment_primary_key_column() {
|
fn mysql_coalesces_renamed_auto_increment_primary_key_column() {
|
||||||
let mut old_pk = existing_pk_column("campaign_rel_id", "bigint", true, false);
|
let mut old_pk = existing_pk_column("campaign_rel_id", "bigint", true, false);
|
||||||
|
|||||||
Reference in new issue
Block a user