fix(mysql): show inherited charset and collation in structure editor
This commit is contained in:
1 parent
48410b70b4
commit
06dcd0013c
16 files changed
+562
-91
No files matched your search
@@ -282,7 +282,7 @@ fn nonblank(value: String) -> Option<String> {
|
||||
}
|
||||
|
||||
async fn query_first_nonblank_string(conn: &mut mysql_async::Conn, sql: &str) -> Option<String> {
|
||||
// MySQL reports nullable metadata such as TABLE_COLLATION as NULL for views.
|
||||
// MySQL reports nullable metadata such as `@@version_comment` as NULL.
|
||||
// Reading it as String makes mysql_async panic during row conversion.
|
||||
match query_first_column::<String>(conn, sql).await {
|
||||
Ok(value) => value.and_then(nonblank),
|
||||
@@ -3697,11 +3697,9 @@ pub async fn list_completion_objects(pool: &MySqlPool, database: &str) -> Result
|
||||
}
|
||||
|
||||
fn columns_sql(database: &str, table: &str) -> String {
|
||||
// Query only information_schema.COLUMNS and fetch TABLE_COLLATION separately via
|
||||
// `table_collation_sql`. A LEFT JOIN onto information_schema.TABLES triggers a
|
||||
// catastrophic plan on MySQL 5.7 (observed ~8s vs ~1ms without the join), because 5.7's
|
||||
// TABLES metadata is materialized per-query with poor predicate pushdown. The separate
|
||||
// lookup matches the `get_columns_show` fallback path and keeps results identical.
|
||||
// Query only information_schema.COLUMNS. A LEFT JOIN onto information_schema.TABLES
|
||||
// triggers a catastrophic plan on MySQL 5.7 (observed ~8s vs ~1ms without the join),
|
||||
// because 5.7's TABLES metadata is materialized per-query with poor predicate pushdown.
|
||||
format!(
|
||||
"SELECT COLUMN_NAME, DATA_TYPE, COLUMN_TYPE, IS_NULLABLE, COLUMN_DEFAULT, EXTRA, \
|
||||
COLUMN_COMMENT, COLUMN_KEY, NUMERIC_PRECISION, NUMERIC_SCALE, CHARACTER_MAXIMUM_LENGTH, \
|
||||
@@ -3802,28 +3800,6 @@ fn apply_mysql_generated_column_expressions(columns: &mut [ColumnInfo], expressi
|
||||
}
|
||||
}
|
||||
|
||||
fn table_collation_sql(database: &str, table: &str) -> String {
|
||||
format!(
|
||||
"SELECT TABLE_COLLATION FROM information_schema.TABLES WHERE TABLE_SCHEMA = {} AND TABLE_NAME = {} LIMIT 1",
|
||||
quote_value(database),
|
||||
quote_value(table),
|
||||
)
|
||||
}
|
||||
|
||||
fn normalize_mysql_column_charset_metadata(columns: &mut [ColumnInfo], table_collation: Option<&str>) {
|
||||
let Some(table_collation) = table_collation.filter(|value| !value.trim().is_empty()) else {
|
||||
return;
|
||||
};
|
||||
for column in columns {
|
||||
if column.collation.as_deref().is_some_and(|value| value.eq_ignore_ascii_case(table_collation)) {
|
||||
// MySQL reports effective values and does not preserve whether an
|
||||
// equivalent table-default collation was explicitly written.
|
||||
column.character_set = None;
|
||||
column.collation = None;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Attempt to reverse CP1252→UTF-8 double-encoding.
|
||||
///
|
||||
/// When Chinese text is written to MySQL through a connection with the wrong
|
||||
@@ -3964,14 +3940,6 @@ where
|
||||
};
|
||||
let rows: Vec<mysql_async::Row> = result.collect_and_drop().await.map_err(|e| e.to_string())?;
|
||||
|
||||
// When database is empty the COLUMNS query returns no rows, so the
|
||||
// function falls through to get_columns_show and this code path is
|
||||
// never reached. Skip the collation lookup to avoid a pointless query.
|
||||
let table_collation = if database.trim().is_empty() {
|
||||
None
|
||||
} else {
|
||||
query_first_nonblank_string(&mut conn, &table_collation_sql(database, table)).await
|
||||
};
|
||||
let mut columns: Vec<ColumnInfo> = rows
|
||||
.iter()
|
||||
.filter_map(|row| {
|
||||
@@ -4019,7 +3987,6 @@ where
|
||||
}
|
||||
|
||||
enrich_mysql_generated_column_expressions(&mut conn, database, table, &mut columns).await;
|
||||
normalize_mysql_column_charset_metadata(&mut columns, table_collation.as_deref());
|
||||
Ok(columns)
|
||||
}
|
||||
|
||||
@@ -4037,11 +4004,6 @@ where
|
||||
result.collect_and_drop().await.map_err(|e| e.to_string())?
|
||||
}
|
||||
};
|
||||
let table_collation = if database.trim().is_empty() {
|
||||
None
|
||||
} else {
|
||||
query_first_nonblank_string(&mut conn, &table_collation_sql(database, table)).await
|
||||
};
|
||||
let mut columns: Vec<ColumnInfo> = rows
|
||||
.iter()
|
||||
.filter_map(|row| {
|
||||
@@ -4076,7 +4038,6 @@ where
|
||||
})
|
||||
.collect();
|
||||
enrich_mysql_generated_column_expressions(&mut conn, database, table, &mut columns).await;
|
||||
normalize_mysql_column_charset_metadata(&mut columns, table_collation.as_deref());
|
||||
Ok(columns)
|
||||
}
|
||||
|
||||
@@ -7106,12 +7067,12 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mysql_columns_sql_uses_column_key_and_table_default_collation() {
|
||||
fn mysql_columns_sql_uses_column_key_for_primary_keys_without_join() {
|
||||
let sql = columns_sql("app", "users");
|
||||
|
||||
assert!(sql.contains("information_schema.COLUMNS"));
|
||||
// TABLE_COLLATION is fetched separately via `table_collation_sql`; the LEFT JOIN onto
|
||||
// information_schema.TABLES is intentionally avoided to keep MySQL 5.7 fast.
|
||||
// The LEFT JOIN onto information_schema.TABLES is intentionally avoided to keep
|
||||
// MySQL 5.7 fast.
|
||||
assert!(!sql.contains("information_schema.TABLES"));
|
||||
assert!(!sql.contains("TABLE_COLLATION"));
|
||||
assert!(!sql.contains("KEY_COLUMN_USAGE"));
|
||||
@@ -7175,54 +7136,13 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mysql_nullable_table_collation_uses_optional_string_conversion() {
|
||||
fn mysql_nullable_metadata_uses_optional_string_conversion() {
|
||||
let collation = mysql_async::from_value_opt::<Option<String>>(mysql_async::Value::NULL)
|
||||
.expect("Option<String> must accept NULL MySQL metadata values");
|
||||
|
||||
assert_eq!(collation, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mysql_column_charset_metadata_clears_values_matching_table_default() {
|
||||
let mut columns = vec![
|
||||
ColumnInfo {
|
||||
name: "inherited_name".to_string(),
|
||||
character_set: Some("utf8mb4".to_string()),
|
||||
collation: Some("utf8mb4_unicode_ci".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
ColumnInfo {
|
||||
name: "explicit_other".to_string(),
|
||||
character_set: Some("latin1".to_string()),
|
||||
collation: Some("latin1_bin".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
ColumnInfo { name: "numeric_value".to_string(), ..Default::default() },
|
||||
];
|
||||
|
||||
normalize_mysql_column_charset_metadata(&mut columns, Some("utf8mb4_unicode_ci"));
|
||||
|
||||
assert_eq!((columns[0].character_set.as_deref(), columns[0].collation.as_deref()), (None, None));
|
||||
assert_eq!(columns[1].character_set.as_deref(), Some("latin1"));
|
||||
assert_eq!(columns[1].collation.as_deref(), Some("latin1_bin"));
|
||||
assert_eq!((columns[2].character_set.as_deref(), columns[2].collation.as_deref()), (None, None));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mysql_column_charset_metadata_preserves_values_without_table_default() {
|
||||
let mut columns = vec![ColumnInfo {
|
||||
name: "name".to_string(),
|
||||
character_set: Some("utf8mb4".to_string()),
|
||||
collation: Some("utf8mb4_unicode_ci".to_string()),
|
||||
..Default::default()
|
||||
}];
|
||||
|
||||
normalize_mysql_column_charset_metadata(&mut columns, None);
|
||||
|
||||
assert_eq!(columns[0].character_set.as_deref(), Some("utf8mb4"));
|
||||
assert_eq!(columns[0].collation.as_deref(), Some("utf8mb4_unicode_ci"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_mysql_enum_values_preserves_mysql_literal_edges() {
|
||||
assert_eq!(
|
||||
|
||||
@@ -2403,6 +2403,7 @@ mod tests {
|
||||
mysql_engine: None,
|
||||
partitioned: false,
|
||||
is_gaussdb_m_mode: false,
|
||||
table_collation: None,
|
||||
});
|
||||
assert_eq!(create.warnings, Vec::<String>::new());
|
||||
assert_eq!(
|
||||
|
||||
@@ -31,6 +31,7 @@ pub(crate) use util::{oracle_new_object_reference, sqlserver_unicode_string_lite
|
||||
|
||||
use crate::models::connection::DatabaseType;
|
||||
|
||||
use column_format::strip_inherited_mysql_column_charsets;
|
||||
use columns::{build_column_sql, validate_primary_key_change_scope};
|
||||
use comments::build_table_comment_sql;
|
||||
use foreign_keys::build_foreign_key_sql;
|
||||
@@ -49,6 +50,7 @@ pub fn build_table_structure_change_sql(mut options: TableStructureSqlOptions) -
|
||||
if options.is_gaussdb_m_mode {
|
||||
options.database_type = Some(DatabaseType::Mysql);
|
||||
}
|
||||
strip_inherited_mysql_column_charsets(&mut options);
|
||||
let primary_key_errors = validate_primary_key_change_scope(&options);
|
||||
if !primary_key_errors.is_empty() {
|
||||
return TableStructureSqlResult { statements: Vec::new(), warnings: primary_key_errors };
|
||||
|
||||
@@ -1,7 +1,41 @@
|
||||
use super::dialect::{is_oracle_like, StructureDialect};
|
||||
use super::types::EditableStructureColumn;
|
||||
use super::types::{EditableStructureColumn, TableStructureSqlOptions};
|
||||
use super::util::{clean, format_default_for_sql, normalize_default, quote_ident, quote_string};
|
||||
|
||||
/// Drops the `CHARACTER SET` / `COLLATE` inputs of MySQL columns that merely
|
||||
/// inherit the table's default collation, so the generated DDL does not spell
|
||||
/// out clauses the server would apply anyway.
|
||||
///
|
||||
/// MySQL reports the *effective* collation of every character column and never
|
||||
/// records whether it was written out explicitly, so "equals the table default"
|
||||
/// is the only signal available. Omitting the clauses is equivalent to keeping
|
||||
/// them: a column definition without `CHARACTER SET` takes the table default,
|
||||
/// which is exactly the value being dropped here. This runs on the DDL inputs
|
||||
/// only — introspection (`get_columns`) still reports the real values so the
|
||||
/// structure editor can show the column's current charset and collation.
|
||||
///
|
||||
/// The original snapshot is normalized alongside the draft so that an untouched
|
||||
/// column does not register as a charset change and trigger a needless `MODIFY`.
|
||||
pub(super) fn strip_inherited_mysql_column_charsets(options: &mut TableStructureSqlOptions) {
|
||||
let Some(table_collation) = options.table_collation.as_deref().map(str::trim).filter(|value| !value.is_empty())
|
||||
else {
|
||||
return;
|
||||
};
|
||||
let inherits = |collation: &str| collation.trim().eq_ignore_ascii_case(table_collation);
|
||||
for column in &mut options.columns {
|
||||
if inherits(&column.collation) {
|
||||
column.character_set = String::new();
|
||||
column.collation = String::new();
|
||||
}
|
||||
if let Some(original) = column.original.as_mut() {
|
||||
if original.collation.as_deref().is_some_and(inherits) {
|
||||
original.character_set = None;
|
||||
original.collation = None;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn column_definition(dialect: StructureDialect, column: &EditableStructureColumn) -> String {
|
||||
let data_type = column_data_type(dialect, column);
|
||||
let mut parts = vec![quote_ident(dialect, &column.name), data_type];
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
use super::column_format::{
|
||||
column_data_type, column_extra_clause, has_dameng_identity, is_dameng_identity_compatible_type,
|
||||
is_mysql_character_data_type, is_mysql_timestamp_type,
|
||||
is_mysql_character_data_type, is_mysql_timestamp_type, strip_inherited_mysql_column_charsets,
|
||||
};
|
||||
use super::comments::{build_sqlserver_column_comment_sql, build_sqlserver_table_comment_sql};
|
||||
use super::dialect::{capabilities_for, database_label, StructureDialect};
|
||||
@@ -26,6 +26,7 @@ pub fn build_create_table_sql(mut options: TableStructureSqlOptions) -> TableStr
|
||||
dialect = capabilities.dialect;
|
||||
}
|
||||
options.table_name = clean(&options.table_name);
|
||||
strip_inherited_mysql_column_charsets(&mut options);
|
||||
let mut warnings = Vec::new();
|
||||
warnings.extend(validate_mysql_engine(&options));
|
||||
// Fail closed: a concurrent-index request on a partitioned parent (or on an
|
||||
|
||||
@@ -1519,6 +1519,7 @@ mod tests {
|
||||
mysql_engine: None,
|
||||
partitioned: false,
|
||||
is_gaussdb_m_mode: false,
|
||||
table_collation: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
File diff suppressed because it is too large.
Load diff
@@ -216,6 +216,14 @@ pub struct TableStructureSqlOptions {
|
||||
pub original_table_comment: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub mysql_engine: Option<String>,
|
||||
/// MySQL only: the table's current default collation
|
||||
/// (`information_schema.TABLES.TABLE_COLLATION`). A column whose collation
|
||||
/// merely matches it inherits the table default, so its `CHARACTER SET` /
|
||||
/// `COLLATE` clauses are redundant and are dropped from the generated DDL.
|
||||
/// Introspection keeps reporting the column's real values, which is what
|
||||
/// the structure editor renders in its charset/collation pickers.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub table_collation: Option<String>,
|
||||
/// Whether the target table is a partitioned parent table (PostgreSQL
|
||||
/// `relkind = 'p'`). PostgreSQL rejects `CREATE INDEX CONCURRENTLY` on
|
||||
/// partitioned parents, so the builder refuses such a request up front
|
||||
|
||||
@@ -49,6 +49,7 @@ fn change_options(table_name: &str, columns: Vec<EditableStructureColumn>) -> Ta
|
||||
mysql_engine: None,
|
||||
partitioned: false,
|
||||
is_gaussdb_m_mode: false,
|
||||
table_collation: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -62,6 +62,7 @@ async fn live_postgres_concurrent_index_builds_valid_index() {
|
||||
mysql_engine: None,
|
||||
partitioned: false,
|
||||
is_gaussdb_m_mode: false,
|
||||
table_collation: None,
|
||||
});
|
||||
assert!(result.warnings.is_empty(), "{:?}", result.warnings);
|
||||
assert_eq!(
|
||||
@@ -193,6 +194,7 @@ async fn live_postgres_partitioned_parent_concurrent_request_rejected() {
|
||||
mysql_engine: None,
|
||||
partitioned: true,
|
||||
is_gaussdb_m_mode: false,
|
||||
table_collation: None,
|
||||
});
|
||||
assert_eq!(
|
||||
result.warnings,
|
||||
|
||||
@@ -1010,6 +1010,7 @@ async fn live_sqlserver_table_structure_default_changes_drop_existing_constraint
|
||||
mysql_engine: None,
|
||||
partitioned: false,
|
||||
is_gaussdb_m_mode: false,
|
||||
table_collation: None,
|
||||
});
|
||||
assert_eq!(result.warnings, Vec::<String>::new());
|
||||
assert_eq!(result.statements.len(), 4);
|
||||
|
||||
Reference in new issue
Block a user