diff --git a/apps/desktop/src/components/structure/TableStructureEditor.vue b/apps/desktop/src/components/structure/TableStructureEditor.vue index 081de755b..16872c5b3 100644 --- a/apps/desktop/src/components/structure/TableStructureEditor.vue +++ b/apps/desktop/src/components/structure/TableStructureEditor.vue @@ -38,7 +38,7 @@ import { type EditableStructureColumn, type EditableStructureIndex } from "@/lib import { getTableStructureCapabilities } from "@/lib/tableStructureCapabilities"; import { buildStructureTargetLabel, - combineDataType, + combineDataTypeForDatabase, createColumnDrafts, createIndexDrafts, DATA_TYPE_OPTIONS, @@ -545,7 +545,12 @@ watch( :allow-custom="true" trigger-class="h-6 w-full font-mono text-[11px]" @update:model-value=" - (v: string) => (column.dataType = combineDataType(v, splitDataType(column.dataType).params)) + (v: string) => + (column.dataType = combineDataTypeForDatabase( + databaseType, + v, + splitDataType(column.dataType).params, + )) " /> diff --git a/apps/desktop/src/lib/tableStructureEditorState.ts b/apps/desktop/src/lib/tableStructureEditorState.ts index 6e5bad353..bcfbf99a2 100644 --- a/apps/desktop/src/lib/tableStructureEditorState.ts +++ b/apps/desktop/src/lib/tableStructureEditorState.ts @@ -1,4 +1,4 @@ -import type { ColumnInfo, IndexInfo } from "../types/database.ts"; +import type { ColumnInfo, DatabaseType, IndexInfo } from "../types/database.ts"; import type { EditableStructureColumn, EditableStructureIndex } from "./tableStructureEditorSql.ts"; export const DATA_TYPE_OPTIONS: Record = { @@ -282,6 +282,59 @@ export function combineDataType(baseType: string, params: string): string { return `${type}(${p})`; } +export function combineDataTypeForDatabase(dbType: DatabaseType | undefined, baseType: string, params: string): string { + return combineDataType(baseType, normalizeDataTypeParams(dbType, baseType, params)); +} + +export function normalizeDataTypeParams(dbType: DatabaseType | undefined, baseType: string, params: string): string { + const p = params.trim(); + if (!p) return ""; + if (!isTemporalPrecisionType(dbType, baseType)) return p; + return isValidTemporalPrecision(dbType, p) ? p : ""; +} + +function isTemporalPrecisionType(dbType: DatabaseType | undefined, baseType: string): boolean { + const normalized = baseType.trim().replace(/\s+/g, " ").toLowerCase(); + switch (dbType) { + case "mysql": + case "doris": + case "starrocks": + case "goldendb": + case "sundb": + return ["time", "datetime", "timestamp"].includes(normalized); + case "postgres": + case "gaussdb": + case "opengauss": + case "highgo": + case "vastbase": + case "kingbase": + case "redshift": + return [ + "time", + "time without time zone", + "time with time zone", + "timestamp", + "timestamp without time zone", + "timestamp with time zone", + ].includes(normalized); + case "sqlserver": + return ["time", "datetime2", "datetimeoffset"].includes(normalized); + case "oracle": + case "dameng": + case "oceanbase-oracle": + return ["timestamp", "timestamp with time zone", "timestamp with local time zone"].includes(normalized); + default: + return false; + } +} + +function isValidTemporalPrecision(dbType: DatabaseType | undefined, params: string): boolean { + if (!/^\d+$/.test(params)) return false; + const value = Number(params); + const max = dbType === "oracle" || dbType === "dameng" || dbType === "oceanbase-oracle" ? 9 : 6; + return Number.isInteger(value) && value >= 0 && value <= max && String(value) === params; +} + export function buildStructureTargetLabel( connectionName: string | undefined, database: string | undefined, diff --git a/crates/dbx-core/src/table_structure_sql.rs b/crates/dbx-core/src/table_structure_sql.rs index ce7156cbf..fe7a14d2b 100644 --- a/crates/dbx-core/src/table_structure_sql.rs +++ b/crates/dbx-core/src/table_structure_sql.rs @@ -315,11 +315,7 @@ pub fn build_create_table_sql(options: TableStructureSqlOptions) -> TableStructu let mut column_definitions = Vec::new(); for column in &active_columns { - let data_type = if dialect == StructureDialect::ClickHouse { - clickhouse_column_type(column) - } else { - column.data_type.trim().to_string() - }; + let data_type = column_data_type(dialect, column); let mut parts = vec![quote_ident(dialect, &column.name), data_type]; if !column.is_nullable && !column.is_primary_key && dialect != StructureDialect::ClickHouse { parts.push("NOT NULL".to_string()); @@ -650,7 +646,7 @@ fn build_postgres_existing_column_sql(table: &str, column: &EditableStructureCol statements.push(format!( "ALTER TABLE {table} ALTER COLUMN {} TYPE {};", quote_ident(StructureDialect::Postgres, current_name), - column.data_type.trim() + column_data_type(StructureDialect::Postgres, column) )); } if column.is_nullable != original.is_nullable { @@ -702,7 +698,7 @@ fn build_oracle_like_existing_column_sql( statements.push(format!( "ALTER TABLE {table} MODIFY ({} {});", quote_ident(dialect, ¤t_name), - column.data_type.trim() + column_data_type(dialect, column) )); } if column.is_nullable != original.is_nullable { @@ -744,7 +740,7 @@ fn build_h2_existing_column_sql(table: &str, column: &EditableStructureColumn) - statements.push(format!( "ALTER TABLE {table} ALTER COLUMN {} SET DATA TYPE {};", quote_ident(StructureDialect::H2, ¤t_name), - column.data_type.trim() + column_data_type(StructureDialect::H2, column) )); } if column.is_nullable != original.is_nullable { @@ -991,11 +987,7 @@ fn validate_columns(columns: &[&EditableStructureColumn], warnings: &mut Vec String { - let data_type = if dialect == StructureDialect::ClickHouse { - clickhouse_column_type(column) - } else { - column.data_type.trim().to_string() - }; + let data_type = column_data_type(dialect, column); let mut parts = vec![quote_ident(dialect, &column.name), data_type]; if !column.is_nullable && !is_oracle_like(dialect) && dialect != StructureDialect::ClickHouse { parts.push("NOT NULL".to_string()); @@ -1010,6 +1002,68 @@ fn column_definition(dialect: StructureDialect, column: &EditableStructureColumn parts.join(" ") } +fn column_data_type(dialect: StructureDialect, column: &EditableStructureColumn) -> String { + if dialect == StructureDialect::ClickHouse { + return clickhouse_column_type(column); + } + normalize_column_data_type(dialect, &column.data_type) +} + +fn normalize_column_data_type(dialect: StructureDialect, data_type: &str) -> String { + let trimmed = data_type.trim(); + let Some(open_index) = trimmed.find('(') else { + return trimmed.to_string(); + }; + if !trimmed.ends_with(')') { + return trimmed.to_string(); + } + + let base_type = trimmed[..open_index].trim(); + let params = trimmed[open_index + 1..trimmed.len() - 1].trim(); + if base_type.is_empty() || params.is_empty() { + return trimmed.to_string(); + } + + if is_temporal_precision_type(dialect, base_type) { + return if is_valid_temporal_precision(params, dialect) { + format!("{base_type}({params})") + } else { + base_type.to_string() + }; + } + + trimmed.to_string() +} + +fn is_temporal_precision_type(dialect: StructureDialect, base_type: &str) -> bool { + let normalized = base_type.split_whitespace().collect::>().join(" ").to_ascii_lowercase(); + match dialect { + StructureDialect::Mysql => matches!(normalized.as_str(), "time" | "datetime" | "timestamp"), + StructureDialect::Postgres => matches!( + normalized.as_str(), + "time" + | "time without time zone" + | "time with time zone" + | "timestamp" + | "timestamp without time zone" + | "timestamp with time zone" + ), + StructureDialect::SqlServer => matches!(normalized.as_str(), "time" | "datetime2" | "datetimeoffset"), + StructureDialect::Oracle => { + matches!(normalized.as_str(), "timestamp" | "timestamp with time zone" | "timestamp with local time zone") + } + _ => false, + } +} + +fn is_valid_temporal_precision(params: &str, dialect: StructureDialect) -> bool { + let Ok(value) = params.parse::() else { + return false; + }; + let max = if dialect == StructureDialect::Oracle { 9 } else { 6 }; + value <= max && params == value.to_string() +} + fn clickhouse_column_type(column: &EditableStructureColumn) -> String { let data_type = column.data_type.trim(); if column.is_nullable { @@ -1299,6 +1353,48 @@ mod tests { ); } + #[test] + fn mysql_add_timestamp_column_drops_invalid_precision() { + let mut created_at = column("created_at"); + created_at.data_type = "timestamp(255)".to_string(); + created_at.default_value = "CURRENT_TIMESTAMP".to_string(); + + let result = build_table_structure_change_sql(TableStructureSqlOptions { + database_type: Some(DatabaseType::Mysql), + schema: None, + table_name: "users".to_string(), + columns: vec![created_at], + indexes: Vec::new(), + }); + + assert_eq!(result.warnings, Vec::::new()); + assert_eq!( + result.statements, + vec!["ALTER TABLE `users` ADD COLUMN `created_at` timestamp DEFAULT CURRENT_TIMESTAMP;"] + ); + } + + #[test] + fn mysql_add_timestamp_column_preserves_valid_precision() { + let mut created_at = column("created_at"); + created_at.data_type = "timestamp(3)".to_string(); + created_at.default_value = "CURRENT_TIMESTAMP(3)".to_string(); + + let result = build_table_structure_change_sql(TableStructureSqlOptions { + database_type: Some(DatabaseType::Mysql), + schema: None, + table_name: "users".to_string(), + columns: vec![created_at], + indexes: Vec::new(), + }); + + assert_eq!(result.warnings, Vec::::new()); + assert_eq!( + result.statements, + vec!["ALTER TABLE `users` ADD COLUMN `created_at` timestamp(3) DEFAULT CURRENT_TIMESTAMP(3);"] + ); + } + #[test] fn builds_postgres_create_table_with_comments_and_index() { let mut id = column("id"); diff --git a/packages/app-tests/tableStructureEditorState.test.ts b/packages/app-tests/tableStructureEditorState.test.ts index c7e4da084..9d38933e2 100644 --- a/packages/app-tests/tableStructureEditorState.test.ts +++ b/packages/app-tests/tableStructureEditorState.test.ts @@ -2,8 +2,10 @@ import assert from "node:assert/strict"; import test from "node:test"; import { buildStructureTargetLabel, + combineDataTypeForDatabase, createColumnDrafts, createIndexDrafts, + normalizeDataTypeParams, toColumnNames, } from "../../apps/desktop/src/lib/tableStructureEditorState.ts"; import type { ColumnInfo, IndexInfo } from "../../apps/desktop/src/types/database.ts"; @@ -123,3 +125,11 @@ test("structure editor target label omits duplicate database and schema", () => "online-postgres / app / public / users", ); }); + +test("normalizes temporal precision when combining data types", () => { + assert.equal(combineDataTypeForDatabase("mysql", "timestamp", "255"), "timestamp"); + assert.equal(combineDataTypeForDatabase("mysql", "timestamp", "3"), "timestamp(3)"); + assert.equal(combineDataTypeForDatabase("mysql", "varchar", "255"), "varchar(255)"); + assert.equal(normalizeDataTypeParams("oracle", "timestamp", "9"), "9"); + assert.equal(normalizeDataTypeParams("oracle", "timestamp", "10"), ""); +});