fix(import): parse numeric thousands separators
This commit is contained in:
1 parent
cddc6a2c40
commit
b6df771760
2 files changed
+274
-3
No files matched your search
@@ -20,7 +20,7 @@ use crate::connection::{task_client_session_id, AppState, PoolKind};
|
||||
use crate::models::connection::DatabaseType;
|
||||
use crate::transfer::{
|
||||
execute_on_pool, generate_insert_typed, generate_insert_typed_sql_batches, get_columns_for_transfer,
|
||||
normalize_integer_literal, qualified_table, quote_identifier, SqlBatchLimits,
|
||||
normalize_integer_literal, normalize_thousands_numeric_literal, qualified_table, quote_identifier, SqlBatchLimits,
|
||||
};
|
||||
|
||||
pub const DEFAULT_PREVIEW_LIMIT: usize = 50;
|
||||
@@ -3360,8 +3360,15 @@ fn normalize_import_value(
|
||||
date_time_format: Option<&str>,
|
||||
) -> serde_json::Value {
|
||||
let normalized = normalize_import_temporal_value(value, data_type, db_type, kingbase_oracle_mode, date_time_format);
|
||||
let integer_text =
|
||||
normalized.as_str().map(str::to_owned).or_else(|| normalized.as_number().map(ToString::to_string));
|
||||
// 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));
|
||||
let integer_text = thousands_canonical
|
||||
.as_deref()
|
||||
.or_else(|| normalized.as_str())
|
||||
.map(str::to_owned)
|
||||
.or_else(|| normalized.as_number().map(ToString::to_string));
|
||||
if let Some(integer_text) = integer_text
|
||||
.as_deref()
|
||||
.and_then(|value| normalize_integer_literal(value, db_type, data_type))
|
||||
@@ -3370,6 +3377,9 @@ fn normalize_import_value(
|
||||
// Normalize before both INSERT and COPY paths; COPY does not pass through SQL literal escaping.
|
||||
return serde_json::Value::Number(integer_text.into());
|
||||
}
|
||||
if let Some(canonical) = thousands_canonical {
|
||||
return serde_json::Value::String(canonical);
|
||||
}
|
||||
normalized
|
||||
}
|
||||
|
||||
@@ -8501,6 +8511,188 @@ mod tests {
|
||||
assert_eq!(display(12.5, "["), "12.5");
|
||||
}
|
||||
|
||||
fn postgres_import_batches(
|
||||
rows: Vec<Vec<serde_json::Value>>,
|
||||
target_types: &[(&str, &str)],
|
||||
) -> Vec<ImportSqlBatch> {
|
||||
let data = ParsedImportFile {
|
||||
columns: target_types.iter().map(|(column, _)| column.to_string()).collect(),
|
||||
rows,
|
||||
total_rows: 1,
|
||||
effective_encoding: None,
|
||||
};
|
||||
let mappings = target_types
|
||||
.iter()
|
||||
.map(|(column, _)| TableImportColumnMapping {
|
||||
source_column: column.to_string(),
|
||||
target_column: column.to_string(),
|
||||
target_data_type: None,
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let target_column_types = target_types
|
||||
.iter()
|
||||
.map(|(column, data_type)| (column.to_string(), data_type.to_string()))
|
||||
.collect::<Vec<_>>();
|
||||
build_import_insert_batches(
|
||||
&data,
|
||||
&mappings,
|
||||
&target_column_types,
|
||||
"issue_6491",
|
||||
"",
|
||||
&DatabaseType::Postgres,
|
||||
500,
|
||||
)
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn postgres_import_converts_valid_thousands_separators_for_numeric_targets() {
|
||||
for (value, data_type, expected) in [
|
||||
("1,234.56", "numeric(18,2)", "'1234.56'"),
|
||||
("-1,234.56", "numeric(18,2)", "'-1234.56'"),
|
||||
("+1,234.56", "numeric(18,2)", "'1234.56'"),
|
||||
("1,234.00", "numeric(18,2)", "'1234.00'"),
|
||||
("1,234,567.89", "decimal(12,2)", "'1234567.89'"),
|
||||
("1,234", "bigint", "'1234'"),
|
||||
("12,345", "integer", "'12345'"),
|
||||
("1,234,567,890", "bigint", "'1234567890'"),
|
||||
("1,234.5", "double precision", "'1234.5'"),
|
||||
("1,234.5", "real", "'1234.5'"),
|
||||
] {
|
||||
let batches = postgres_import_batches(vec![vec![serde_json::json!(value)]], &[("amount", data_type)]);
|
||||
assert_eq!(
|
||||
batches[0].sql,
|
||||
format!("INSERT INTO \"issue_6491\" (\"amount\") VALUES\n({expected})"),
|
||||
"{value} -> {data_type}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn postgres_import_preserves_thousands_separators_for_text_targets() {
|
||||
for data_type in ["varchar(64)", "text"] {
|
||||
let batches = postgres_import_batches(vec![vec![serde_json::json!("1,234.56")]], &[("amount", data_type)]);
|
||||
assert_eq!(
|
||||
batches[0].sql,
|
||||
format!("INSERT INTO \"issue_6491\" (\"amount\") VALUES\n('1,234.56')"),
|
||||
"{data_type}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn postgres_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 = postgres_import_batches(vec![vec![serde_json::json!(value)]], &[("amount", "numeric(18,2)")]);
|
||||
assert_eq!(
|
||||
batches[0].sql,
|
||||
format!("INSERT INTO \"issue_6491\" (\"amount\") VALUES\n('{value}')"),
|
||||
"{value}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn postgres_import_keeps_plain_numeric_and_empty_values_unchanged() {
|
||||
for (value, data_type, expected) in [
|
||||
(serde_json::json!("1234.56"), "numeric(18,2)", "'1234.56'"),
|
||||
(serde_json::json!("0"), "numeric(18,2)", "'0'"),
|
||||
(serde_json::json!("1234.56"), "bigint", "'1234.56'"),
|
||||
(serde_json::json!(1234.56), "numeric(18,2)", "1234.56"),
|
||||
] {
|
||||
let label = value.to_string();
|
||||
let batches = postgres_import_batches(vec![vec![value]], &[("amount", data_type)]);
|
||||
assert_eq!(
|
||||
batches[0].sql,
|
||||
format!("INSERT INTO \"issue_6491\" (\"amount\") VALUES\n({expected})"),
|
||||
"{label} -> {data_type}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn postgres_copy_import_uses_canonical_numeric_text() {
|
||||
let plan = CompiledImportPlan {
|
||||
mapped_source_indexes: vec![0],
|
||||
target_columns: vec!["amount".to_string()],
|
||||
column_types: vec![Some("numeric(18,2)".to_string())],
|
||||
};
|
||||
let (_, data) =
|
||||
build_postgres_copy_text_batch(&[vec![serde_json::json!("1,234.56")]], &plan, "issue_6491", "", None)
|
||||
.unwrap();
|
||||
assert_eq!(data, b"1234.56\n");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn excel_text_cell_with_thousands_separator_imports_to_postgres_numeric() {
|
||||
let path = std::env::temp_dir().join(format!("dbx-table-import-6491-{}.xlsx", uuid::Uuid::new_v4()));
|
||||
let sheet_xml = r#"<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
|
||||
<worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main">
|
||||
<dimension ref="A1:A3"/>
|
||||
<sheetData>
|
||||
<row r="1"><c r="A1" t="inlineStr"><is><t>amount</t></is></c></row>
|
||||
<row r="2"><c r="A2" t="inlineStr"><is><t>1,234.56</t></is></c></row>
|
||||
<row r="3"><c r="A3" t="inlineStr"><is><t>-1,234</t></is></c></row>
|
||||
</sheetData>
|
||||
</worksheet>"#;
|
||||
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!("1,234.56")], vec![serde_json::json!("-1,234")]]);
|
||||
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(), "numeric(18,2)".to_string())],
|
||||
"issue_6491",
|
||||
"",
|
||||
&DatabaseType::Postgres,
|
||||
500,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(batches[0].sql, "INSERT INTO \"issue_6491\" (\"amount\") VALUES\n('1234.56'),\n('-1234')");
|
||||
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();
|
||||
let mappings = vec![TableImportColumnMapping {
|
||||
source_column: "amount".to_string(),
|
||||
target_column: "amount".to_string(),
|
||||
target_data_type: None,
|
||||
}];
|
||||
let batches = build_import_insert_batches(
|
||||
&parsed,
|
||||
&mappings,
|
||||
&[("amount".to_string(), "numeric(18,2)".to_string())],
|
||||
"issue_6491",
|
||||
"",
|
||||
&DatabaseType::Postgres,
|
||||
500,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(batches[0].sql, "INSERT INTO \"issue_6491\" (\"amount\") VALUES\n('1234.56'),\n('12345')");
|
||||
let text_batches = build_import_insert_batches(
|
||||
&parsed,
|
||||
&mappings,
|
||||
&[("amount".to_string(), "varchar(32)".to_string())],
|
||||
"issue_6491",
|
||||
"",
|
||||
&DatabaseType::Postgres,
|
||||
500,
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(text_batches[0].sql, "INSERT INTO \"issue_6491\" (\"amount\") VALUES\n('1,234.56'),\n('12,345')");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn formats_only_excel_columns_mapped_to_text_targets() {
|
||||
let path = std::env::temp_dir().join(format!("dbx-table-import-display-formats-{}.xlsx", uuid::Uuid::new_v4()));
|
||||
|
||||
@@ -1303,6 +1303,85 @@ pub(crate) fn normalize_integer_literal(
|
||||
Some(integer.to_string())
|
||||
}
|
||||
|
||||
fn is_postgres_numeric_family(data_type: &str) -> bool {
|
||||
let normalized = data_type.trim().to_ascii_lowercase();
|
||||
let base = normalized.split(['(', ' ']).next().unwrap_or("");
|
||||
matches!(
|
||||
base,
|
||||
"smallint"
|
||||
| "int2"
|
||||
| "integer"
|
||||
| "int4"
|
||||
| "bigint"
|
||||
| "int8"
|
||||
| "numeric"
|
||||
| "decimal"
|
||||
| "real"
|
||||
| "float4"
|
||||
| "float"
|
||||
| "double"
|
||||
| "doubleprecision"
|
||||
| "float8"
|
||||
)
|
||||
}
|
||||
|
||||
/// Strips validated en-US thousands separators from a numeric literal for numeric target
|
||||
/// columns. Only standard 3-digit grouping is accepted ("1,234", "12,345,678"); malformed
|
||||
/// 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<String> {
|
||||
if !is_postgres_transfer_dialect(db_type) || !column_type.is_some_and(is_postgres_numeric_family) {
|
||||
return None;
|
||||
}
|
||||
let trimmed = value.trim();
|
||||
if trimmed.is_empty() || trimmed.bytes().any(|byte| matches!(byte, b'e' | b'E')) {
|
||||
return None;
|
||||
}
|
||||
let (negative, unsigned) = match trimmed.as_bytes().first() {
|
||||
Some(b'-') => (true, &trimmed[1..]),
|
||||
Some(b'+') => (false, &trimmed[1..]),
|
||||
_ => (false, trimmed),
|
||||
};
|
||||
if unsigned.is_empty() {
|
||||
return None;
|
||||
}
|
||||
let (integer_part, fraction) = match unsigned.split_once('.') {
|
||||
Some((integer, fraction)) => (integer, Some(fraction)),
|
||||
None => (unsigned, None),
|
||||
};
|
||||
if fraction.is_some_and(|fraction| fraction.is_empty() || !fraction.bytes().all(|byte| byte.is_ascii_digit())) {
|
||||
return None;
|
||||
}
|
||||
let mut digits = String::with_capacity(unsigned.len());
|
||||
for (index, group) in integer_part.split(',').enumerate() {
|
||||
if group.is_empty() || !group.bytes().all(|byte| byte.is_ascii_digit()) {
|
||||
return None;
|
||||
}
|
||||
if (index == 0 && group.len() > 3) || (index > 0 && group.len() != 3) {
|
||||
return None;
|
||||
}
|
||||
digits.push_str(group);
|
||||
}
|
||||
if !integer_part.contains(',') {
|
||||
return None;
|
||||
}
|
||||
let mut canonical = String::with_capacity(trimmed.len());
|
||||
if negative {
|
||||
canonical.push('-');
|
||||
}
|
||||
canonical.push_str(&digits);
|
||||
if let Some(fraction) = fraction {
|
||||
canonical.push('.');
|
||||
canonical.push_str(fraction);
|
||||
}
|
||||
Some(canonical)
|
||||
}
|
||||
|
||||
fn is_postgres_sequence_default(default_value: Option<&str>) -> bool {
|
||||
default_value.is_some_and(|value| value.to_ascii_lowercase().contains("nextval("))
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user