diff --git a/apps/desktop/src/lib/__tests__/tableStructureEditorState.spec.ts b/apps/desktop/src/lib/__tests__/tableStructureEditorState.spec.ts index 41bc8584e..3d0942a6e 100644 --- a/apps/desktop/src/lib/__tests__/tableStructureEditorState.spec.ts +++ b/apps/desktop/src/lib/__tests__/tableStructureEditorState.spec.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { combineDataTypeForDatabase, dataTypeLengthInputValue, isDataTypeLengthDisabled, splitDataType } from "../tableStructureEditorState"; +import { combineDataTypeForDatabase, createColumnDrafts, dataTypeLengthInputValue, isDataTypeLengthDisabled, splitDataType } from "../tableStructureEditorState"; describe("tableStructureEditorState", () => { it("keeps mysql unsigned attributes in the editable base type", () => { @@ -27,4 +27,47 @@ describe("tableStructureEditorState", () => { expect(dataTypeLengthInputValue("mysql", dataType)).toBe(""); expect(dataTypeLengthInputValue("mysql", "set('manual','auto')")).toBe(""); }); + + it("strips SQL Server metadata parentheses from editable defaults", () => { + const drafts = createColumnDrafts( + [ + { + name: "name", + data_type: "nvarchar(100)", + is_nullable: true, + column_default: "('')", + is_primary_key: false, + extra: null, + }, + { + name: "active", + data_type: "bit", + is_nullable: false, + column_default: "((1))", + is_primary_key: false, + extra: null, + }, + { + name: "created_at", + data_type: "datetime2(7)", + is_nullable: false, + column_default: "((sysdatetime()))", + is_primary_key: false, + extra: null, + }, + { + name: "label", + data_type: "nvarchar(100)", + is_nullable: true, + column_default: "('prefix (internal)')", + is_primary_key: false, + extra: null, + }, + ], + "sqlserver", + ); + + expect(drafts.map((draft) => draft.defaultValue)).toEqual(["''", "1", "sysdatetime()", "'prefix (internal)'"]); + expect(drafts.map((draft) => draft.original?.column_default)).toEqual(["''", "1", "sysdatetime()", "'prefix (internal)'"]); + }); }); diff --git a/apps/desktop/src/lib/tableStructureEditorState.ts b/apps/desktop/src/lib/tableStructureEditorState.ts index 4987a6995..be8e182f1 100644 --- a/apps/desktop/src/lib/tableStructureEditorState.ts +++ b/apps/desktop/src/lib/tableStructureEditorState.ts @@ -533,9 +533,58 @@ function stripPostgresStringDefaultCast(defaultValue: string, dataType: string): return match?.[1] ?? defaultValue; } +function isWrappedByOuterParens(value: string): boolean { + if (value.length < 2 || value[0] !== "(" || value[value.length - 1] !== ")") return false; + + let depth = 0; + let inString = false; + let inBracketIdentifier = false; + for (let index = 0; index < value.length; index += 1) { + const char = value[index]; + if (inString) { + if (char === "'" && value[index + 1] === "'") { + index += 1; + } else if (char === "'") { + inString = false; + } + continue; + } + if (inBracketIdentifier) { + if (char === "]") inBracketIdentifier = false; + continue; + } + if (char === "'") { + inString = true; + continue; + } + if (char === "[") { + inBracketIdentifier = true; + continue; + } + if (char === "(") { + depth += 1; + } else if (char === ")") { + depth -= 1; + if (depth < 0) return false; + if (depth === 0 && index < value.length - 1) return false; + } + } + return depth === 0; +} + +function stripSqlServerDefaultOuterParens(defaultValue: string): string { + let value = defaultValue.trim(); + while (isWrappedByOuterParens(value)) { + value = value.slice(1, -1).trim(); + } + return value; +} + function columnDefaultForEditor(column: ColumnInfo, databaseType?: DatabaseType): string { const defaultValue = column.column_default ?? ""; - return databaseType === "postgres" ? stripPostgresStringDefaultCast(defaultValue, column.data_type) : defaultValue; + if (databaseType === "postgres") return stripPostgresStringDefaultCast(defaultValue, column.data_type); + if (databaseType === "sqlserver") return stripSqlServerDefaultOuterParens(defaultValue); + return defaultValue; } export function createColumnDrafts(columns: ColumnInfo[], databaseType?: DatabaseType): EditableStructureColumn[] { diff --git a/crates/dbx-core/src/table_structure_sql/column_alter.rs b/crates/dbx-core/src/table_structure_sql/column_alter.rs index 00065e45b..37e988fb9 100644 --- a/crates/dbx-core/src/table_structure_sql/column_alter.rs +++ b/crates/dbx-core/src/table_structure_sql/column_alter.rs @@ -444,12 +444,13 @@ pub(super) fn build_sqlserver_existing_column_sql( .unwrap_or(false); if has_old_default { + let sql_var = sqlserver_default_constraint_sql_var(table, ¤t_name); statements.push(format!( - "DECLARE @sql NVARCHAR(MAX);\ - SELECT @sql = 'ALTER TABLE {table} DROP CONSTRAINT [' + name + ']'\ - FROM sys.default_constraints\ - WHERE parent_object_id = OBJECT_ID('{table}') AND parent_column_id = COLUMNPROPERTY(OBJECT_ID('{table}'), '{col_name}', 'ColumnId');\ - EXEC sp_executesql @sql;", + "DECLARE {sql_var} NVARCHAR(MAX); \ + SELECT TOP (1) {sql_var} = N'ALTER TABLE {table} DROP CONSTRAINT ' + QUOTENAME(dc.name) \ + FROM sys.default_constraints AS dc \ + WHERE dc.parent_object_id = OBJECT_ID(N'{table}') AND dc.parent_column_id = COLUMNPROPERTY(OBJECT_ID(N'{table}'), N'{col_name}', 'ColumnId'); \ + IF {sql_var} IS NOT NULL EXEC sp_executesql {sql_var};", table = table.replace('\'', "''"), col_name = current_name.replace('\'', "''") )); @@ -484,6 +485,15 @@ pub(super) fn build_sqlserver_existing_column_sql( statements } +fn sqlserver_default_constraint_sql_var(table: &str, column_name: &str) -> String { + let mut hash = 0x811c_9dc5u32; + for byte in table.bytes().chain([0]).chain(column_name.bytes()) { + hash ^= u32::from(byte); + hash = hash.wrapping_mul(0x0100_0193); + } + format!("@dbx_default_sql_{hash:08x}") +} + fn has_sqlserver_identity_change(column: &EditableStructureColumn) -> bool { let Some(original) = &column.original else { return false; diff --git a/crates/dbx-core/src/table_structure_sql/tests.rs b/crates/dbx-core/src/table_structure_sql/tests.rs index 399884e67..6dc1070e5 100644 --- a/crates/dbx-core/src/table_structure_sql/tests.rs +++ b/crates/dbx-core/src/table_structure_sql/tests.rs @@ -1021,6 +1021,78 @@ fn builds_sql_server_quoted_column_and_index_statements() { ); } +#[test] +fn sqlserver_default_changes_drop_old_constraints_with_isolated_batches() { + let mut sku = column("sku"); + sku.data_type = "nvarchar(64)".to_string(); + sku.default_value = "new sku".to_string(); + sku.original = Some(ColumnInfo { + name: "sku".to_string(), + data_type: "nvarchar(64)".to_string(), + is_nullable: true, + column_default: Some("'old sku'".to_string()), + is_primary_key: false, + extra: None, + comment: None, + }); + + let mut active = column("active"); + active.data_type = "bit".to_string(); + active.is_nullable = false; + active.default_value = "1".to_string(); + active.original = Some(ColumnInfo { + name: "active".to_string(), + data_type: "bit".to_string(), + is_nullable: false, + column_default: Some("0".to_string()), + is_primary_key: false, + extra: None, + comment: None, + }); + + let result = build_table_structure_change_sql(TableStructureSqlOptions { + database_type: Some(DatabaseType::SqlServer), + schema: Some("core".to_string()), + table_name: "products".to_string(), + columns: vec![sku, active], + indexes: Vec::new(), + foreign_keys: Vec::new(), + triggers: Vec::new(), + table_comment: None, + original_table_comment: None, + }); + + assert_eq!(result.warnings, Vec::::new()); + assert_eq!(result.statements.len(), 4); + + let sku_drop = &result.statements[0]; + let active_drop = &result.statements[2]; + let sku_var = sku_drop.strip_prefix("DECLARE ").unwrap().split_once(" NVARCHAR(MAX);").unwrap().0; + let active_var = active_drop.strip_prefix("DECLARE ").unwrap().split_once(" NVARCHAR(MAX);").unwrap().0; + assert_ne!(sku_var, "@sql"); + assert_ne!(active_var, "@sql"); + assert_ne!(sku_var, active_var); + + for (sql, column_name) in [(sku_drop, "sku"), (active_drop, "active")] { + assert!(sql.contains("SELECT TOP (1)")); + assert!(sql.contains(" + QUOTENAME(dc.name) FROM sys.default_constraints AS dc WHERE ")); + assert!(sql.contains("OBJECT_ID(N'[core].[products]')")); + assert!(sql.contains(&format!("N'{column_name}', 'ColumnId'"))); + assert!(sql.contains(" IF ")); + assert!(!sql.contains("]'FROM")); + assert!(!sql.contains("constraintsWHERE")); + } + + assert_eq!( + result.statements[1], + "ALTER TABLE [core].[products] ADD CONSTRAINT [DF_products_sku] DEFAULT 'new sku' FOR [sku];" + ); + assert_eq!( + result.statements[3], + "ALTER TABLE [core].[products] ADD CONSTRAINT [DF_products_active] DEFAULT 1 FOR [active];" + ); +} + #[test] fn sqlserver_unchanged_foreign_key_does_not_warn_when_saving_other_changes() { let mut email = column("email"); diff --git a/crates/dbx-core/tests/live_sqlserver_completion.rs b/crates/dbx-core/tests/live_sqlserver_completion.rs index e13be8bde..296bf72c8 100644 --- a/crates/dbx-core/tests/live_sqlserver_completion.rs +++ b/crates/dbx-core/tests/live_sqlserver_completion.rs @@ -2,6 +2,9 @@ use dbx_core::connection::{AppState, PoolKind}; use dbx_core::models::connection::DatabaseType; use dbx_core::query_result_export::{export_query_result_core, ExportStatus, QueryResultExportRequest}; use dbx_core::storage::Storage; +use dbx_core::table_structure_sql::{ + build_table_structure_change_sql, ColumnInfo, EditableStructureColumn, TableStructureSqlOptions, +}; use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Duration; @@ -85,6 +88,110 @@ async fn live_sqlserver_execute_query_creates_schema() { assert!(schemas.expect("list schemas").contains(&schema)); } +#[tokio::test] +#[ignore = "requires DBX_LIVE_SQLSERVER_HOST/PORT/USER/PASSWORD pointing at a writable SQL Server database"] +async fn live_sqlserver_table_structure_default_changes_drop_existing_constraints() { + let database = std::env::var("DBX_LIVE_SQLSERVER_DATABASE").unwrap_or_else(|_| "tempdb".to_string()); + let host = std::env::var("DBX_LIVE_SQLSERVER_HOST").unwrap_or_else(|_| "127.0.0.1".to_string()); + let port = std::env::var("DBX_LIVE_SQLSERVER_PORT").ok().and_then(|value| value.parse().ok()).unwrap_or(1433); + let user = std::env::var("DBX_LIVE_SQLSERVER_USER").unwrap_or_else(|_| "sa".to_string()); + let password = std::env::var("DBX_LIVE_SQLSERVER_PASSWORD").expect("DBX_LIVE_SQLSERVER_PASSWORD"); + let mut client = + dbx_core::db::sqlserver::connect(&host, port, &user, &password, Some(&database), Duration::from_secs(10)) + .await + .expect("connect SQL Server"); + + let suffix = uuid::Uuid::new_v4().simple().to_string(); + let schema = format!("dbx_default_{suffix}"); + let table = "products"; + let create_schema = format!("CREATE SCHEMA [{schema}];"); + let create_table = format!( + "\ + CREATE TABLE [{schema}].[{table}] (\ + [sku] NVARCHAR(64) NULL CONSTRAINT [DF_{schema}_{table}_sku_old] DEFAULT N'old sku',\ + [active] BIT NOT NULL CONSTRAINT [DF_{schema}_{table}_active_old] DEFAULT 0\ + );" + ); + dbx_core::db::sqlserver::execute_query(&mut client, &create_schema).await.expect("create live test schema"); + dbx_core::db::sqlserver::execute_query(&mut client, &create_table).await.expect("create table with defaults"); + + let mut sku = structure_column("sku", "nvarchar(64)", true, "new sku", Some("'old sku'")); + let mut active = structure_column("active", "bit", false, "1", Some("0")); + sku.original_position = Some(0); + active.original_position = Some(1); + let result = build_table_structure_change_sql(TableStructureSqlOptions { + database_type: Some(DatabaseType::SqlServer), + schema: Some(schema.clone()), + table_name: table.to_string(), + columns: vec![sku, active], + indexes: Vec::new(), + foreign_keys: Vec::new(), + triggers: Vec::new(), + table_comment: None, + original_table_comment: None, + }); + assert_eq!(result.warnings, Vec::::new()); + assert_eq!(result.statements.len(), 4); + + let execution_result = async { + for statement in &result.statements { + dbx_core::db::sqlserver::execute_query(&mut client, statement).await?; + } + Ok::<(), String>(()) + } + .await; + + let verify_sql = format!( + "\ + SELECT c.name, dc.definition \ + FROM sys.default_constraints AS dc \ + JOIN sys.columns AS c ON c.object_id = dc.parent_object_id AND c.column_id = dc.parent_column_id \ + WHERE dc.parent_object_id = OBJECT_ID(N'[{schema}].[{table}]') \ + ORDER BY c.name;" + ); + let verify_result = dbx_core::db::sqlserver::execute_query(&mut client, &verify_sql).await; + let cleanup = format!("DROP TABLE IF EXISTS [{schema}].[{table}]; DROP SCHEMA IF EXISTS [{schema}];"); + let _ = dbx_core::db::sqlserver::execute_batch(&mut client, &cleanup).await; + + execution_result.expect("execute generated default constraint SQL"); + let verify_result = verify_result.expect("verify changed defaults"); + assert_eq!(verify_result.rows.len(), 2, "rows={:?}", verify_result.rows); + assert_eq!(verify_result.rows[0][0], serde_json::json!("active")); + assert_eq!(verify_result.rows[0][1], serde_json::json!("((1))")); + assert_eq!(verify_result.rows[1][0], serde_json::json!("sku")); + assert_eq!(verify_result.rows[1][1], serde_json::json!("('new sku')")); +} + +fn structure_column( + name: &str, + data_type: &str, + is_nullable: bool, + default_value: &str, + original_default: Option<&str>, +) -> EditableStructureColumn { + EditableStructureColumn { + id: name.to_string(), + name: name.to_string(), + data_type: data_type.to_string(), + is_nullable, + default_value: default_value.to_string(), + comment: String::new(), + is_primary_key: false, + extra: None, + original: Some(ColumnInfo { + name: name.to_string(), + data_type: data_type.to_string(), + is_nullable, + column_default: original_default.map(str::to_string), + is_primary_key: false, + extra: None, + comment: None, + }), + original_position: None, + marked_for_drop: false, + } +} + #[tokio::test] #[ignore = "requires DBX_LIVE_SQLSERVER_HOST/PORT/USER/PASSWORD pointing at a writable SQL Server database"] async fn live_sqlserver_stream_first_result_set_exports_cte_query_rows() {