diff --git a/crates/dbx-core/src/data/table_import.rs b/crates/dbx-core/src/data/table_import.rs index 2d4b86bd10..ae1893ece2 100644 --- a/crates/dbx-core/src/data/table_import.rs +++ b/crates/dbx-core/src/data/table_import.rs @@ -5069,7 +5069,7 @@ fn normalize_import_value_cow<'a>( // Strip validated thousands separators before integer canonicalization so "1,234.00" // still collapses to a plain integer literal for integer targets. let thousands_canonical = - normalized.as_str().and_then(|value| normalize_thousands_numeric_literal(value, db_type, data_type)); + normalized.as_str().and_then(|value| normalize_thousands_numeric_literal(value, data_type)); if let Some(integer_text) = thousands_canonical .as_deref() .or_else(|| normalized.as_str()) @@ -11793,6 +11793,14 @@ mod tests { fn postgres_import_batches( rows: Vec>, target_types: &[(&str, &str)], + ) -> Vec { + import_batches_for(rows, target_types, &DatabaseType::Postgres) + } + + fn import_batches_for( + rows: Vec>, + target_types: &[(&str, &str)], + db_type: &DatabaseType, ) -> Vec { let data = ParsedImportFile { columns: target_types.iter().map(|(column, _)| column.to_string()).collect(), @@ -11812,16 +11820,7 @@ mod tests { .iter() .map(|(column, data_type)| (column.to_string(), data_type.to_string())) .collect::>(); - build_import_insert_batches( - &data, - &mappings, - &target_column_types, - "issue_6491", - "", - &DatabaseType::Postgres, - 500, - ) - .unwrap() + build_import_insert_batches(&data, &mappings, &target_column_types, "issue_6491", "", db_type, 500).unwrap() } #[test] @@ -11847,6 +11846,59 @@ mod tests { } } + #[test] + fn mysql_import_converts_valid_thousands_separators_for_numeric_targets() { + // #11993: MySQL/TiDB imports previously kept "123,456" as text, so a + // numeric target column rejected the row. + for (value, data_type, expected) in [ + ("123,456", "bigint", "123456"), + ("1,234.56", "decimal(18,2)", "1234.56"), + ("-1,234.56", "decimal(18,2)", "-1234.56"), + ("1,234,567,890", "bigint", "1234567890"), + ("1,234.5", "double", "1234.5"), + ] { + let batches = import_batches_for( + vec![vec![serde_json::json!(value)]], + &[("amount", data_type)], + &DatabaseType::Mysql, + ); + assert_eq!( + batches[0].sql, + format!("INSERT INTO `issue_6491` (`amount`) VALUES\n({expected})"), + "{value} -> {data_type}" + ); + } + } + + #[test] + fn mysql_import_preserves_thousands_separators_for_text_targets() { + // A text column must keep the literal characters the user wrote. + for data_type in ["varchar(64)", "text"] { + let batches = import_batches_for( + vec![vec![serde_json::json!("123,456")]], + &[("amount", data_type)], + &DatabaseType::Mysql, + ); + assert_eq!( + batches[0].sql, + format!("INSERT INTO `issue_6491` (`amount`) VALUES\n('123,456')"), + "{data_type}" + ); + } + } + + #[test] + fn mysql_import_keeps_malformed_grouping_untouched() { + for value in ["1,23,4", "12,34.56", "1,,234", ",123", "123,", "1,234,", "1,234.5.6", "abc,123", "1,234abc"] { + let batches = import_batches_for( + vec![vec![serde_json::json!(value)]], + &[("amount", "decimal(18,2)")], + &DatabaseType::Mysql, + ); + assert_eq!(batches[0].sql, format!("INSERT INTO `issue_6491` (`amount`) VALUES\n('{value}')"), "{value}"); + } + } + #[test] fn postgres_import_preserves_thousands_separators_for_text_targets() { for data_type in ["varchar(64)", "text"] { @@ -11939,6 +11991,45 @@ mod tests { let _ = std::fs::remove_file(path); } + #[test] + fn excel_text_cell_with_thousands_separator_imports_to_mysql_numeric() { + // #11993: the same spreadsheet into a MySQL/TiDB numeric column used to be + // inserted as the literal string "123,456". + let path = std::env::temp_dir().join(format!("dbx-table-import-11993-{}.xlsx", uuid::Uuid::new_v4())); + let sheet_xml = r#" + + + + amount + 123,456 + 1,234.56 + +"#; + std::fs::write(&path, build_preview_test_xlsx(sheet_xml, None)).unwrap(); + let options = TableImportParseOptions::default(); + + let data = parse_xlsx_file_with_options(&path.to_string_lossy(), &options, 10).unwrap(); + assert_eq!(data.rows, vec![vec![serde_json::json!("123,456")], vec![serde_json::json!("1,234.56")]]); + let mappings = vec![TableImportColumnMapping { + source_column: "amount".to_string(), + target_column: "amount".to_string(), + target_data_type: None, + }]; + let batches = build_import_insert_batches( + &data, + &mappings, + &[("amount".to_string(), "decimal(18,2)".to_string())], + "issue_11993", + "", + &DatabaseType::Mysql, + 500, + ) + .unwrap(); + + assert_eq!(batches[0].sql, "INSERT INTO `issue_11993` (`amount`) VALUES\n(123456),\n(1234.56)"); + let _ = std::fs::remove_file(path); + } + #[test] fn csv_thousands_separator_uses_same_numeric_normalization() { let parsed = parse_csv_bytes(b"amount\n\"1,234.56\"\n\"12,345\"\n", 10).unwrap(); diff --git a/crates/dbx-core/src/data/transfer.rs b/crates/dbx-core/src/data/transfer.rs index 46a1a6de64..7e78c38f5c 100644 --- a/crates/dbx-core/src/data/transfer.rs +++ b/crates/dbx-core/src/data/transfer.rs @@ -1711,7 +1711,11 @@ pub(crate) fn normalize_integer_literal( Some(integer.to_string()) } -fn is_postgres_numeric_family(data_type: &str) -> bool { +/// Target types whose stored value is a plain number, so an en-US thousands +/// separator in the source text is formatting rather than data. Covers the +/// PostgreSQL/Oracle spellings and the MySQL-family ones (`int`, `tinyint`, +/// `mediumint`); a type not listed here keeps its literal text. +fn is_numeric_target_type(data_type: &str) -> bool { let normalized = data_type.trim().to_ascii_lowercase(); let base = normalized.split(['(', ' ']).next().unwrap_or(""); matches!( @@ -1720,8 +1724,11 @@ fn is_postgres_numeric_family(data_type: &str) -> bool { | "int2" | "integer" | "int4" + | "int" | "bigint" | "int8" + | "tinyint" + | "mediumint" | "numeric" | "decimal" | "real" @@ -1738,12 +1745,13 @@ fn is_postgres_numeric_family(data_type: &str) -> bool { /// grouping ("1,23,4", "1,,234") or any non-numeric character returns None so the original /// text reaches the database and keeps its existing validation error instead of being /// silently coerced. Values without a comma are left untouched. -pub(crate) fn normalize_thousands_numeric_literal( - value: &str, - db_type: &DatabaseType, - column_type: Option<&str>, -) -> Option { - if !is_postgres_transfer_dialect(db_type) || !column_type.is_some_and(is_postgres_numeric_family) { +/// +/// Keyed on the target column type rather than the dialect: `123,456` is a formatted +/// number wherever the column stores a number, and a text column keeps the characters +/// as written. This started PostgreSQL-only, which left MySQL/TiDB imports inserting +/// `'123,456'` into a numeric column and failing (#11993). +pub(crate) fn normalize_thousands_numeric_literal(value: &str, column_type: Option<&str>) -> Option { + if !column_type.is_some_and(is_numeric_target_type) { return None; } let trimmed = value.trim();