perf(structure): minimize column reorder SQL generation

This commit is contained in:
ptma 2026-07-01 14:38:34 +08:00 committed by GitHub
parent e910a578f6
commit 889d0af92c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 171 additions and 20 deletions

View File

@ -12,6 +12,7 @@ use super::util::{
clean, is_protected_manticore_id_column, normalize_default, original_comment, original_default, qualified_table,
quote_ident, quote_string,
};
use std::collections::HashSet;
pub(super) fn build_column_sql(options: &TableStructureSqlOptions, warnings: &mut Vec<String>) -> Vec<String> {
let capabilities = capabilities_for(options.database_type);
@ -22,6 +23,15 @@ pub(super) fn build_column_sql(options: &TableStructureSqlOptions, warnings: &mu
let has_original_column_positions = active_columns.iter().any(|column| column.original_position.is_some());
let mut simulated_column_order =
if has_original_column_positions { original_active_column_order(&active_columns) } else { Vec::new() };
// Pre-compute the minimal set of existing columns that really need an explicit move.
// For MySQL/ClickHouse we keep the largest already-ordered subset in place and only
// emit FIRST/AFTER SQL for columns outside that subset.
let reordered_existing_column_ids =
if has_original_column_positions && matches!(dialect, StructureDialect::Mysql | StructureDialect::ClickHouse) {
planned_existing_column_move_ids(&active_columns)
} else {
HashSet::new()
};
let mut statements = Vec::new();
for column in &options.columns {
@ -52,8 +62,11 @@ pub(super) fn build_column_sql(options: &TableStructureSqlOptions, warnings: &mu
String::new()
};
let desired_previous_column_id = active_previous_column_id(&active_columns, active_index);
// A position change only matters when this column is part of the planned move set
// and its predecessor still differs in the simulated order.
let has_position_change = has_original_column_positions
&& matches!(dialect, StructureDialect::Mysql | StructureDialect::ClickHouse)
&& reordered_existing_column_ids.contains(&column.id)
&& column.original.is_some()
&& column.original_position.is_some()
&& simulated_column_position_changed(&simulated_column_order, &column.id, desired_previous_column_id);
@ -314,6 +327,79 @@ pub(super) fn original_active_column_order(columns: &[&EditableStructureColumn])
original_columns.into_iter().map(|column| column.id.clone()).collect()
}
/// Returns the ids of existing columns that must be explicitly moved to reach the target order.
///
/// The function keeps the longest subsequence of existing columns whose relative order is already
/// correct, and marks only the remaining columns for FIRST/AFTER reordering SQL.
pub(super) fn planned_existing_column_move_ids(columns: &[&EditableStructureColumn]) -> HashSet<String> {
// Only existing columns with an original position participate in move planning.
// Newly added columns are positioned directly from the target order.
let reorderable_columns: Vec<_> = columns
.iter()
.filter_map(|column| {
column
.original
.as_ref()
.zip(column.original_position)
.map(|_| (column.id.as_str(), column.original_position.unwrap_or(0)))
})
.collect();
if reorderable_columns.len() < 2 {
return HashSet::new();
}
// Map the target order back to original positions, then keep the largest increasing subsequence.
let original_positions: Vec<_> = reorderable_columns.iter().map(|(_, position)| *position).collect();
// Columns inside the LIS can stay where they are; everything else needs an explicit move.
let kept_indices: HashSet<_> = longest_increasing_subsequence_indices(&original_positions).into_iter().collect();
reorderable_columns
.into_iter()
.enumerate()
.filter(|(index, _)| !kept_indices.contains(index))
.map(|(_, (column_id, _))| column_id.to_string())
.collect()
}
/// Returns the indices of one longest increasing subsequence within `values`.
///
/// In the reorder planner, an increasing subsequence represents existing columns whose relative
/// order still matches the original table layout, so they can remain untouched.
fn longest_increasing_subsequence_indices(values: &[usize]) -> Vec<usize> {
if values.is_empty() {
return Vec::new();
}
// O(n^2) is sufficient here because table editors deal with relatively small column counts
// and the simpler implementation is easier to maintain.
let mut lengths = vec![1; values.len()];
let mut previous = vec![None; values.len()];
let mut best_end_index = 0;
for current_index in 0..values.len() {
for previous_index in 0..current_index {
if values[previous_index] < values[current_index] && lengths[previous_index] + 1 > lengths[current_index] {
lengths[current_index] = lengths[previous_index] + 1;
previous[current_index] = Some(previous_index);
}
}
if lengths[current_index] > lengths[best_end_index] {
best_end_index = current_index;
}
}
// Reconstruct the subsequence by following the predecessor chain backwards.
let mut indices = Vec::new();
let mut cursor = Some(best_end_index);
while let Some(index) = cursor {
indices.push(index);
cursor = previous[index];
}
indices.reverse();
indices
}
pub(super) fn active_previous_column_id<'a>(columns: &[&'a EditableStructureColumn], index: usize) -> Option<&'a str> {
if index == 0 {
None

View File

@ -107,14 +107,14 @@ fn builds_mysql_column_and_index_changes() {
assert_eq!(result.warnings, Vec::<String>::new());
assert_eq!(
result.statements,
vec![
"ALTER TABLE `users` CHANGE COLUMN `name` `display_name` varchar(120) NOT NULL DEFAULT 'guest' COMMENT 'Shown name';",
"ALTER TABLE `users` ADD COLUMN `email` varchar(255) NOT NULL;",
"DROP INDEX `idx_old` ON `users`;",
"CREATE UNIQUE INDEX `uniq_users_email` ON `users` (`email`);",
]
);
result.statements,
vec![
"ALTER TABLE `users` CHANGE COLUMN `name` `display_name` varchar(120) NOT NULL DEFAULT 'guest' COMMENT 'Shown name';",
"ALTER TABLE `users` ADD COLUMN `email` varchar(255) NOT NULL;",
"DROP INDEX `idx_old` ON `users`;",
"CREATE UNIQUE INDEX `uniq_users_email` ON `users` (`email`);",
]
);
}
#[test]
@ -490,7 +490,7 @@ fn gbase8a_allows_mysql_style_column_reorder() {
});
assert_eq!(result.warnings, Vec::<String>::new());
assert_eq!(result.statements, vec!["ALTER TABLE `users` MODIFY COLUMN `email` varchar(255) AFTER `id`;"]);
assert_eq!(result.statements, vec!["ALTER TABLE `users` MODIFY COLUMN `name` varchar(255) AFTER `email`;"]);
}
#[test]
@ -935,10 +935,7 @@ fn builds_mysql_column_reorder_statements() {
assert_eq!(result.warnings, Vec::<String>::new());
assert_eq!(
result.statements,
vec![
"ALTER TABLE `users` MODIFY COLUMN `email` varchar(255) AFTER `id`;",
"ALTER TABLE `users` CHANGE COLUMN `name` `display_name` varchar(120);",
]
vec!["ALTER TABLE `users` CHANGE COLUMN `name` `display_name` varchar(120) AFTER `email`;"]
);
}
@ -1044,7 +1041,75 @@ fn mysql_existing_column_reorder_does_not_reorder_columns_shifted_by_prior_move(
});
assert_eq!(result.warnings, Vec::<String>::new());
assert_eq!(result.statements, vec!["ALTER TABLE `users` MODIFY COLUMN `email` varchar(255) AFTER `id`;"]);
assert_eq!(result.statements, vec!["ALTER TABLE `users` MODIFY COLUMN `name` varchar(255) AFTER `email`;"]);
}
#[test]
fn mysql_moving_first_column_to_end_uses_single_reorder_statement() {
let mut col_0 = column("col_0");
col_0.data_type = "int(11)".to_string();
col_0.is_nullable = false;
col_0.original_position = Some(0);
col_0.original = Some(ColumnInfo {
name: "col_0".to_string(),
data_type: "int(11)".to_string(),
is_nullable: false,
column_default: None,
is_primary_key: false,
extra: None,
comment: None,
});
let mut col_1 = column("col_1");
col_1.original_position = Some(1);
col_1.original = Some(ColumnInfo {
name: "col_1".to_string(),
data_type: "varchar(255)".to_string(),
is_nullable: true,
column_default: None,
is_primary_key: false,
extra: None,
comment: None,
});
let mut col_2 = column("col_2");
col_2.original_position = Some(2);
col_2.original = Some(ColumnInfo {
name: "col_2".to_string(),
data_type: "varchar(255)".to_string(),
is_nullable: true,
column_default: None,
is_primary_key: false,
extra: None,
comment: None,
});
let mut col_3 = column("col_3");
col_3.original_position = Some(3);
col_3.original = Some(ColumnInfo {
name: "col_3".to_string(),
data_type: "varchar(255)".to_string(),
is_nullable: true,
column_default: None,
is_primary_key: false,
extra: None,
comment: None,
});
let result = build_table_structure_change_sql(TableStructureSqlOptions {
database_type: Some(DatabaseType::Mysql),
schema: None,
table_name: "users".to_string(),
columns: vec![col_1, col_2, col_3, col_0],
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!["ALTER TABLE `users` MODIFY COLUMN `col_0` int(11) NOT NULL AFTER `col_3`;"]);
}
#[test]
@ -1381,12 +1446,12 @@ fn builds_duckdb_create_table_statements() {
assert_eq!(result.warnings, Vec::<String>::new());
assert_eq!(
result.statements,
vec![
"CREATE TABLE \"events\" (\n \"name\" VARCHAR NOT NULL,\n \"created_at\" TIMESTAMP DEFAULT current_timestamp\n);",
"CREATE INDEX \"idx_events_name\" ON \"events\" (\"name\");",
]
);
result.statements,
vec![
"CREATE TABLE \"events\" (\n \"name\" VARCHAR NOT NULL,\n \"created_at\" TIMESTAMP DEFAULT current_timestamp\n);",
"CREATE INDEX \"idx_events_name\" ON \"events\" (\"name\");",
]
);
}
#[test]