From fe8737550c3891471d33d60853f84eb92e780590 Mon Sep 17 00:00:00 2001 From: zipg Date: Sat, 26 Sep 2026 06:48:05 +0800 Subject: [PATCH] fix(sqlserver): preserve computed column DDL in view and export --- crates/dbx-core/src/data/table_import.rs | 1 + crates/dbx-core/src/schema/mod.rs | 100 ++++++++++++- .../live_sqlserver_computed_column_ddl.rs | 134 ++++++++++++++++++ crates/dbx-driver-sqlserver/src/sqlserver.rs | 88 ++++++++++-- 4 files changed, 307 insertions(+), 16 deletions(-) create mode 100644 crates/dbx-core/tests/live_sqlserver_computed_column_ddl.rs diff --git a/crates/dbx-core/src/data/table_import.rs b/crates/dbx-core/src/data/table_import.rs index 1a6b01590a..530003412e 100644 --- a/crates/dbx-core/src/data/table_import.rs +++ b/crates/dbx-core/src/data/table_import.rs @@ -11471,6 +11471,7 @@ mod tests { is_computed, is_hidden, generated_always_type: i32::from(is_hidden), + computed_clause: None, } } diff --git a/crates/dbx-core/src/schema/mod.rs b/crates/dbx-core/src/schema/mod.rs index 8d13044840..a08b23353f 100644 --- a/crates/dbx-core/src/schema/mod.rs +++ b/crates/dbx-core/src/schema/mod.rs @@ -12678,6 +12678,49 @@ mod ddl_tests { assert!(!ddl.contains("audit_user();;"), "ddl: {ddl}"); } + #[test] + fn sqlserver_table_ddl_renders_computed_columns_with_their_definition() { + let mut columns = vec![column("id", "nvarchar(100)"), column("code", "binary(32)")]; + columns[1].is_nullable = false; + columns[1].extra = Some("computed".to_string()); + let computed = HashMap::from([( + "code".to_string(), + "AS (CONVERT([binary](32),hashbytes('SHA2_256',[id]))) PERSISTED".to_string(), + )]); + + let ddl = render_sqlserver_table_ddl_with_computed("dbo", "authorization", &columns, &computed, &[], &[], None); + + assert!( + ddl.contains("\n [code] AS (CONVERT([binary](32),hashbytes('SHA2_256',[id]))) PERSISTED NOT NULL"), + "computed column keeps its definition: {ddl}" + ); + // The derived result type must not be rendered as a storable column. + assert!(!ddl.contains("[code] binary(32)"), "derived result type must be dropped: {ddl}"); + assert!(ddl.contains("[id] nvarchar(100)"), "plain columns are unchanged: {ddl}"); + } + + #[test] + fn sqlserver_table_ddl_ignores_computed_clauses_for_other_columns() { + let columns = vec![column("id", "int")]; + let computed = HashMap::from([("other".to_string(), "AS (1)".to_string())]); + + let ddl = render_sqlserver_table_ddl_with_computed("dbo", "users", &columns, &computed, &[], &[], None); + + assert_eq!(ddl, render_sqlserver_table_ddl("dbo", "users", &columns, &[], &[], None)); + assert!(ddl.contains("[id] int"), "ddl: {ddl}"); + } + + #[test] + fn sqlserver_table_ddl_skips_blank_computed_clauses() { + let mut columns = vec![column("code", "binary(32)")]; + columns[0].is_nullable = false; + let computed = HashMap::from([("code".to_string(), " ".to_string())]); + + let ddl = render_sqlserver_table_ddl_with_computed("dbo", "users", &columns, &computed, &[], &[], None); + + assert!(ddl.contains("[code] binary(32) NOT NULL"), "blank clause falls back to the type: {ddl}"); + } + #[test] fn sqlserver_table_ddl_includes_column_comments() { let mut display_name = column("display]name", "nvarchar(100)"); @@ -14597,12 +14640,36 @@ pub async fn build_sqlserver_ddl( schema: &str, table: &str, ) -> Result { - let columns = db::sqlserver::get_columns(client, schema, table).await?; + // The computed-column definitions come from the same metadata query as the + // columns, so the DDL path reads the richer driver record instead of the + // flattened `ColumnInfo` (which cannot carry `AS (...) PERSISTED`). + let metadata = db::sqlserver::get_column_metadata(client, schema, table).await?; let indexes = db::sqlserver::list_indexes(client, schema, table).await?; let fkeys = db::sqlserver::list_foreign_keys(client, schema, table).await?; let table_comment = db::sqlserver::get_table_comment(client, schema, table).await?; - Ok(render_sqlserver_table_ddl(schema, table, &columns, &indexes, &fkeys, table_comment.as_deref())) + let columns = metadata.iter().map(|metadata| metadata.column.clone()).collect::>(); + let computed_clauses = metadata + .iter() + .filter_map(|metadata| { + metadata + .computed_clause + .as_deref() + .map(str::trim) + .filter(|clause| !clause.is_empty()) + .map(|clause| (metadata.column.name.clone(), clause.to_string())) + }) + .collect::>(); + + Ok(render_sqlserver_table_ddl_with_computed( + schema, + table, + &columns, + &computed_clauses, + &indexes, + &fkeys, + table_comment.as_deref(), + )) } fn sqlserver_fk_action_clause(kind: &str, value: Option<&str>) -> String { @@ -14623,12 +14690,41 @@ pub fn render_sqlserver_table_ddl( indexes: &[db::IndexInfo], fkeys: &[db::ForeignKeyInfo], table_comment: Option<&str>, +) -> String { + render_sqlserver_table_ddl_with_computed(schema, table, columns, &HashMap::new(), indexes, fkeys, table_comment) +} + +/// Renders a `CREATE TABLE` script for a SQL Server table. `computed_clauses` +/// maps a column name to its `AS (expression) [PERSISTED]` definition: those +/// columns have no storable `data_type` of their own in metadata (only the +/// derived result type), so the definition is written instead of the type. +#[allow(clippy::too_many_arguments)] +pub fn render_sqlserver_table_ddl_with_computed( + schema: &str, + table: &str, + columns: &[db::ColumnInfo], + computed_clauses: &HashMap, + indexes: &[db::IndexInfo], + fkeys: &[db::ForeignKeyInfo], + table_comment: Option<&str>, ) -> String { let table_name = format!("{}.{}", sqlserver_ident(schema), sqlserver_ident(table)); let mut ddl = format!("CREATE TABLE {table_name} (\n"); let col_lines: Vec = columns .iter() .map(|c| { + // A computed column can never have a default, and its `data_type` + // is only the derived result type: rendering either would emit a + // table that differs from the one being scripted. + if let Some(clause) = + computed_clauses.get(&c.name).map(String::as_str).map(str::trim).filter(|clause| !clause.is_empty()) + { + let mut line = format!(" {} {clause}", sqlserver_ident(&c.name)); + if !c.is_nullable { + line.push_str(" NOT NULL"); + } + return line; + } let mut line = format!(" {} {}", sqlserver_ident(&c.name), c.data_type); if let Some(identity) = sqlserver_identity_clause(c.extra.as_deref()) { line.push_str(&format!(" {identity}")); diff --git a/crates/dbx-core/tests/live_sqlserver_computed_column_ddl.rs b/crates/dbx-core/tests/live_sqlserver_computed_column_ddl.rs new file mode 100644 index 0000000000..4d672a655f --- /dev/null +++ b/crates/dbx-core/tests/live_sqlserver_computed_column_ddl.rs @@ -0,0 +1,134 @@ +//! Live regression coverage for issue #10055: a SQL Server computed column +//! (`AS (expression) [PERSISTED]`) used to be scripted as a plain column with +//! its derived result type, so the generated `CREATE TABLE` did not recreate +//! the table. The display DDL has to keep the computed definition, and the +//! emitted script has to replay onto the server. + +use dbx_core::connection::AppState; +use dbx_core::models::connection::{ConnectionConfig, DatabaseType}; +use dbx_core::query::execute_sql_statement; +use dbx_core::storage::Storage; +use std::sync::Arc; + +fn live_sqlserver_config(id: &str, database: &str) -> ConnectionConfig { + let host = std::env::var("DBX_LIVE_SQLSERVER_HOST").unwrap_or_else(|_| "127.0.0.1".to_string()); + let port: u16 = std::env::var("DBX_LIVE_SQLSERVER_PORT").ok().and_then(|value| value.parse().ok()).unwrap_or(1433); + let username = 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"); + + serde_json::from_value::(serde_json::json!({ + "id": id, + "name": id, + "db_type": DatabaseType::SqlServer, + "host": host, + "port": port, + "username": username, + "password": password, + "database": database, + "connect_timeout_secs": 15, + "query_timeout_secs": 30, + "idle_timeout_secs": 60, + "keepalive_interval_secs": 0 + })) + .expect("live SQL Server config should deserialize") +} + +#[tokio::test] +#[ignore = "requires DBX_LIVE_SQLSERVER_HOST/PORT/USER/PASSWORD pointing at a writable SQL Server"] +async fn live_sqlserver_display_ddl_keeps_computed_column_definitions() { + let suffix = uuid::Uuid::new_v4().simple().to_string(); + let connection_id = format!("live-sqlserver-computed-ddl-{suffix}"); + let database = format!("dbx_ddl_computed_{suffix}"); + let restored_database = format!("{database}_replay"); + let dir = std::env::temp_dir().join(format!("dbx-live-sqlserver-computed-ddl-{suffix}")); + std::fs::create_dir_all(&dir).unwrap(); + let storage = Storage::open(&dir.join("storage.db")).await.unwrap(); + let state = Arc::new(AppState::new(storage)); + state.configs.write().await.insert(connection_id.clone(), live_sqlserver_config(&connection_id, "master")); + + let table = format!("authorization_{suffix}"); + let create_table = format!( + "CREATE TABLE [dbo].[{table}] ([id] [nvarchar](100) NOT NULL, [authorization_code_value] [nvarchar](max) NULL, \ + [authorization_code_hash] AS (CONVERT([binary](32), hashbytes('SHA2_256', [authorization_code_value]))) PERSISTED, \ + [id_hash] AS (CONVERT([binary](32), hashbytes('SHA2_256', [id]))) PERSISTED NOT NULL, \ + [name_upper] AS (upper([id])), CONSTRAINT [PK_{table}] PRIMARY KEY CLUSTERED ([id] ASC))" + ); + for sql in [ + format!("CREATE DATABASE [{database}]"), + format!("USE [{database}]; {create_table}"), + format!("CREATE DATABASE [{restored_database}]"), + ] { + execute_sql_statement(&state, &connection_id, "", &sql, None, None) + .await + .unwrap_or_else(|error| panic!("fixture statement failed: {error}")); + } + + let ddl = dbx_core::schema::get_table_display_ddl_core(&state, &connection_id, &database, "dbo", &table, None) + .await + .expect("display DDL for a table owning computed columns"); + + println!("=== display DDL ===\n{ddl}\n==================="); + + // SQL Server normalizes the stored definition (no whitespace from the + // original statement), so the script has to reuse that text. + assert!( + ddl.contains("[authorization_code_hash] AS (CONVERT([binary](32),hashbytes('SHA2_256',[authorization_code_value]))) PERSISTED"), + "persisted computed column keeps its expression: {ddl}" + ); + assert!( + ddl.contains("[id_hash] AS (CONVERT([binary](32),hashbytes('SHA2_256',[id]))) PERSISTED NOT NULL"), + "a NOT NULL computed column keeps both attributes: {ddl}" + ); + assert!(ddl.contains("[name_upper] AS (upper([id]))"), "non-persisted computed column keeps its expression: {ddl}"); + assert!( + !ddl.contains("[authorization_code_hash] binary(32)"), + "the derived result type must not be rendered as the column definition: {ddl}" + ); + + // Replaying the script must recreate both computed columns instead of + // flattening them into their derived result types. + execute_sql_statement(&state, &connection_id, "", &format!("USE [{restored_database}]; {ddl}"), None, None) + .await + .unwrap_or_else(|error| panic!("replayed DDL must execute: {error}")); + + let computed_columns_sql = format!( + "USE [{restored_database}]; SELECT c.name, cc.definition, cc.is_persisted FROM sys.columns c \ + JOIN sys.computed_columns cc ON cc.object_id = c.object_id AND cc.column_id = c.column_id \ + WHERE c.object_id = OBJECT_ID(N'dbo.{table}') ORDER BY c.column_id" + ); + let restored = execute_sql_statement(&state, &connection_id, "", &computed_columns_sql, None, None) + .await + .expect("read the replayed computed columns"); + let restored_rows = restored + .rows + .iter() + .map(|row| { + ( + row[0].as_str().unwrap_or_default().to_string(), + row[1].as_str().unwrap_or_default().to_string(), + row[2].as_bool().unwrap_or(false), + ) + }) + .collect::>(); + + let cleanup_sql = format!( + "USE [master]; ALTER DATABASE [{database}] SET SINGLE_USER WITH ROLLBACK IMMEDIATE; DROP DATABASE [{database}]; \ + ALTER DATABASE [{restored_database}] SET SINGLE_USER WITH ROLLBACK IMMEDIATE; DROP DATABASE [{restored_database}]" + ); + let cleanup = execute_sql_statement(&state, &connection_id, "", &cleanup_sql, None, None).await; + + assert_eq!( + restored_rows, + vec![ + ( + "authorization_code_hash".to_string(), + "(CONVERT([binary](32),hashbytes('SHA2_256',[authorization_code_value])))".to_string(), + true + ), + ("id_hash".to_string(), "(CONVERT([binary](32),hashbytes('SHA2_256',[id])))".to_string(), true), + ("name_upper".to_string(), "(upper([id]))".to_string(), false), + ], + "the replayed DDL recreates both computed columns: {ddl}" + ); + cleanup.expect("drop the live SQL Server computed-column fixture databases"); +} diff --git a/crates/dbx-driver-sqlserver/src/sqlserver.rs b/crates/dbx-driver-sqlserver/src/sqlserver.rs index 393e1e8f3a..bb95ba2a95 100644 --- a/crates/dbx-driver-sqlserver/src/sqlserver.rs +++ b/crates/dbx-driver-sqlserver/src/sqlserver.rs @@ -198,6 +198,11 @@ pub struct SqlServerColumnMetadata { pub is_computed: bool, pub is_hidden: bool, pub generated_always_type: i32, + /// `AS (expression) [PERSISTED]` of a computed column. `column.data_type` + /// only carries the derived result type of those columns, so a rebuilt + /// `CREATE TABLE` has to use this definition instead of the type. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub computed_clause: Option, } #[derive(Debug, Clone, serde::Serialize, serde::Deserialize, PartialEq, Eq)] @@ -2979,6 +2984,12 @@ fn sqlserver_column_metadata_from_row(row: &Row) -> SqlServerColumnMetadata { }, _ => base, }; + let is_computed = row.get::(12).unwrap_or(0) == 1; + let computed_clause = if is_computed { + sqlserver_computed_column_clause(row.get::<&str, _>(15).unwrap_or(""), row.get::(16).unwrap_or(0) == 1) + } else { + None + }; let column = ColumnInfo { name: row.get::<&str, _>(0).unwrap_or("").to_string(), data_type, @@ -2996,12 +3007,30 @@ fn sqlserver_column_metadata_from_row(row: &Row) -> SqlServerColumnMetadata { SqlServerColumnMetadata { column, is_identity: row.get::(11).unwrap_or(0) == 1, - is_computed: row.get::(12).unwrap_or(0) == 1, + is_computed, is_hidden: row.get::(13).unwrap_or(0) == 1, generated_always_type: row.get::(14).unwrap_or(0), + computed_clause, } } +/// SQL Server stores a computed column's definition as the normalized parse +/// tree (`(CONVERT([binary](32),hashbytes('SHA2_256',[c])))`) in +/// `sys.computed_columns`. `CREATE TABLE` needs `AS ()`, so the +/// stored text is reused verbatim when it is already parenthesized (SQL Server +/// always wraps the full expression) and wrapped otherwise. `PERSISTED` is +/// part of the column definition and has to travel with the expression — +/// `data_type` only carries the derived result type, which must not be written +/// into the script. +fn sqlserver_computed_column_clause(definition: &str, is_persisted: bool) -> Option { + let definition = definition.trim(); + if definition.is_empty() { + return None; + } + let expression = if definition.starts_with('(') { definition.to_string() } else { format!("({definition})") }; + Some(if is_persisted { format!("AS {expression} PERSISTED") } else { format!("AS {expression}") }) +} + fn sqlserver_columns_sql(schema: &str, table: &str) -> String { let t = table.replace('\'', "''"); let schema_filter = sqlserver_object_schema_name_predicate(schema, table, "s.name"); @@ -3030,13 +3059,16 @@ fn sqlserver_columns_sql(schema: &str, table: &str) -> String { CONVERT(INT, COLUMNPROPERTY(c.object_id, c.name, 'IsIdentity')) AS IS_IDENTITY, \ CONVERT(INT, COLUMNPROPERTY(c.object_id, c.name, 'IsComputed')) AS IS_COMPUTED, \ CONVERT(INT, COLUMNPROPERTY(c.object_id, c.name, 'IsHidden')) AS IS_HIDDEN, \ - CONVERT(INT, COLUMNPROPERTY(c.object_id, c.name, 'GeneratedAlwaysType')) AS GENERATED_ALWAYS_TYPE \ + CONVERT(INT, COLUMNPROPERTY(c.object_id, c.name, 'GeneratedAlwaysType')) AS GENERATED_ALWAYS_TYPE, \ + cc.definition AS COMPUTED_DEFINITION, \ + CONVERT(INT, ISNULL(cc.is_persisted, 0)) AS IS_PERSISTED \ FROM sys.objects o \ JOIN sys.schemas s ON s.schema_id = o.schema_id \ JOIN sys.columns c ON c.object_id = o.object_id \ JOIN sys.types ty ON ty.user_type_id = c.user_type_id \ LEFT JOIN sys.default_constraints dc ON dc.object_id = c.default_object_id \ LEFT JOIN sys.identity_columns ic ON ic.object_id = c.object_id AND ic.column_id = c.column_id \ + LEFT JOIN sys.computed_columns cc ON cc.object_id = c.object_id AND cc.column_id = c.column_id \ LEFT JOIN ( \ SELECT ic.object_id, ic.column_id \ FROM sys.indexes i \ @@ -3969,18 +4001,18 @@ mod tests { requires_simple_query_batch, restore_sqlserver_blank_column_names, restore_sqlserver_legacy_probe_output_names, restore_sqlserver_spatial_column_types, restore_sqlserver_unsafe_column_types, server_messages_query_result, sqlserver_batch_can_use_execute, sqlserver_bulk_token_row, sqlserver_cell_to_json, sqlserver_columns_sql, - sqlserver_completion_assistant_sql, sqlserver_constraints_sql, sqlserver_dml_output_returns_rows, - sqlserver_done_trace_event, sqlserver_filter_definition_error, sqlserver_foreign_keys_sql, - sqlserver_hidden_schema_names, sqlserver_indexes_sql, sqlserver_legacy_indexes_sql, sqlserver_legacy_probe, - sqlserver_legacy_probe_with_nonce, sqlserver_legacy_wildcard_metadata_query, sqlserver_list_objects_sql, - sqlserver_list_schemas_sql, sqlserver_list_tables_sql, sqlserver_narrow_i32, sqlserver_probe_explicit_alias, - sqlserver_query_messages, sqlserver_query_transport_for_engine_edition, sqlserver_query_transport_for_request, - sqlserver_referential_action, sqlserver_schema_name_predicate, sqlserver_spatial_marker, - sqlserver_split_name_list, sqlserver_supports_session_database_switch, sqlserver_table_comment_sql, - sqlserver_table_objects_sql, sqlserver_triggers_sql, sqlserver_visible_object_predicate, - strip_dbx_sqlserver_row_number_column, SqlServerDescribedColumn, SqlServerProbeOutputNameOverride, - SqlServerQueryTransport, SqlServerRestoredColumn, SqlServerResultSet, SqlServerSpatialColumn, - SqlServerTdsEvent, SQLSERVER_COMPLETION_CONTEXT_SQL, SQLSERVER_RESULT_TYPE_PROBE_SQL, + sqlserver_completion_assistant_sql, sqlserver_computed_column_clause, sqlserver_constraints_sql, + sqlserver_dml_output_returns_rows, sqlserver_done_trace_event, sqlserver_filter_definition_error, + sqlserver_foreign_keys_sql, sqlserver_hidden_schema_names, sqlserver_indexes_sql, sqlserver_legacy_indexes_sql, + sqlserver_legacy_probe, sqlserver_legacy_probe_with_nonce, sqlserver_legacy_wildcard_metadata_query, + sqlserver_list_objects_sql, sqlserver_list_schemas_sql, sqlserver_list_tables_sql, sqlserver_narrow_i32, + sqlserver_probe_explicit_alias, sqlserver_query_messages, sqlserver_query_transport_for_engine_edition, + sqlserver_query_transport_for_request, sqlserver_referential_action, sqlserver_schema_name_predicate, + sqlserver_spatial_marker, sqlserver_split_name_list, sqlserver_supports_session_database_switch, + sqlserver_table_comment_sql, sqlserver_table_objects_sql, sqlserver_triggers_sql, + sqlserver_visible_object_predicate, strip_dbx_sqlserver_row_number_column, SqlServerDescribedColumn, + SqlServerProbeOutputNameOverride, SqlServerQueryTransport, SqlServerRestoredColumn, SqlServerResultSet, + SqlServerSpatialColumn, SqlServerTdsEvent, SQLSERVER_COMPLETION_CONTEXT_SQL, SQLSERVER_RESULT_TYPE_PROBE_SQL, }; use crate::types::{ CompletionAssistantMatchMode, CompletionAssistantObjectKind, CompletionAssistantRequest, QueryResult, @@ -4879,6 +4911,34 @@ mod tests { assert!(sql.contains("c.is_computed = 1 THEN 'computed'")); } + #[test] + fn sqlserver_columns_sql_reads_computed_column_definitions() { + let sql = sqlserver_columns_sql("dbo", "orders"); + + assert!(sql.contains( + "LEFT JOIN sys.computed_columns cc ON cc.object_id = c.object_id AND cc.column_id = c.column_id" + )); + assert!(sql.contains("cc.definition AS COMPUTED_DEFINITION")); + assert!(sql.contains("CONVERT(INT, ISNULL(cc.is_persisted, 0)) AS IS_PERSISTED")); + } + + #[test] + fn sqlserver_computed_column_clause_reuses_the_stored_definition() { + // SQL Server stores the whole expression already wrapped in parentheses. + assert_eq!( + sqlserver_computed_column_clause("(CONVERT([binary](32),hashbytes('SHA2_256',[c])))", true).as_deref(), + Some("AS (CONVERT([binary](32),hashbytes('SHA2_256',[c]))) PERSISTED") + ); + assert_eq!(sqlserver_computed_column_clause("(upper([id]))", false).as_deref(), Some("AS (upper([id]))")); + } + + #[test] + fn sqlserver_computed_column_clause_wraps_a_bare_expression() { + assert_eq!(sqlserver_computed_column_clause(" [a] + [b] ", false).as_deref(), Some("AS ([a] + [b])")); + assert_eq!(sqlserver_computed_column_clause(" ", true), None); + assert_eq!(sqlserver_computed_column_clause("", false), None); + } + #[test] fn sqlserver_columns_sql_exposes_structured_generation_flags() { let sql = sqlserver_columns_sql("dbo", "orders");