From dbc9072d30a0e461cdda9c8975ee7ec357b448c1 Mon Sep 17 00:00:00 2001 From: zipg Date: Sun, 28 Jun 2026 00:11:22 +0800 Subject: [PATCH] fix(data-grid): convert empty string to NULL for nullable non-text columns Co-authored-by: zipg <4047349+zipg@users.noreply.github.com> --- crates/dbx-core/src/data_grid_sql.rs | 153 +++++++++++++++++++++++++-- 1 file changed, 145 insertions(+), 8 deletions(-) diff --git a/crates/dbx-core/src/data_grid_sql.rs b/crates/dbx-core/src/data_grid_sql.rs index 64ff120eb9..be8d11204e 100644 --- a/crates/dbx-core/src/data_grid_sql.rs +++ b/crates/dbx-core/src/data_grid_sql.rs @@ -568,7 +568,7 @@ fn build_data_grid_save_statements(options: &DataGridSaveStatementOptions) -> Ve Some(format!( "{} = {}", quote_ident(options.database_type, column), - format_grid_sql_literal(value, options.database_type, column_info_for(column_info, column)) + format_grid_save_sql_literal(value, options.database_type, column_info_for(column_info, column)) )) }) .collect::>() @@ -631,7 +631,7 @@ fn build_data_grid_save_statements(options: &DataGridSaveStatementOptions) -> Ve let values = insert_pairs .iter() .map(|(column, value)| { - format_grid_sql_literal(value, options.database_type, column_info_for(column_info, column)) + format_grid_save_sql_literal(value, options.database_type, column_info_for(column_info, column)) }) .collect::>() .join(", "); @@ -662,7 +662,7 @@ fn build_data_grid_rollback_statements(options: &DataGridSaveStatementOptions) - let mut statements = Vec::new(); for row in &options.new_rows { - let where_clause = build_row_where(options.database_type, &save_columns, row, column_info); + let where_clause = build_save_row_where(options.database_type, &save_columns, row, column_info); if !where_clause.is_empty() { statements .push(data_grid_statement(options.database_type, format!("DELETE FROM {table} WHERE {where_clause}"))); @@ -754,7 +754,13 @@ fn build_data_grid_rollback_statements(options: &DataGridSaveStatementOptions) - column_info, )]; predicates.extend(writable_changes.iter().map(|((_, value), column)| { - build_column_predicate(options.database_type, column, value, column_info_for(column_info, column), true) + build_save_column_predicate( + options.database_type, + column, + value, + column_info_for(column_info, column), + true, + ) })); statements.push(data_grid_statement( options.database_type, @@ -883,6 +889,23 @@ pub fn format_grid_sql_literal( } } +fn format_grid_save_sql_literal( + value: &Value, + database_type: Option, + column_info: Option<&DataGridColumnInfo>, +) -> String { + if empty_string_saves_as_null(value, column_info) { + "NULL".to_string() + } else { + format_grid_sql_literal(value, database_type, column_info) + } +} + +fn empty_string_saves_as_null(value: &Value, column_info: Option<&DataGridColumnInfo>) -> bool { + value.as_str() == Some("") + && column_info.is_some_and(|column| column.is_nullable && !is_textual_column_type(&column.data_type)) +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum OracleTemporalKind { Date, @@ -1238,6 +1261,32 @@ fn build_row_where( .join(" AND ") } +fn build_save_row_where( + database_type: Option, + columns: &[Option], + row: &[Value], + column_info: &[DataGridColumnInfo], +) -> String { + columns + .iter() + .enumerate() + .filter_map(|(index, column)| { + let column = column.as_deref()?; + if is_oracle_row_id(database_type, Some(column)) { + return None; + } + Some(build_save_column_predicate( + database_type, + column, + row.get(index).unwrap_or(&Value::Null), + column_info_for(column_info, column), + true, + )) + }) + .collect::>() + .join(" AND ") +} + fn build_column_predicate( database_type: Option, column: &str, @@ -1255,6 +1304,23 @@ fn build_column_predicate( } } +fn build_save_column_predicate( + database_type: Option, + column: &str, + value: &Value, + column_info: Option<&DataGridColumnInfo>, + use_binary_text_comparison: bool, +) -> String { + let ident = predicate_ident(database_type, column); + if value.is_null() || empty_string_saves_as_null(value, column_info) { + format!("{ident} IS NULL") + } else if use_binary_text_comparison && uses_mysql_binary_text_predicate(database_type, value, column_info) { + format!("BINARY {ident} = {}", format_grid_save_sql_literal(value, database_type, column_info)) + } else { + format!("{ident} = {}", format_grid_save_sql_literal(value, database_type, column_info)) + } +} + fn data_grid_statement(database_type: Option, sql: String) -> String { if database_type == Some(DatabaseType::ManticoreSearch) { sql @@ -1290,10 +1356,29 @@ fn uses_mysql_binary_text_predicate( } fn is_textual_column_type(data_type: &str) -> bool { - let lower = data_type.to_ascii_lowercase(); - lower.split(|ch: char| !ch.is_ascii_alphanumeric()).any(|token| { - matches!(token, "char" | "varchar" | "text" | "tinytext" | "mediumtext" | "longtext" | "enum" | "set") - }) + let lower = data_type.trim().to_ascii_lowercase(); + let base = lower.split(['(', ':', ' ']).next().unwrap_or("").trim(); + matches!( + base, + "char" + | "character" + | "varchar" + | "varchar2" + | "nvarchar" + | "nvarchar2" + | "nchar" + | "string" + | "text" + | "tinytext" + | "mediumtext" + | "longtext" + | "ntext" + | "clob" + | "nclob" + | "enum" + | "set" + ) || lower.starts_with("character varying") + || lower.starts_with("national character varying") } fn is_oracle_row_id(database_type: Option, name: Option<&str>) -> bool { @@ -1887,6 +1972,58 @@ mod tests { assert_eq!(format_grid_sql_literal(&json!("0"), Some(DatabaseType::Postgres), Some(&bit)), "'0'"); } + #[test] + fn saves_empty_nullable_mysql_numeric_cell_as_null() { + let result = prepare_data_grid_save(DataGridSaveStatementOptions { + database_type: Some(DatabaseType::Mysql), + table_meta: DataGridTableMeta { + schema: None, + table_name: "employees".to_string(), + primary_keys: vec!["id".to_string()], + columns: Some(vec![column("id", "int(11)", false, None), column("age", "int(11)", true, None)]), + }, + columns: vec!["id".to_string(), "age".to_string()], + source_columns: None, + rows: vec![vec![json!(2), json!(36)]], + dirty_rows: vec![(0, vec![(1, json!(""))])], + deleted_rows: vec![], + new_rows: vec![], + }); + + assert_eq!(result.validation_error, None); + assert_eq!(result.statements, vec!["UPDATE `employees` SET `age` = NULL WHERE `id` = 2;"]); + assert_eq!( + result.rollback_statements, + vec!["UPDATE `employees` SET `age` = 36 WHERE `id` = 2 AND `age` IS NULL;"] + ); + } + + #[test] + fn keeps_empty_nullable_mysql_text_cell_as_empty_string() { + let result = prepare_data_grid_save(DataGridSaveStatementOptions { + database_type: Some(DatabaseType::Mysql), + table_meta: DataGridTableMeta { + schema: None, + table_name: "employees".to_string(), + primary_keys: vec!["id".to_string()], + columns: Some(vec![column("id", "int(11)", false, None), column("name", "varchar(50)", true, None)]), + }, + columns: vec!["id".to_string(), "name".to_string()], + source_columns: None, + rows: vec![vec![json!(2), json!("Ada")]], + dirty_rows: vec![(0, vec![(1, json!(""))])], + deleted_rows: vec![], + new_rows: vec![], + }); + + assert_eq!(result.validation_error, None); + assert_eq!(result.statements, vec!["UPDATE `employees` SET `name` = '' WHERE `id` = 2;"]); + assert_eq!( + result.rollback_statements, + vec!["UPDATE `employees` SET `name` = 'Ada' WHERE `id` = 2 AND BINARY `name` = '';"] + ); + } + #[test] fn prepares_sqlserver_save_statements() { let result = prepare_data_grid_save(DataGridSaveStatementOptions {