fix(sqlserver): tolerate tinyint foreign key referential actions

crates/dbx-core/tests/live_sqlserver_ddl_fk_display.rs

@@ -0,0 +1,87 @@
+//! Live regression coverage for the DDL path of a SQL Server table that owns a
+//! foreign key: `sys.foreign_keys.delete_referential_action` /
+//! `update_referential_action` are `tinyint` and used to be read with
+//! `row.get::<i32>()`, which panicked inside tiberius and aborted the process
+//! (the desktop app crashed on "查看 DDL" and on the hover DDL preview).
+
+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").expect("DBX_LIVE_SQLSERVER_HOST");
+    let port: u16 = std::env::var("DBX_LIVE_SQLSERVER_PORT")
+        .expect("DBX_LIVE_SQLSERVER_PORT")
+        .parse()
+        .expect("valid DBX_LIVE_SQLSERVER_PORT");
+    let username = std::env::var("DBX_LIVE_SQLSERVER_USER").expect("DBX_LIVE_SQLSERVER_USER");
+    let password = std::env::var("DBX_LIVE_SQLSERVER_PASSWORD").expect("DBX_LIVE_SQLSERVER_PASSWORD");
+
+    serde_json::from_value::<ConnectionConfig>(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_reads_foreign_key_actions() {
+    let suffix = uuid::Uuid::new_v4().simple().to_string();
+    let connection_id = format!("live-sqlserver-ddl-fk-{suffix}");
+    let database = format!("dbx_ddl_fk_{suffix}");
+    let dir = std::env::temp_dir().join(format!("dbx-live-sqlserver-ddl-fk-{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 parent = format!("dbx_ddl_parent_{suffix}");
+    let child = format!("dbx_ddl_child_{suffix}");
+    for sql in [
+        format!("CREATE DATABASE [{database}]"),
+        format!("CREATE TABLE [dbo].[{parent}] ([id] INT NOT NULL CONSTRAINT [PK_{parent}] PRIMARY KEY CLUSTERED ([id] ASC))"),
+        format!(
+            "CREATE TABLE [dbo].[{child}] ([id] INT NOT NULL CONSTRAINT [PK_{child}] PRIMARY KEY CLUSTERED ([id] ASC), \
+             [parent_id] INT NULL, CONSTRAINT [FK_{child}_parent] FOREIGN KEY ([parent_id]) \
+             REFERENCES [dbo].[{parent}] ([id]) ON DELETE CASCADE ON UPDATE NO ACTION)"
+        ),
+    ] {
+        let sql = if sql.starts_with("CREATE TABLE") { format!("USE [{database}]; {sql}") } else { sql };
+        execute_sql_statement(&state, &connection_id, "", &sql, None, None)
+            .await
+            .unwrap_or_else(|error| panic!("fixture statement failed: {error}"));
+    }
+
+    // The desktop app calls exactly this path for the DDL tab and the hover preview.
+    let ddl = dbx_core::schema::get_table_display_ddl_core(&state, &connection_id, &database, "dbo", &child, None)
+        .await
+        .expect("display DDL for a table owning a foreign key");
+
+    let cleanup = execute_sql_statement(
+        &state,
+        &connection_id,
+        "",
+        &format!("USE [master]; ALTER DATABASE [{database}] SET SINGLE_USER WITH ROLLBACK IMMEDIATE; DROP DATABASE [{database}]"),
+        None,
+        None,
+    )
+    .await;
+
+    assert!(ddl.contains("FOREIGN KEY ([parent_id])"), "DDL keeps the foreign key: {ddl}");
+    assert!(ddl.contains("ON DELETE CASCADE"), "DDL keeps the delete action: {ddl}");
+    assert!(!ddl.contains("ON UPDATE"), "NO ACTION stays implicit: {ddl}");
+    cleanup.expect("drop the live SQL Server DDL fixture database");
+}

crates/dbx-core/tests/live_sqlserver_foreign_keys.rs

@@ -0,0 +1,92 @@
+use std::time::Duration;
+
+/// `sys.foreign_keys.delete_referential_action` and `update_referential_action` are
+/// `tinyint`, which tiberius decodes as `ColumnData::U8`. Reading them with
+/// `row.get::<i32>()` panicked and aborted the process, so schema comparison crashed
+/// on the first table that owned a foreign key. This covers the live read path.
+#[tokio::test]
+#[ignore = "requires DBX_LIVE_SQLSERVER_HOST/PORT/USER/PASSWORD pointing at a writable SQL Server database"]
+async fn live_sqlserver_foreign_key_referential_actions_are_read_from_tinyint_columns() {
+    let host = std::env::var("DBX_LIVE_SQLSERVER_HOST").expect("DBX_LIVE_SQLSERVER_HOST");
+    let port = std::env::var("DBX_LIVE_SQLSERVER_PORT")
+        .expect("DBX_LIVE_SQLSERVER_PORT")
+        .parse()
+        .expect("valid DBX_LIVE_SQLSERVER_PORT");
+    let user = std::env::var("DBX_LIVE_SQLSERVER_USER").expect("DBX_LIVE_SQLSERVER_USER");
+    let password = std::env::var("DBX_LIVE_SQLSERVER_PASSWORD").expect("DBX_LIVE_SQLSERVER_PASSWORD");
+    let database = std::env::var("DBX_LIVE_SQLSERVER_DATABASE").unwrap_or_else(|_| "tempdb".to_string());
+
+    let mut client =
+        dbx_core::db::sqlserver::connect(&host, port, &user, &password, Some(&database), None, Duration::from_secs(15))
+            .await
+            .expect("connect SQL Server");
+
+    let suffix = uuid::Uuid::new_v4().simple().to_string();
+    let parent_table = format!("dbx_fk_parent_{suffix}");
+    let cascade_child_table = format!("dbx_fk_cascade_child_{suffix}");
+    let set_null_child_table = format!("dbx_fk_setnull_child_{suffix}");
+
+    let cleanup = format!(
+        "DROP TABLE IF EXISTS [dbo].[{cascade_child_table}];\
+         DROP TABLE IF EXISTS [dbo].[{set_null_child_table}];\
+         DROP TABLE IF EXISTS [dbo].[{parent_table}];"
+    );
+    let _ = dbx_core::db::sqlserver::execute_batch(&mut client, &cleanup).await;
+
+    let setup = format!(
+        "CREATE TABLE [dbo].[{parent_table}] (\
+             [id] INT NOT NULL CONSTRAINT [PK_{parent_table}] PRIMARY KEY CLUSTERED ([id] ASC)\
+         );\
+         CREATE TABLE [dbo].[{cascade_child_table}] (\
+             [id] INT NOT NULL CONSTRAINT [PK_{cascade_child_table}] PRIMARY KEY CLUSTERED ([id] ASC),\
+             [parent_id] INT NULL,\
+             CONSTRAINT [FK_{cascade_child_table}_parent] FOREIGN KEY ([parent_id])\
+                 REFERENCES [dbo].[{parent_table}] ([id]) ON DELETE CASCADE ON UPDATE NO ACTION\
+         );\
+         CREATE TABLE [dbo].[{set_null_child_table}] (\
+             [id] INT NOT NULL CONSTRAINT [PK_{set_null_child_table}] PRIMARY KEY CLUSTERED ([id] ASC),\
+             [parent_id] INT NULL,\
+             CONSTRAINT [FK_{set_null_child_table}_parent] FOREIGN KEY ([parent_id])\
+                 REFERENCES [dbo].[{parent_table}] ([id]) ON DELETE SET NULL ON UPDATE SET DEFAULT\
+         );"
+    );
+    dbx_core::db::sqlserver::execute_batch(&mut client, &setup).await.expect("create foreign key fixtures");
+
+    let exercise = async {
+        let cascade = dbx_core::db::sqlserver::list_foreign_keys(&mut client, "dbo", &cascade_child_table).await?;
+        assert_eq!(cascade.len(), 1, "expected one foreign key on the cascade child table");
+        assert_eq!(cascade[0].ref_table, parent_table);
+        assert_eq!(cascade[0].ref_column.as_str(), "id");
+        assert_eq!(cascade[0].column.as_str(), "parent_id");
+        assert_eq!(cascade[0].on_delete.as_deref(), Some("CASCADE"));
+        assert_eq!(cascade[0].on_update, None, "ON UPDATE NO ACTION is the default and stays unset");
+
+        let set_null = dbx_core::db::sqlserver::list_foreign_keys(&mut client, "dbo", &set_null_child_table).await?;
+        assert_eq!(set_null.len(), 1, "expected one foreign key on the set-null child table");
+        assert_eq!(set_null[0].on_delete.as_deref(), Some("SET NULL"));
+        assert_eq!(set_null[0].on_update.as_deref(), Some("SET DEFAULT"));
+
+        let parent = dbx_core::db::sqlserver::list_foreign_keys(&mut client, "dbo", &parent_table).await?;
+        assert!(parent.is_empty(), "parent table owns no outbound foreign keys");
+
+        // Mirror the schema comparison loop, which loads every table's metadata and
+        // DDL one table at a time and used to abort on the first table with a
+        // foreign key.
+        let tables = dbx_core::db::sqlserver::list_tables(&mut client, "dbo", None, None, None).await?;
+        for table in tables.iter().filter(|table| table.table_type.eq_ignore_ascii_case("BASE TABLE")) {
+            dbx_core::db::sqlserver::list_foreign_keys(&mut client, "dbo", &table.name)
+                .await
+                .unwrap_or_else(|error| panic!("list_foreign_keys failed for {}: {error}", table.name));
+            dbx_core::schema::build_sqlserver_ddl(&mut client, "dbo", &table.name)
+                .await
+                .unwrap_or_else(|error| panic!("build_sqlserver_ddl failed for {}: {error}", table.name));
+        }
+
+        Ok::<(), String>(())
+    }
+    .await;
+
+    let cleanup_result = dbx_core::db::sqlserver::execute_batch(&mut client, &cleanup).await;
+    exercise.expect("read referential actions from tinyint catalog columns");
+    cleanup_result.expect("drop foreign key fixtures");
+}

crates/dbx-drivers/src/db/sqlserver.rs

@@ -3015,7 +3015,25 @@ pub async fn list_foreign_keys(
     schema: &str,
     table: &str,
 ) -> Result<Vec<ForeignKeyInfo>, String> {
-    let sql = format!(
+    let sql = sqlserver_foreign_keys_sql(schema, table);
+    let stream = client.query(&*sql, &[]).await.map_err(|e| e.to_string())?;
+    let rows = stream.into_first_result().await.map_err(|e| e.to_string())?;
+    Ok(rows
+        .iter()
+        .map(|row| ForeignKeyInfo {
+            name: row.get::<&str, _>(0).unwrap_or("").to_string(),
+            column: row.get::<&str, _>(1).unwrap_or("").to_string(),
+            ref_schema: Some(row.get::<&str, _>(2).unwrap_or("").to_string()),
+            ref_table: row.get::<&str, _>(3).unwrap_or("").to_string(),
+            ref_column: row.get::<&str, _>(4).unwrap_or("").to_string(),
+            on_update: sqlserver_referential_action_from_row(row, 6),
+            on_delete: sqlserver_referential_action_from_row(row, 5),
+        })
+        .collect())
+}
+
+fn sqlserver_foreign_keys_sql(schema: &str, table: &str) -> String {
+    format!(
         "SELECT fk.name, c.name, SCHEMA_NAME(rt.schema_id), rt.name, rc.name, \
          fk.delete_referential_action, fk.update_referential_action \
          FROM sys.foreign_keys fk \
@@ -3027,21 +3045,36 @@ pub async fn list_foreign_keys(
          ORDER BY fk.name, fkc.constraint_column_id",
         s = schema.replace('\'', "''"),
         t = table.replace('\'', "''")
-    );
-    let stream = client.query(&*sql, &[]).await.map_err(|e| e.to_string())?;
-    let rows = stream.into_first_result().await.map_err(|e| e.to_string())?;
-    Ok(rows
-        .iter()
-        .map(|row| ForeignKeyInfo {
-            name: row.get::<&str, _>(0).unwrap_or("").to_string(),
-            column: row.get::<&str, _>(1).unwrap_or("").to_string(),
-            ref_schema: Some(row.get::<&str, _>(2).unwrap_or("").to_string()),
-            ref_table: row.get::<&str, _>(3).unwrap_or("").to_string(),
-            ref_column: row.get::<&str, _>(4).unwrap_or("").to_string(),
-            on_update: sqlserver_referential_action(row.get::<i32, _>(6).unwrap_or(0)),
-            on_delete: sqlserver_referential_action(row.get::<i32, _>(5).unwrap_or(0)),
-        })
-        .collect())
+    )
+}
+
+/// `sys.foreign_keys.delete_referential_action` and `update_referential_action`
+/// are `tinyint`. tiberius picks the integer width from the received byte length
+/// rather than the declared type, so those columns arrive as `ColumnData::U8` and
+/// `row.get::<i32, _>` panics on the failed conversion (the whole process aborts
+/// under `panic = "abort"`). Read through the narrow integer widths instead.
+fn sqlserver_referential_action_from_row(row: &Row, index: usize) -> Option<String> {
+    sqlserver_referential_action(sqlserver_narrow_i32(
+        row.try_get::<i32, _>(index),
+        row.try_get::<i16, _>(index),
+        row.try_get::<u8, _>(index),
+    ))
+}
+
+/// Collapses the `int` / `smallint` / `tinyint` reads of one column into a single
+/// value. A failed or null conversion falls through to the narrower widths, so
+/// `tinyint` columns (decoded as `U8`) are read correctly instead of panicking.
+fn sqlserver_narrow_i32(
+    as_i32: tiberius::Result<Option<i32>>,
+    as_i16: tiberius::Result<Option<i16>>,
+    as_u8: tiberius::Result<Option<u8>>,
+) -> i32 {
+    as_i32
+        .ok()
+        .flatten()
+        .or_else(|| as_i16.ok().flatten().map(i32::from))
+        .or_else(|| as_u8.ok().flatten().map(i32::from))
+        .unwrap_or(0)
 }
 
 /// sys.foreign_keys referential actions: 0 = NO ACTION (default, omitted),
@@ -3806,17 +3839,17 @@ mod tests {
         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_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_probe_explicit_alias, sqlserver_query_messages,
-        sqlserver_query_transport_for_engine_edition, sqlserver_query_transport_for_request,
-        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_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,
@@ -4658,6 +4691,71 @@ mod tests {
         assert!(!sql.contains("STRING_AGG"));
     }
 
+    #[test]
+    fn sqlserver_foreign_keys_sql_selects_referential_actions_and_escapes_names() {
+        let sql = sqlserver_foreign_keys_sql("d'bo", "t'able");
+
+        assert!(sql.contains("fk.delete_referential_action"));
+        assert!(sql.contains("fk.update_referential_action"));
+        assert!(sql.contains("sys.foreign_keys fk"));
+        assert!(sql.contains("OBJECT_ID('d''bo.t''able')"));
+        assert!(sql.contains("ORDER BY fk.name, fkc.constraint_column_id"));
+    }
+
+    #[test]
+    fn sqlserver_tinyint_referential_actions_decode_as_u8_not_i32() {
+        // sys.foreign_keys.delete_referential_action / update_referential_action
+        // are tinyint, and tiberius derives the integer width from the received
+        // byte length rather than the declared type, so the value arrives as
+        // ColumnData::U8.
+        let tinyint = ColumnData::U8(Some(1));
+
+        assert_eq!(<u8 as tiberius::FromSql>::from_sql(&tinyint).unwrap(), Some(1));
+        // row.get::<i32, _>() panics on this conversion error, which aborts the
+        // release build (`panic = "abort"`), so the reader must tolerate the
+        // narrow widths instead.
+        assert!(<i32 as tiberius::FromSql>::from_sql(&tinyint).is_err());
+        assert!(<i16 as tiberius::FromSql>::from_sql(&tinyint).is_err());
+    }
+
+    #[test]
+    fn sqlserver_narrow_i32_falls_back_to_tinyint_without_panicking() {
+        let conversion_error: tiberius::Result<Option<i32>> =
+            <i32 as tiberius::FromSql>::from_sql(&ColumnData::U8(Some(1)));
+        assert!(conversion_error.is_err());
+
+        // A tinyint CASCADE arrives as U8 while the wider reads fail; the value
+        // must survive instead of aborting the process.
+        let cascade: tiberius::Result<Option<i16>> = <i16 as tiberius::FromSql>::from_sql(&ColumnData::U8(Some(1)));
+        let as_u8: tiberius::Result<Option<u8>> = <u8 as tiberius::FromSql>::from_sql(&ColumnData::U8(Some(1)));
+        assert_eq!(sqlserver_narrow_i32(conversion_error, cascade, as_u8), 1);
+    }
+
+    #[test]
+    fn sqlserver_narrow_i32_prefers_wider_widths_and_defaults_to_zero() {
+        assert_eq!(sqlserver_narrow_i32(Ok(Some(2)), Ok(Some(9)), Ok(Some(9))), 2);
+        assert_eq!(sqlserver_narrow_i32(Ok(None), Ok(Some(3)), Ok(Some(9))), 3);
+        // Null or unreadable values fall back to 0, which maps to no action.
+        assert_eq!(sqlserver_narrow_i32(Ok(None), Ok(None), Ok(None)), 0);
+        assert_eq!(
+            sqlserver_narrow_i32(
+                Err(tiberius::error::Error::Conversion("boom".into())),
+                Err(tiberius::error::Error::Conversion("boom".into())),
+                Err(tiberius::error::Error::Conversion("boom".into()))
+            ),
+            0
+        );
+    }
+
+    #[test]
+    fn sqlserver_referential_action_maps_tinyint_codes() {
+        assert_eq!(sqlserver_referential_action(0), None);
+        assert_eq!(sqlserver_referential_action(1).as_deref(), Some("CASCADE"));
+        assert_eq!(sqlserver_referential_action(2).as_deref(), Some("SET NULL"));
+        assert_eq!(sqlserver_referential_action(3).as_deref(), Some("SET DEFAULT"));
+        assert_eq!(sqlserver_referential_action(4), None);
+    }
+
     #[test]
     fn sqlserver_constraints_sql_unions_every_constraint_catalog() {
         let sql = sqlserver_constraints_sql("d'bo", "t'able");