From e8fe9c3a61a50da6448ab8b08c59490cf6ffcd63 Mon Sep 17 00:00:00 2001 From: David Engel Date: Wed, 12 Aug 2026 00:10:10 +0000 Subject: [PATCH 01/11] odbc: implement SQLColAttributeW (P2) [AB#46579] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the descriptor-field query mssql-python needs to load the driver at all — it resolves SQLColAttributeW and includes the pointer in its non-null check — plus the common fields general ODBC applications use. Values come from the same ColumnMetadata mapping SQLDescribeColW uses (odbc_sql_type / column_size / decimal_digits, now pub(crate)), so the two APIs cannot disagree about a column. Reported fields: concise type and type, length, octet length, precision, scale, nullable, unsigned, display size, count, case sensitive, fixed precision scale, numeric precision radix, unnamed, updatable, auto unique value, searchable, name, label, base column name, and type name. An identifier outside that set is HY091 rather than a silent zero. SQL_DESC_COUNT describes the result set, so it is answered before the column number is validated. String attributes report their length in bytes, per the wide entry point, and truncate with 01004. A null character_attribute_ptr is tolerated because mssql-python passes one and reads only the numeric attribute. sql_variant's SQL_CA_SS_VARIANT_TYPE is deliberately not handled yet; it needs the base type plumbed up from the decoder and lands in a follow-up commit. 499 tests pass, workspace clippy and fmt clean. --- mssql-odbc/src/api/col_attribute.rs | 559 ++++++++++++++++++++++++++++ mssql-odbc/src/api/describe_col.rs | 6 +- mssql-odbc/src/api/exports.rs | 33 ++ mssql-odbc/src/api/mod.rs | 1 + mssql-odbc/src/api/odbc_types.rs | 42 +++ mssql-odbc/src/api/sqlstate.rs | 5 + 6 files changed, 643 insertions(+), 3 deletions(-) create mode 100644 mssql-odbc/src/api/col_attribute.rs diff --git a/mssql-odbc/src/api/col_attribute.rs b/mssql-odbc/src/api/col_attribute.rs new file mode 100644 index 00000000..ae23f313 --- /dev/null +++ b/mssql-odbc/src/api/col_attribute.rs @@ -0,0 +1,559 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Implementation of SQLColAttributeW. +//! +//! Field values come from the same `ColumnMetadata` mapping `SQLDescribeColW` +//! uses, so the two APIs cannot report different types for the same column. + +use mssql_tds::datatypes::sqldatatypes::TdsDataType; +use mssql_tds::query::metadata::ColumnMetadata; +use tracing::{debug, error}; + +use crate::api::describe_col::{column_size, decimal_digits, odbc_sql_type}; +use crate::api::odbc_types::{ + SQL_ATTR_READWRITE_UNKNOWN, SQL_DESC_AUTO_UNIQUE_VALUE, SQL_DESC_BASE_COLUMN_NAME, + SQL_DESC_CASE_SENSITIVE, SQL_DESC_CONCISE_TYPE, SQL_DESC_COUNT, SQL_DESC_DISPLAY_SIZE, + SQL_DESC_FIXED_PREC_SCALE, SQL_DESC_LABEL, SQL_DESC_LENGTH, SQL_DESC_NAME, SQL_DESC_NULLABLE, + SQL_DESC_NUM_PREC_RADIX, SQL_DESC_OCTET_LENGTH, SQL_DESC_PRECISION, SQL_DESC_SCALE, + SQL_DESC_SEARCHABLE, SQL_DESC_TYPE, SQL_DESC_TYPE_NAME, SQL_DESC_UNNAMED, SQL_DESC_UNSIGNED, + SQL_DESC_UPDATABLE, SQL_ERROR, SQL_INVALID_HANDLE, SQL_NAMED, SQL_NO_NULLS, SQL_NULLABLE, + SQL_PRED_SEARCHABLE, SQL_SUCCESS, SQL_SUCCESS_WITH_INFO, SQL_UNNAMED, SqlHandle, SqlLen, + SqlPointer, SqlReturn, SqlSmallInt, SqlUSmallInt, SqlWChar, +}; +use crate::api::sqlstate::{ + ERR_FUNCTION_SEQUENCE, ERR_INVALID_DESCRIPTOR_FIELD, ERR_INVALID_DESCRIPTOR_INDEX, + ERR_STRING_RIGHT_TRUNCATION, post_diag, +}; +use crate::api::util::{copy_with_nul, write_if_some}; +use crate::error::free_errors; +use crate::handles::stmt::STMT_STATE_EXEC_CONTEXT; +use crate::handles::{HandleType, StmtHandle, handle_from_raw}; + +#[allow(clippy::too_many_arguments)] +pub(crate) unsafe fn sql_col_attribute_w( + statement_handle: SqlHandle, + column_number: SqlUSmallInt, + field_identifier: SqlUSmallInt, + character_attribute_ptr: SqlPointer, + buffer_length: SqlSmallInt, + string_length_ptr: *mut SqlSmallInt, + numeric_attribute_ptr: *mut SqlLen, +) -> SqlReturn { + debug!( + ?statement_handle, + column_number, + field_identifier, + ?character_attribute_ptr, + buffer_length, + ?string_length_ptr, + ?numeric_attribute_ptr, + "SQLColAttributeW called", + ); + + crate::ffi_entry!("SQLColAttributeW", unsafe { + sql_col_attribute_w_impl( + statement_handle, + column_number, + field_identifier, + character_attribute_ptr, + buffer_length, + string_length_ptr, + numeric_attribute_ptr, + ) + }) +} + +#[allow(clippy::too_many_arguments)] +unsafe fn sql_col_attribute_w_impl( + statement_handle: SqlHandle, + column_number: SqlUSmallInt, + field_identifier: SqlUSmallInt, + character_attribute_ptr: SqlPointer, + buffer_length: SqlSmallInt, + string_length_ptr: *mut SqlSmallInt, + numeric_attribute_ptr: *mut SqlLen, +) -> SqlReturn { + if statement_handle.is_null() { + error!("SQLColAttributeW: statement_handle is null"); + return SQL_INVALID_HANDLE; + } + + let stmt = unsafe { handle_from_raw::(statement_handle) }; + debug_assert_eq!( + stmt.object_type, + HandleType::Stmt, + "SQLColAttributeW: handle is not a STMT" + ); + + sql_col_attribute_w_safe( + stmt, + column_number, + field_identifier, + character_attribute_ptr, + buffer_length, + string_length_ptr, + numeric_attribute_ptr, + ) +} + +/// Which output parameter a field identifier writes to. +enum Attr { + Numeric(SqlLen), + Text(String), +} + +#[allow(clippy::too_many_arguments)] +fn sql_col_attribute_w_safe( + stmt: &StmtHandle, + column_number: SqlUSmallInt, + field_identifier: SqlUSmallInt, + character_attribute_ptr: SqlPointer, + buffer_length: SqlSmallInt, + string_length_ptr: *mut SqlSmallInt, + numeric_attribute_ptr: *mut SqlLen, +) -> SqlReturn { + let Ok(mut stmt_state) = stmt.inner.lock() else { + error!("SQLColAttributeW: stmt mutex poisoned"); + return SQL_ERROR; + }; + + free_errors(&mut stmt_state); + + if !stmt_state.has_state(STMT_STATE_EXEC_CONTEXT) { + post_diag(&mut stmt_state, ERR_FUNCTION_SEQUENCE); + return SQL_ERROR; + } + + // SQL_DESC_COUNT describes the result set, not a column, so it is answered + // before the column number is validated. + if field_identifier == SQL_DESC_COUNT { + let count = SqlLen::try_from(stmt_state.column_metadata.len()).unwrap_or(SqlLen::MAX); + unsafe { write_if_some(numeric_attribute_ptr, count) }; + return SQL_SUCCESS; + } + + if column_number == 0 || column_number as usize > stmt_state.column_metadata.len() { + post_diag(&mut stmt_state, ERR_INVALID_DESCRIPTOR_INDEX); + return SQL_ERROR; + } + + let meta = &stmt_state.column_metadata[(column_number - 1) as usize]; + let Some(attr) = column_attribute(meta, field_identifier) else { + post_diag(&mut stmt_state, ERR_INVALID_DESCRIPTOR_FIELD); + return SQL_ERROR; + }; + + match attr { + Attr::Numeric(v) => { + unsafe { write_if_some(numeric_attribute_ptr, v) }; + SQL_SUCCESS + } + Attr::Text(s) => { + let utf16: Vec = s.encode_utf16().collect(); + // StringLengthPtr is in bytes for the wide entry point, and excludes + // the terminator. + let byte_len = SqlSmallInt::try_from(utf16.len() * std::mem::size_of::()) + .unwrap_or(SqlSmallInt::MAX); + unsafe { write_if_some(string_length_ptr, byte_len) }; + + let buf_elements = if buffer_length > 0 { + (buffer_length as usize) / std::mem::size_of::() + } else { + 0 + }; + let truncated = unsafe { + copy_with_nul( + character_attribute_ptr as *mut SqlWChar, + buf_elements, + &utf16, + ) + }; + if truncated { + post_diag(&mut stmt_state, ERR_STRING_RIGHT_TRUNCATION); + SQL_SUCCESS_WITH_INFO + } else { + SQL_SUCCESS + } + } + } +} + +/// Maps a field identifier to its value, or `None` when the field is not one +/// this driver reports. +fn column_attribute(meta: &ColumnMetadata, field_identifier: SqlUSmallInt) -> Option { + let sql_type = odbc_sql_type(meta); + let attr = match field_identifier { + // `SQL_DESC_TYPE` and `SQL_DESC_CONCISE_TYPE` differ only for the + // datetime/interval types, which this driver reports as concise types. + SQL_DESC_TYPE | SQL_DESC_CONCISE_TYPE => Attr::Numeric(SqlLen::from(sql_type)), + SQL_DESC_LENGTH | SQL_DESC_DISPLAY_SIZE => { + Attr::Numeric(SqlLen::try_from(column_size(meta)).unwrap_or(SqlLen::MAX)) + } + SQL_DESC_OCTET_LENGTH => Attr::Numeric(octet_length(meta)), + SQL_DESC_PRECISION => Attr::Numeric(SqlLen::from(precision(meta))), + SQL_DESC_SCALE => Attr::Numeric(SqlLen::from(decimal_digits(meta))), + SQL_DESC_NULLABLE => Attr::Numeric(SqlLen::from(if meta.is_nullable() { + SQL_NULLABLE + } else { + SQL_NO_NULLS + })), + // The boolean attributes are SQL_TRUE (1) / SQL_FALSE (0), which is what + // `bool` converts to. + SQL_DESC_UNSIGNED => Attr::Numeric(SqlLen::from(is_unsigned(meta))), + SQL_DESC_CASE_SENSITIVE => Attr::Numeric(SqlLen::from(meta.is_case_sensitive())), + SQL_DESC_FIXED_PREC_SCALE => Attr::Numeric(SqlLen::from(matches!( + meta.data_type, + TdsDataType::Money | TdsDataType::Money4 | TdsDataType::MoneyN + ))), + SQL_DESC_NUM_PREC_RADIX => Attr::Numeric(num_prec_radix(meta)), + SQL_DESC_UNNAMED => Attr::Numeric(if meta.column_name.is_empty() { + SQL_UNNAMED + } else { + SQL_NAMED + }), + // The result set is not known to be updatable, and no column here is a + // known auto-increment column; report the "unknown"/false forms rather + // than claiming either way. + SQL_DESC_UPDATABLE => Attr::Numeric(SQL_ATTR_READWRITE_UNKNOWN), + SQL_DESC_AUTO_UNIQUE_VALUE => Attr::Numeric(SqlLen::from(false)), + SQL_DESC_SEARCHABLE => Attr::Numeric(SQL_PRED_SEARCHABLE), + SQL_DESC_NAME | SQL_DESC_LABEL | SQL_DESC_BASE_COLUMN_NAME => { + Attr::Text(meta.column_name.clone()) + } + SQL_DESC_TYPE_NAME => Attr::Text(type_name(meta).to_string()), + _ => return None, + }; + Some(attr) +} + +/// Storage size in bytes of the column's value on the wire. +fn octet_length(meta: &ColumnMetadata) -> SqlLen { + if meta.is_plp() { + return 0; + } + // `type_info.length` is already a byte count for every type, including the + // national character types. + SqlLen::try_from(meta.type_info.length).unwrap_or(SqlLen::MAX) +} + +/// `SQL_DESC_PRECISION`: the number of significant digits for the exact and +/// approximate numeric types, otherwise the column size. +fn precision(meta: &ColumnMetadata) -> SqlSmallInt { + if let Some(p) = meta.get_precision() { + return SqlSmallInt::from(p); + } + SqlSmallInt::try_from(column_size(meta)).unwrap_or(SqlSmallInt::MAX) +} + +fn num_prec_radix(meta: &ColumnMetadata) -> SqlLen { + match meta.data_type { + TdsDataType::Flt4 | TdsDataType::Flt8 | TdsDataType::FltN => 2, + TdsDataType::Int1 + | TdsDataType::Int2 + | TdsDataType::Int4 + | TdsDataType::Int8 + | TdsDataType::IntN + | TdsDataType::Decimal + | TdsDataType::DecimalN + | TdsDataType::Numeric + | TdsDataType::NumericN + | TdsDataType::Money + | TdsDataType::Money4 + | TdsDataType::MoneyN => 10, + // Non-numeric columns have no radix. + _ => 0, + } +} + +/// `tinyint` is the only unsigned integer SQL Server exposes. +fn is_unsigned(meta: &ColumnMetadata) -> bool { + match meta.data_type { + TdsDataType::Int1 => true, + TdsDataType::IntN => meta.type_info.length == 1, + _ => false, + } +} + +fn type_name(meta: &ColumnMetadata) -> &'static str { + match meta.data_type { + TdsDataType::Int1 => "tinyint", + TdsDataType::Int2 => "smallint", + TdsDataType::Int4 => "int", + TdsDataType::Int8 => "bigint", + TdsDataType::IntN => match meta.type_info.length { + 1 => "tinyint", + 2 => "smallint", + 4 => "int", + 8 => "bigint", + _ => "int", + }, + TdsDataType::Bit | TdsDataType::BitN => "bit", + TdsDataType::Flt4 => "real", + TdsDataType::Flt8 => "float", + TdsDataType::FltN => { + if meta.type_info.length == 4 { + "real" + } else { + "float" + } + } + TdsDataType::Decimal | TdsDataType::DecimalN => "decimal", + TdsDataType::Numeric | TdsDataType::NumericN => "numeric", + TdsDataType::Money | TdsDataType::MoneyN => "money", + TdsDataType::Money4 => "smallmoney", + TdsDataType::DateN => "date", + TdsDataType::TimeN => "time", + TdsDataType::DateTime | TdsDataType::DateTimeN => "datetime", + TdsDataType::DateTim4 => "smalldatetime", + TdsDataType::DateTime2N => "datetime2", + TdsDataType::DateTimeOffsetN => "datetimeoffset", + TdsDataType::Char | TdsDataType::BigChar => "char", + TdsDataType::VarChar | TdsDataType::BigVarChar => "varchar", + TdsDataType::Text => "text", + TdsDataType::NChar => "nchar", + TdsDataType::NVarChar => "nvarchar", + TdsDataType::NText => "ntext", + TdsDataType::Binary | TdsDataType::BigBinary => "binary", + TdsDataType::VarBinary | TdsDataType::BigVarBinary => "varbinary", + TdsDataType::Image => "image", + TdsDataType::Guid => "uniqueidentifier", + TdsDataType::Xml => "xml", + TdsDataType::Json => "json", + TdsDataType::Vector => "vector", + TdsDataType::SsVariant => "sql_variant", + TdsDataType::Udt => "udt", + _ => "unknown", + } +} + +// Only `int` column metadata can be built outside the decoder (`int_columns`), +// so the per-type mapping tables are covered end-to-end by +// `tests/e2e/tests/col_attribute_test.cpp` against a live SQL Server. +#[cfg(test)] +mod tests { + use std::ptr; + + use mssql_tds::test_client_support::int_columns; + + use super::*; + use crate::api::odbc_types::{SQL_INTEGER, SQL_NULLABLE}; + use crate::api::sqlstate::ERR_INVALID_DESCRIPTOR_FIELD; + use crate::test_support::TestHandles; + + /// A statement positioned on a result set of `n` nullable `int` columns. + fn stmt_with_int_columns(h: &TestHandles, n: usize) { + let stmt_handle = unsafe { handle_from_raw::(h.stmt) }; + let mut s = stmt_handle.inner.lock().unwrap(); + s.set_state(STMT_STATE_EXEC_CONTEXT); + s.column_metadata = int_columns(n); + } + + /// Reads a numeric attribute, asserting the call succeeded. + fn numeric(h: &TestHandles, col: SqlUSmallInt, field: SqlUSmallInt) -> SqlLen { + let mut out: SqlLen = -1; + let rc = unsafe { + sql_col_attribute_w( + h.stmt, + col, + field, + ptr::null_mut(), + 0, + ptr::null_mut(), + &mut out, + ) + }; + assert_eq!(rc, SQL_SUCCESS, "field {field}"); + out + } + + #[test] + fn null_handle_returns_invalid_handle() { + let mut out: SqlLen = 0; + let rc = unsafe { + sql_col_attribute_w( + ptr::null_mut(), + 1, + SQL_DESC_CONCISE_TYPE, + ptr::null_mut(), + 0, + ptr::null_mut(), + &mut out, + ) + }; + assert_eq!(rc, SQL_INVALID_HANDLE); + } + + #[test] + fn fresh_stmt_returns_sequence_error() { + let h = TestHandles::with_env_dbc_stmt(); + let mut out: SqlLen = 0; + let rc = unsafe { + sql_col_attribute_w( + h.stmt, + 1, + SQL_DESC_CONCISE_TYPE, + ptr::null_mut(), + 0, + ptr::null_mut(), + &mut out, + ) + }; + assert_eq!(rc, SQL_ERROR); + let sh = unsafe { handle_from_raw::(h.stmt) }; + let s = sh.inner.lock().unwrap(); + assert_eq!( + s.diag_records.last().unwrap().sql_state, + ERR_FUNCTION_SEQUENCE.state + ); + } + + #[test] + fn column_out_of_range_is_invalid_descriptor_index() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 2); + for col in [0, 3] { + let mut out: SqlLen = 0; + let rc = unsafe { + sql_col_attribute_w( + h.stmt, + col, + SQL_DESC_CONCISE_TYPE, + ptr::null_mut(), + 0, + ptr::null_mut(), + &mut out, + ) + }; + assert_eq!(rc, SQL_ERROR, "column {col}"); + let sh = unsafe { handle_from_raw::(h.stmt) }; + let s = sh.inner.lock().unwrap(); + assert_eq!( + s.diag_records.last().unwrap().sql_state, + ERR_INVALID_DESCRIPTOR_INDEX.state + ); + } + } + + /// An identifier this driver does not report is HY091, not a silent zero. + #[test] + fn unknown_field_identifier_is_invalid_descriptor_field() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 1); + let mut out: SqlLen = 0; + let rc = unsafe { + sql_col_attribute_w( + h.stmt, + 1, + 9999, + ptr::null_mut(), + 0, + ptr::null_mut(), + &mut out, + ) + }; + assert_eq!(rc, SQL_ERROR); + let sh = unsafe { handle_from_raw::(h.stmt) }; + let s = sh.inner.lock().unwrap(); + assert_eq!( + s.diag_records.last().unwrap().sql_state, + ERR_INVALID_DESCRIPTOR_FIELD.state + ); + } + + /// SQL_DESC_COUNT describes the result set, so it answers even for a column + /// number that would otherwise be out of range. + #[test] + fn desc_count_ignores_column_number() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 3); + assert_eq!(numeric(&h, 0, SQL_DESC_COUNT), 3); + assert_eq!(numeric(&h, 99, SQL_DESC_COUNT), 3); + } + + #[test] + fn int_column_numeric_attributes() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 2); + assert_eq!( + numeric(&h, 1, SQL_DESC_CONCISE_TYPE), + SqlLen::from(SQL_INTEGER) + ); + assert_eq!(numeric(&h, 1, SQL_DESC_TYPE), SqlLen::from(SQL_INTEGER)); + assert_eq!( + numeric(&h, 1, SQL_DESC_NULLABLE), + SqlLen::from(SQL_NULLABLE) + ); + // `int` is signed, and base 10. + assert_eq!(numeric(&h, 1, SQL_DESC_UNSIGNED), 0); + assert_eq!(numeric(&h, 1, SQL_DESC_NUM_PREC_RADIX), 10); + assert_eq!(numeric(&h, 1, SQL_DESC_UNNAMED), SQL_NAMED); + } + + /// The wide entry point reports the name length in bytes, and a short buffer + /// truncates with 01004 rather than failing. + #[test] + fn name_is_written_as_utf16_with_byte_length() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 1); + + let mut buf = [0u16; 16]; + let mut len: SqlSmallInt = 0; + let rc = unsafe { + sql_col_attribute_w( + h.stmt, + 1, + SQL_DESC_NAME, + buf.as_mut_ptr() as SqlPointer, + (buf.len() * 2) as SqlSmallInt, + &mut len, + ptr::null_mut(), + ) + }; + assert_eq!(rc, SQL_SUCCESS); + // "c1" is two characters, so four bytes. + assert_eq!(len, 4); + let name = String::from_utf16_lossy(&buf[..2]); + assert_eq!(name, "c1"); + + let mut small = [0u16; 2]; + let rc = unsafe { + sql_col_attribute_w( + h.stmt, + 1, + SQL_DESC_NAME, + small.as_mut_ptr() as SqlPointer, + (small.len() * 2) as SqlSmallInt, + &mut len, + ptr::null_mut(), + ) + }; + assert_eq!(rc, SQL_SUCCESS_WITH_INFO); + let sh = unsafe { handle_from_raw::(h.stmt) }; + let s = sh.inner.lock().unwrap(); + assert_eq!( + s.diag_records.last().unwrap().sql_state, + ERR_STRING_RIGHT_TRUNCATION.state + ); + } + + /// mssql-python passes a null string buffer and reads only the numeric + /// attribute, so a null `character_attribute_ptr` must not fault. + #[test] + fn null_output_pointers_are_tolerated() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 1); + let rc = unsafe { + sql_col_attribute_w( + h.stmt, + 1, + SQL_DESC_NAME, + ptr::null_mut(), + 0, + ptr::null_mut(), + ptr::null_mut(), + ) + }; + assert_eq!(rc, SQL_SUCCESS); + } +} diff --git a/mssql-odbc/src/api/describe_col.rs b/mssql-odbc/src/api/describe_col.rs index 9f22243d..50a835f5 100644 --- a/mssql-odbc/src/api/describe_col.rs +++ b/mssql-odbc/src/api/describe_col.rs @@ -161,7 +161,7 @@ fn sql_describe_col_w_safe( } } -fn odbc_sql_type(meta: &mssql_tds::query::metadata::ColumnMetadata) -> SqlSmallInt { +pub(crate) fn odbc_sql_type(meta: &mssql_tds::query::metadata::ColumnMetadata) -> SqlSmallInt { match meta.data_type { TdsDataType::Int1 => SQL_TINYINT, TdsDataType::Int2 => SQL_SMALLINT, @@ -215,7 +215,7 @@ fn odbc_sql_type(meta: &mssql_tds::query::metadata::ColumnMetadata) -> SqlSmallI } } -fn column_size(meta: &mssql_tds::query::metadata::ColumnMetadata) -> u64 { +pub(crate) fn column_size(meta: &mssql_tds::query::metadata::ColumnMetadata) -> u64 { // PLP / `*(max)` / xml / json: ColumnSize is "unbounded". Report 0 per ODBC spec if meta.is_plp() { return 0; @@ -282,7 +282,7 @@ fn column_size(meta: &mssql_tds::query::metadata::ColumnMetadata) -> u64 { } } -fn decimal_digits(meta: &mssql_tds::query::metadata::ColumnMetadata) -> SqlSmallInt { +pub(crate) fn decimal_digits(meta: &mssql_tds::query::metadata::ColumnMetadata) -> SqlSmallInt { match meta.data_type { // T-SQL `money` and `smallmoney` both have a fixed scale of 4. They are stored // as FixedLen/VarLen variants without a scale field, so `get_scale()` returns diff --git a/mssql-odbc/src/api/exports.rs b/mssql-odbc/src/api/exports.rs index efaf4dd3..badad144 100644 --- a/mssql-odbc/src/api/exports.rs +++ b/mssql-odbc/src/api/exports.rs @@ -564,6 +564,39 @@ pub unsafe extern "C" fn SQLDescribeColW( } } +/// Gets a descriptor field for a result set column. +/// +/// # Safety +/// - `statement_handle` must be a valid STMT handle. +/// - `column_number` must be a valid column index (1-based), except for +/// `SQL_DESC_COUNT`, which describes the result set. +/// - `character_attribute_ptr` must be writable for `buffer_length` bytes when +/// non-null; `string_length_ptr` and `numeric_attribute_ptr` must be null or +/// writable for their types. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn SQLColAttributeW( + statement_handle: SqlHandle, + column_number: SqlUSmallInt, + field_identifier: SqlUSmallInt, + character_attribute_ptr: SqlPointer, + buffer_length: SqlSmallInt, + string_length_ptr: *mut SqlSmallInt, + numeric_attribute_ptr: *mut SqlLen, +) -> SqlReturn { + crate::init_tracing(); + unsafe { + super::col_attribute::sql_col_attribute_w( + statement_handle, + column_number, + field_identifier, + character_attribute_ptr, + buffer_length, + string_length_ptr, + numeric_attribute_ptr, + ) + } +} + /// Retrieves data for a single column in the current fetched row. /// /// # Safety diff --git a/mssql-odbc/src/api/mod.rs b/mssql-odbc/src/api/mod.rs index b51a87f3..46b24b4f 100644 --- a/mssql-odbc/src/api/mod.rs +++ b/mssql-odbc/src/api/mod.rs @@ -4,6 +4,7 @@ pub(crate) mod alloc_handle; mod bind_param; mod close_cursor; +mod col_attribute; mod connect; mod describe_col; mod disconnect; diff --git a/mssql-odbc/src/api/odbc_types.rs b/mssql-odbc/src/api/odbc_types.rs index b4e9742e..8fc27c50 100644 --- a/mssql-odbc/src/api/odbc_types.rs +++ b/mssql-odbc/src/api/odbc_types.rs @@ -344,6 +344,48 @@ pub const SQL_C_SS_TIMESTAMPOFFSET: SqlSmallInt = SQL_C_TYPES_EXTENDED + 1; // 0 // sql_variant probe. pub const SQL_CA_SS_VARIANT_TYPE: SqlUSmallInt = 1215; +// ---- SQLColAttribute field identifiers -------------------------------------- +// The low-numbered ids are the ODBC 3.x descriptor fields that double as the +// ODBC 2.x `SQL_COLUMN_*` ids; the 100x block is descriptor-header/record only. +pub const SQL_DESC_CONCISE_TYPE: SqlUSmallInt = 2; +pub const SQL_DESC_DISPLAY_SIZE: SqlUSmallInt = 6; +pub const SQL_DESC_UNSIGNED: SqlUSmallInt = 8; +pub const SQL_DESC_FIXED_PREC_SCALE: SqlUSmallInt = 9; +pub const SQL_DESC_UPDATABLE: SqlUSmallInt = 10; +pub const SQL_DESC_AUTO_UNIQUE_VALUE: SqlUSmallInt = 11; +pub const SQL_DESC_CASE_SENSITIVE: SqlUSmallInt = 12; +pub const SQL_DESC_SEARCHABLE: SqlUSmallInt = 13; +pub const SQL_DESC_TYPE_NAME: SqlUSmallInt = 14; +pub const SQL_DESC_TABLE_NAME: SqlUSmallInt = 15; +pub const SQL_DESC_SCHEMA_NAME: SqlUSmallInt = 16; +pub const SQL_DESC_CATALOG_NAME: SqlUSmallInt = 17; +pub const SQL_DESC_LABEL: SqlUSmallInt = 18; +pub const SQL_DESC_BASE_COLUMN_NAME: SqlUSmallInt = 22; +pub const SQL_DESC_BASE_TABLE_NAME: SqlUSmallInt = 23; +pub const SQL_DESC_NUM_PREC_RADIX: SqlUSmallInt = 32; +pub const SQL_DESC_COUNT: SqlUSmallInt = 1001; +pub const SQL_DESC_TYPE: SqlUSmallInt = 1002; +pub const SQL_DESC_LENGTH: SqlUSmallInt = 1003; +pub const SQL_DESC_PRECISION: SqlUSmallInt = 1005; +pub const SQL_DESC_SCALE: SqlUSmallInt = 1006; +pub const SQL_DESC_NULLABLE: SqlUSmallInt = 1008; +pub const SQL_DESC_NAME: SqlUSmallInt = 1011; +pub const SQL_DESC_UNNAMED: SqlUSmallInt = 1012; +pub const SQL_DESC_OCTET_LENGTH: SqlUSmallInt = 1013; + +// ---- SQLColAttribute value constants ---------------------------------------- +pub const SQL_NULLABLE_UNKNOWN: SqlLen = 2; +pub const SQL_NAMED: SqlLen = 0; +pub const SQL_UNNAMED: SqlLen = 1; +// SQL_DESC_SEARCHABLE +pub const SQL_PRED_NONE: SqlLen = 0; +pub const SQL_PRED_CHAR: SqlLen = 1; +pub const SQL_PRED_BASIC: SqlLen = 2; +pub const SQL_PRED_SEARCHABLE: SqlLen = 3; +// SQL_DESC_UPDATABLE +pub const SQL_ATTR_READONLY: SqlLen = 0; +pub const SQL_ATTR_READWRITE_UNKNOWN: SqlLen = 2; + // ---- Statement attribute identifiers (SQLSetStmtAttr / SQLGetStmtAttr) ------ pub const SQL_ATTR_ROW_BIND_TYPE: SqlInteger = 5; pub const SQL_ATTR_CURSOR_TYPE: SqlInteger = 6; diff --git a/mssql-odbc/src/api/sqlstate.rs b/mssql-odbc/src/api/sqlstate.rs index d3edf015..ab48004a 100644 --- a/mssql-odbc/src/api/sqlstate.rs +++ b/mssql-odbc/src/api/sqlstate.rs @@ -34,6 +34,7 @@ pub(crate) const SQLSTATE_HY010: [u8; 5] = *b"HY010"; pub(crate) const SQLSTATE_HY011: [u8; 5] = *b"HY011"; pub(crate) const SQLSTATE_HY024: [u8; 5] = *b"HY024"; pub(crate) const SQLSTATE_HY090: [u8; 5] = *b"HY090"; +pub(crate) const SQLSTATE_HY091: [u8; 5] = *b"HY091"; pub(crate) const SQLSTATE_HY092: [u8; 5] = *b"HY092"; pub(crate) const SQLSTATE_HY096: [u8; 5] = *b"HY096"; pub(crate) const SQLSTATE_HY110: [u8; 5] = *b"HY110"; @@ -77,6 +78,10 @@ pub(crate) const ERR_INVALID_DESCRIPTOR_INDEX: DiagMsg = DiagMsg { state: SQLSTATE_07009, text: "Invalid descriptor index", }; +pub(crate) const ERR_INVALID_DESCRIPTOR_FIELD: DiagMsg = DiagMsg { + state: SQLSTATE_HY091, + text: "Invalid descriptor field identifier", +}; pub(crate) const ERR_UNBOUND_PARAMETER: DiagMsg = DiagMsg { state: SQLSTATE_07002, text: "COUNT field incorrect or syntax error", From 673aa4d681f6c77d83b99895703a4443bd244f23 Mon Sep 17 00:00:00 2001 From: David Engel Date: Wed, 12 Aug 2026 00:11:21 +0000 Subject: [PATCH 02/11] odbc: report sql_variant columns as SQL_SS_VARIANT SQLDescribeCol mapped sql_variant to SQL_VARCHAR, so an application could not tell a variant column from character data. mssql-python branches on `dataType == SQL_SS_VARIANT` before it will probe the column and query SQL_CA_SS_VARIANT_TYPE, so against this driver it never entered that path and treated every variant column as a string. Behaviour change: applications that keyed on SQL_VARCHAR for these columns now see SQL_SS_VARIANT (-150), which is what msodbcsql reports. Vector and UDT columns are left as they were; whether UDT should report SQL_SS_UDT is a separate question. 499 tests pass, workspace clippy and fmt clean. --- mssql-odbc/src/api/describe_col.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/mssql-odbc/src/api/describe_col.rs b/mssql-odbc/src/api/describe_col.rs index 50a835f5..09aace28 100644 --- a/mssql-odbc/src/api/describe_col.rs +++ b/mssql-odbc/src/api/describe_col.rs @@ -9,10 +9,10 @@ use tracing::{debug, error}; use crate::api::odbc_types::{ SQL_BIGINT, SQL_BINARY, SQL_BIT, SQL_CHAR, SQL_DECIMAL, SQL_DOUBLE, SQL_ERROR, SQL_GUID, SQL_INTEGER, SQL_INVALID_HANDLE, SQL_LONGVARBINARY, SQL_LONGVARCHAR, SQL_NO_NULLS, - SQL_NULLABLE, SQL_REAL, SQL_SMALLINT, SQL_SS_TIME2, SQL_SS_TIMESTAMPOFFSET, SQL_SUCCESS, - SQL_SUCCESS_WITH_INFO, SQL_TINYINT, SQL_TYPE_DATE, SQL_TYPE_TIMESTAMP, SQL_UNKNOWN_TYPE, - SQL_VARBINARY, SQL_VARCHAR, SQL_WCHAR, SQL_WLONGVARCHAR, SQL_WVARCHAR, SqlHandle, SqlReturn, - SqlSmallInt, SqlUSmallInt, SqlWChar, + SQL_NULLABLE, SQL_REAL, SQL_SMALLINT, SQL_SS_TIME2, SQL_SS_TIMESTAMPOFFSET, SQL_SS_VARIANT, + SQL_SUCCESS, SQL_SUCCESS_WITH_INFO, SQL_TINYINT, SQL_TYPE_DATE, SQL_TYPE_TIMESTAMP, + SQL_UNKNOWN_TYPE, SQL_VARBINARY, SQL_VARCHAR, SQL_WCHAR, SQL_WLONGVARCHAR, SQL_WVARCHAR, + SqlHandle, SqlReturn, SqlSmallInt, SqlUSmallInt, SqlWChar, }; use crate::api::sqlstate::{ ERR_FUNCTION_SEQUENCE, ERR_INVALID_DESCRIPTOR_INDEX, ERR_STRING_RIGHT_TRUNCATION, post_diag, @@ -210,7 +210,10 @@ pub(crate) fn odbc_sql_type(meta: &mssql_tds::query::metadata::ColumnMetadata) - TdsDataType::Image => SQL_LONGVARBINARY, TdsDataType::Guid => SQL_GUID, TdsDataType::Xml | TdsDataType::Json => SQL_WLONGVARCHAR, - TdsDataType::Vector | TdsDataType::SsVariant | TdsDataType::Udt => SQL_VARCHAR, + // mssql-python keys its sql_variant handling off this exact type, so + // reporting the column as character data hides the variant entirely. + TdsDataType::SsVariant => SQL_SS_VARIANT, + TdsDataType::Vector | TdsDataType::Udt => SQL_VARCHAR, _ => SQL_UNKNOWN_TYPE, } } From 7712038221ceaaa302bffdbec97fa19be12c770b Mon Sep 17 00:00:00 2001 From: David Engel Date: Thu, 13 Aug 2026 17:30:05 +0000 Subject: [PATCH 03/11] odbc: resolve the sql_variant underlying type via SQL_CA_SS_VARIANT_TYPE (P2) mssql-python cannot read a sql_variant column without this: it probes the column with SQLGetData, then asks SQLColAttribute for the underlying C type and routes its conversion on the answer. The base type is a property of the value, not the column -- a variant column can hold a different type in every row -- and read_sql_variant discarded it after decoding, so the decoded ColumnValues could not always recover it (varchar and nvarchar both arrive as ColumnValues::String). Carry it instead: - RowWriter gains a defaulted write_variant_base_type, so PyRowWriter and the mssql-js writer are unaffected. ColumnValues is untouched, which is what keeps this off the Python and Node bindings entirely. - decode_into grows an explicit SsVariant arm that reports the base type before writing the value, rather than falling through to decode(). - DefaultRowWriter records it per value position; CursorColumn::Value carries it to the ODBC layer, mirroring how PlpStreaming already carries its collation. CursorColumn is only used by mssql-odbc, so widening it is contained. - StmtState holds it alongside last_captured and clears it with the rest of the row-stream state. SQL_CA_SS_VARIANT_TYPE returns the C type mssql-python's MapVariantCTypeToSQLType expects. Two deliberate departures from msodbcsql: the exact numerics report SQL_C_CHAR rather than SQL_C_NUMERIC, because emitting SQL_NUMERIC_STRUCT is a permanent non-goal and character is how they are actually delivered; and a non-variant column is HY113, matching msodbcsql's IDS_S1_113 (its S1 prefix is the ODBC 2.x spelling of HY). Verified against SQL Server 2025 in Docker: 529 mssql-odbc unit tests, 1619 mssql-tds unit tests and all 24 test_client_read_apis integration tests pass. Workspace clippy and fmt clean. --- mssql-odbc/src/api/col_attribute.rs | 114 +++++++++++++++++++++-- mssql-odbc/src/api/get_data.rs | 6 +- mssql-odbc/src/api/sqlstate.rs | 7 ++ mssql-odbc/src/handles/stmt.rs | 6 ++ mssql-tds/src/connection/tds_client.rs | 22 ++++- mssql-tds/src/datatypes/decoder.rs | 26 +++++- mssql-tds/src/datatypes/row_writer.rs | 25 +++++ mssql-tds/tests/test_client_read_apis.rs | 45 +++++++-- 8 files changed, 228 insertions(+), 23 deletions(-) diff --git a/mssql-odbc/src/api/col_attribute.rs b/mssql-odbc/src/api/col_attribute.rs index ae23f313..ec6884d6 100644 --- a/mssql-odbc/src/api/col_attribute.rs +++ b/mssql-odbc/src/api/col_attribute.rs @@ -12,18 +12,21 @@ use tracing::{debug, error}; use crate::api::describe_col::{column_size, decimal_digits, odbc_sql_type}; use crate::api::odbc_types::{ - SQL_ATTR_READWRITE_UNKNOWN, SQL_DESC_AUTO_UNIQUE_VALUE, SQL_DESC_BASE_COLUMN_NAME, - SQL_DESC_CASE_SENSITIVE, SQL_DESC_CONCISE_TYPE, SQL_DESC_COUNT, SQL_DESC_DISPLAY_SIZE, - SQL_DESC_FIXED_PREC_SCALE, SQL_DESC_LABEL, SQL_DESC_LENGTH, SQL_DESC_NAME, SQL_DESC_NULLABLE, - SQL_DESC_NUM_PREC_RADIX, SQL_DESC_OCTET_LENGTH, SQL_DESC_PRECISION, SQL_DESC_SCALE, - SQL_DESC_SEARCHABLE, SQL_DESC_TYPE, SQL_DESC_TYPE_NAME, SQL_DESC_UNNAMED, SQL_DESC_UNSIGNED, - SQL_DESC_UPDATABLE, SQL_ERROR, SQL_INVALID_HANDLE, SQL_NAMED, SQL_NO_NULLS, SQL_NULLABLE, - SQL_PRED_SEARCHABLE, SQL_SUCCESS, SQL_SUCCESS_WITH_INFO, SQL_UNNAMED, SqlHandle, SqlLen, - SqlPointer, SqlReturn, SqlSmallInt, SqlUSmallInt, SqlWChar, + SQL_ATTR_READWRITE_UNKNOWN, SQL_C_BINARY, SQL_C_BIT, SQL_C_CHAR, SQL_C_DOUBLE, SQL_C_FLOAT, + SQL_C_GUID, SQL_C_SBIGINT, SQL_C_SLONG, SQL_C_SS_TIME2, SQL_C_SS_TIMESTAMPOFFSET, SQL_C_SSHORT, + SQL_C_TINYINT, SQL_C_TYPE_DATE, SQL_C_TYPE_TIMESTAMP, SQL_C_WCHAR, SQL_CA_SS_VARIANT_TYPE, + SQL_DESC_AUTO_UNIQUE_VALUE, SQL_DESC_BASE_COLUMN_NAME, SQL_DESC_CASE_SENSITIVE, + SQL_DESC_CONCISE_TYPE, SQL_DESC_COUNT, SQL_DESC_DISPLAY_SIZE, SQL_DESC_FIXED_PREC_SCALE, + SQL_DESC_LABEL, SQL_DESC_LENGTH, SQL_DESC_NAME, SQL_DESC_NULLABLE, SQL_DESC_NUM_PREC_RADIX, + SQL_DESC_OCTET_LENGTH, SQL_DESC_PRECISION, SQL_DESC_SCALE, SQL_DESC_SEARCHABLE, SQL_DESC_TYPE, + SQL_DESC_TYPE_NAME, SQL_DESC_UNNAMED, SQL_DESC_UNSIGNED, SQL_DESC_UPDATABLE, SQL_ERROR, + SQL_INVALID_HANDLE, SQL_NAMED, SQL_NO_NULLS, SQL_NULLABLE, SQL_PRED_SEARCHABLE, SQL_SUCCESS, + SQL_SUCCESS_WITH_INFO, SQL_UNNAMED, SqlHandle, SqlLen, SqlPointer, SqlReturn, SqlSmallInt, + SqlUSmallInt, SqlWChar, }; use crate::api::sqlstate::{ ERR_FUNCTION_SEQUENCE, ERR_INVALID_DESCRIPTOR_FIELD, ERR_INVALID_DESCRIPTOR_INDEX, - ERR_STRING_RIGHT_TRUNCATION, post_diag, + ERR_NOT_VARIANT_COLUMN, ERR_STRING_RIGHT_TRUNCATION, post_diag, }; use crate::api::util::{copy_with_nul, write_if_some}; use crate::error::free_errors; @@ -138,6 +141,25 @@ fn sql_col_attribute_w_safe( return SQL_ERROR; } + // The underlying type of a `sql_variant` is a property of the value, not the + // column, so it comes from the row that was read rather than the metadata. + if field_identifier == SQL_CA_SS_VARIANT_TYPE { + let is_variant = stmt_state.column_metadata[(column_number - 1) as usize].data_type + == TdsDataType::SsVariant; + if !is_variant { + post_diag(&mut stmt_state, ERR_NOT_VARIANT_COLUMN); + return SQL_ERROR; + } + let Some(base) = stmt_state.last_variant_base else { + // Callers probe the column with SQLGetData first; that read is what + // supplies the base type. + post_diag(&mut stmt_state, ERR_FUNCTION_SEQUENCE); + return SQL_ERROR; + }; + unsafe { write_if_some(numeric_attribute_ptr, SqlLen::from(variant_c_type(base))) }; + return SQL_SUCCESS; + } + let meta = &stmt_state.column_metadata[(column_number - 1) as usize]; let Some(attr) = column_attribute(meta, field_identifier) else { post_diag(&mut stmt_state, ERR_INVALID_DESCRIPTOR_FIELD); @@ -266,6 +288,53 @@ fn num_prec_radix(meta: &ColumnMetadata) -> SqlLen { } } +/// The C type a `sql_variant` value reports for `SQL_CA_SS_VARIANT_TYPE`. +/// +/// msodbcsql answers this from its per-row column info, so the value's base type +/// decides it rather than the column's declared type. +fn variant_c_type(base: TdsDataType) -> SqlSmallInt { + match base { + TdsDataType::Int1 => SQL_C_TINYINT, + TdsDataType::Int2 => SQL_C_SSHORT, + TdsDataType::Int4 => SQL_C_SLONG, + TdsDataType::Int8 => SQL_C_SBIGINT, + TdsDataType::Bit | TdsDataType::BitN => SQL_C_BIT, + TdsDataType::Flt4 => SQL_C_FLOAT, + TdsDataType::Flt8 | TdsDataType::FltN => SQL_C_DOUBLE, + // msodbcsql reports SQL_C_NUMERIC here, but emitting SQL_NUMERIC_STRUCT + // is a permanent non-goal for this driver (see the divergence table), so + // the exact numerics are advertised as character data, which is how they + // are actually delivered. + TdsDataType::Decimal + | TdsDataType::DecimalN + | TdsDataType::Numeric + | TdsDataType::NumericN + | TdsDataType::Money + | TdsDataType::Money4 + | TdsDataType::MoneyN => SQL_C_CHAR, + TdsDataType::DateN => SQL_C_TYPE_DATE, + TdsDataType::TimeN => SQL_C_SS_TIME2, + TdsDataType::DateTime | TdsDataType::DateTim4 | TdsDataType::DateTimeN => { + SQL_C_TYPE_TIMESTAMP + } + TdsDataType::DateTime2N => SQL_C_TYPE_TIMESTAMP, + TdsDataType::DateTimeOffsetN => SQL_C_SS_TIMESTAMPOFFSET, + TdsDataType::Char + | TdsDataType::BigChar + | TdsDataType::VarChar + | TdsDataType::BigVarChar => SQL_C_CHAR, + TdsDataType::NChar | TdsDataType::NVarChar => SQL_C_WCHAR, + TdsDataType::Binary + | TdsDataType::BigBinary + | TdsDataType::VarBinary + | TdsDataType::BigVarBinary => SQL_C_BINARY, + TdsDataType::Guid => SQL_C_GUID, + // SQL Server rejects the remaining types at insert time, so a variant + // cannot actually carry them; character is the safe fallback. + _ => SQL_C_CHAR, + } +} + /// `tinyint` is the only unsigned integer SQL Server exposes. fn is_unsigned(meta: &ColumnMetadata) -> bool { match meta.data_type { @@ -537,6 +606,33 @@ mod tests { ); } + /// The variant attribute is rejected outright on a column that is not a + /// `sql_variant`, rather than reporting a type the caller would then trust. + #[test] + fn variant_type_on_non_variant_column_is_rejected() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 1); + let mut out: SqlLen = 0; + let rc = unsafe { + sql_col_attribute_w( + h.stmt, + 1, + SQL_CA_SS_VARIANT_TYPE, + ptr::null_mut(), + 0, + ptr::null_mut(), + &mut out, + ) + }; + assert_eq!(rc, SQL_ERROR); + let sh = unsafe { handle_from_raw::(h.stmt) }; + let s = sh.inner.lock().unwrap(); + assert_eq!( + s.diag_records.last().unwrap().sql_state, + ERR_NOT_VARIANT_COLUMN.state + ); + } + /// mssql-python passes a null string buffer and reads only the numeric /// attribute, so a null `character_attribute_ptr` must not fault. #[test] diff --git a/mssql-odbc/src/api/get_data.rs b/mssql-odbc/src/api/get_data.rs index 5f6611be..7fa7e6a8 100644 --- a/mssql-odbc/src/api/get_data.rs +++ b/mssql-odbc/src/api/get_data.rs @@ -438,9 +438,13 @@ fn resume_row_to_column( drop(dbc_state); match cursor_result { - Ok(CursorColumn::Value(value)) => { + Ok(CursorColumn::Value { + value, + variant_base, + }) => { if let Ok(mut stmt_state) = stmt.inner.lock() { stmt_state.last_captured = Some((column_number, value)); + stmt_state.last_variant_base = variant_base; stmt_state.row_exhausted = false; stmt_state.partial_text_offset = None; return SQL_SUCCESS; diff --git a/mssql-odbc/src/api/sqlstate.rs b/mssql-odbc/src/api/sqlstate.rs index ab48004a..89b24b02 100644 --- a/mssql-odbc/src/api/sqlstate.rs +++ b/mssql-odbc/src/api/sqlstate.rs @@ -38,6 +38,9 @@ pub(crate) const SQLSTATE_HY091: [u8; 5] = *b"HY091"; pub(crate) const SQLSTATE_HY092: [u8; 5] = *b"HY092"; pub(crate) const SQLSTATE_HY096: [u8; 5] = *b"HY096"; pub(crate) const SQLSTATE_HY110: [u8; 5] = *b"HY110"; +// msodbcsql spells this IDS_S1_113; its `S1` prefix is the ODBC 2.x form of +// `HY` (IDS_S1_C00 is HYC00), so the 3.x state is HY113. +pub(crate) const SQLSTATE_HY113: [u8; 5] = *b"HY113"; // Driver-raised diagnostics: a fixed SQLSTATE paired with its canonical // message text. Bundling the two means a call site posts one value and can't @@ -82,6 +85,10 @@ pub(crate) const ERR_INVALID_DESCRIPTOR_FIELD: DiagMsg = DiagMsg { state: SQLSTATE_HY091, text: "Invalid descriptor field identifier", }; +pub(crate) const ERR_NOT_VARIANT_COLUMN: DiagMsg = DiagMsg { + state: SQLSTATE_HY113, + text: "Variant operation requested on column that is not sql_variant", +}; pub(crate) const ERR_UNBOUND_PARAMETER: DiagMsg = DiagMsg { state: SQLSTATE_07002, text: "COUNT field incorrect or syntax error", diff --git a/mssql-odbc/src/handles/stmt.rs b/mssql-odbc/src/handles/stmt.rs index a302dc96..63eeef13 100644 --- a/mssql-odbc/src/handles/stmt.rs +++ b/mssql-odbc/src/handles/stmt.rs @@ -15,6 +15,7 @@ use crate::api::odbc_types::{SqlULen, SqlUSmallInt}; use crate::error::{DiagRecord, HasDiagnostics}; use crate::params::BoundParam; use mssql_tds::datatypes::column_values::ColumnValues; +use mssql_tds::datatypes::sqldatatypes::TdsDataType; use mssql_tds::query::metadata::{ColumnMetadata, PlpEncoding}; /// State for a PLP column being streamed across repeated SQLGetData calls. @@ -93,6 +94,9 @@ pub(crate) struct StmtState { pub(crate) row_positioned: bool, /// The column value captured by the most recent resume_row_to_column call, with its 1-based column index. pub(crate) last_captured: Option<(usize, ColumnValues)>, + /// Base type of `last_captured` when that column is `sql_variant`. Set per + /// value, since a variant column can hold a different type in every row. + pub(crate) last_variant_base: Option, /// `true` when the last resume consumed the row's final column /// (`CursorColumn::RowEnded`). Distinguishes "row exhausted" from "decoder /// paused at a PLP column" when `last_captured` is `None` (see @@ -165,6 +169,7 @@ impl StmtState { pub(crate) fn reset_row_stream(&mut self) { self.row_positioned = false; self.last_captured = None; + self.last_variant_base = None; self.row_exhausted = false; self.active_plp = None; self.current_row_last_col = 0; @@ -242,6 +247,7 @@ impl StmtHandle { pending_unprepare: None, row_positioned: false, last_captured: None, + last_variant_base: None, row_exhausted: false, active_plp: None, current_row_last_col: 0, diff --git a/mssql-tds/src/connection/tds_client.rs b/mssql-tds/src/connection/tds_client.rs index f00fcebe..3fc2dc33 100644 --- a/mssql-tds/src/connection/tds_client.rs +++ b/mssql-tds/src/connection/tds_client.rs @@ -8,6 +8,7 @@ use crate::connection::session_recovery::RecoveryContext; use crate::datatypes::bulk_copy_metadata::BulkCopyColumnMetadata; use crate::datatypes::row_writer::{DefaultRowWriter, DiscardRowWriter, RowWriter}; use crate::datatypes::sql_string::SqlString; +use crate::datatypes::sqldatatypes::TdsDataType; use crate::datatypes::sqltypes::SqlType; use crate::error::Error::UsageError; use crate::error::{SqlErrorInfo, SqlInfoMessage}; @@ -114,7 +115,14 @@ pub struct PlpChunk { #[derive(Debug, PartialEq)] pub enum CursorColumn { /// A fully decoded, materialized column value (non-PLP). - Value(ColumnValues), + Value { + /// The decoded value. + value: ColumnValues, + /// Base type declared by a `sql_variant` column, `None` otherwise. The + /// decoded value cannot always recover it, since `varchar` and + /// `nvarchar` both arrive as [`ColumnValues::String`]. + variant_base: Option, + }, /// `target` is a PLP column; its bytes are streamed via /// [`TdsClient::read_active_plp_chunk`] until /// [`PlpChunk::reached_end`] is `true`. @@ -3760,12 +3768,16 @@ impl TdsClient { match result { RowReadResult::RowPaused(next_pause) => { self.active_row_read_state = ActiveRowReadState::RowPaused(Box::new(next_pause)); + let variant_base = capture.variant_base(0); let value = capture.take_row().into_iter().next().ok_or_else(|| { crate::error::Error::ProtocolError(format!( "Decoder produced no value for non-null column {target}" )) })?; - Ok(CursorColumn::Value(value)) + Ok(CursorColumn::Value { + value, + variant_base, + }) } RowReadResult::RowWritten => { // `target` was the last column; the row is now fully consumed. @@ -3773,12 +3785,16 @@ impl TdsClient { // pull reports `RowEnded`. Callers needing to distinguish a // rewind from "no row positioned" track the column themselves. self.active_row_read_state = ActiveRowReadState::Idle; + let variant_base = capture.variant_base(0); let value = capture.take_row().into_iter().next().ok_or_else(|| { crate::error::Error::ProtocolError(format!( "Decoder produced no value for non-null column {target}" )) })?; - Ok(CursorColumn::Value(value)) + Ok(CursorColumn::Value { + value, + variant_base, + }) } RowReadResult::PlpPaused(plp_state) => { let collation = plp_state.collation(); diff --git a/mssql-tds/src/datatypes/decoder.rs b/mssql-tds/src/datatypes/decoder.rs index acf90efc..f7530ee3 100644 --- a/mssql-tds/src/datatypes/decoder.rs +++ b/mssql-tds/src/datatypes/decoder.rs @@ -510,6 +510,21 @@ impl GenericDecoder { // Reads a SQL_VARIANT type from the TDS stream. async fn read_sql_variant(&self, reader: &mut T) -> TdsResult + where + T: TdsPacketReader + Send + Sync, + { + self.read_sql_variant_with_base(reader) + .await + .map(|(_, value)| value) + } + + /// As [`Self::read_sql_variant`], but also returns the base type the variant + /// declared on the wire. The decoded value alone cannot always recover it — + /// `varchar` and `nvarchar` both arrive as [`ColumnValues::String`]. + async fn read_sql_variant_with_base( + &self, + reader: &mut T, + ) -> TdsResult<(TdsDataType, ColumnValues)> where T: TdsPacketReader + Send + Sync, { @@ -553,7 +568,7 @@ impl GenericDecoder { ))); } }; - Ok(col_value) + Ok((tds_type, col_value)) } async fn decode_zero_propbyte_variant( @@ -1373,6 +1388,15 @@ impl GenericDecoder { } } + // A variant carries its own base type, which the caller may need in + // order to report the underlying type; the decoded value cannot + // always recover it. + TdsDataType::SsVariant => { + let (base, value) = self.read_sql_variant_with_base(reader).await?; + writer.write_variant_base_type(col, base); + write_column_value(writer, col, value); + } + // === Fallback: rare types go through decode() → write_column_value() === _ => { let value = self.decode(reader, metadata).await?; diff --git a/mssql-tds/src/datatypes/row_writer.rs b/mssql-tds/src/datatypes/row_writer.rs index 53c146fd..e96a68b7 100644 --- a/mssql-tds/src/datatypes/row_writer.rs +++ b/mssql-tds/src/datatypes/row_writer.rs @@ -9,6 +9,7 @@ use crate::datatypes::decoder::DecimalParts; use crate::datatypes::sql_json::SqlJson; use crate::datatypes::sql_string::SqlString; use crate::datatypes::sql_vector::SqlVector; +use crate::datatypes::sqldatatypes::TdsDataType; use uuid::Uuid; /// Pluggable decode sink for TDS row data. @@ -65,6 +66,10 @@ pub trait RowWriter { fn write_json(&mut self, col: usize, val: SqlJson); /// Writes a `vector` value. fn write_vector(&mut self, col: usize, val: SqlVector); + /// Reports the base type a `sql_variant` column carries, immediately before + /// the value write. Defaulted, so writers that do not surface the variant's + /// underlying type are unaffected. + fn write_variant_base_type(&mut self, _col: usize, _base: TdsDataType) {} /// Signals the end of the current row. fn end_row(&mut self); } @@ -73,6 +78,9 @@ pub trait RowWriter { /// the current decoder behavior. Existing `next_row()` callers see no change. pub struct DefaultRowWriter { row: Vec, + /// Base type of each `sql_variant` value, keyed by its position in `row`. + /// Empty unless the row contained a variant column. + variant_bases: Vec<(usize, TdsDataType)>, } impl DefaultRowWriter { @@ -80,11 +88,22 @@ impl DefaultRowWriter { pub fn new(col_count: usize) -> Self { Self { row: Vec::with_capacity(col_count), + variant_bases: Vec::new(), } } + /// Base type of the `sql_variant` value at `index`, or `None` when that + /// column was not a variant. Valid until [`Self::take_row`]. + pub fn variant_base(&self, index: usize) -> Option { + self.variant_bases + .iter() + .find(|(i, _)| *i == index) + .map(|(_, base)| *base) + } + /// Takes the completed row, leaving the writer ready for reuse. pub fn take_row(&mut self) -> Vec { + self.variant_bases.clear(); std::mem::take(&mut self.row) } } @@ -94,6 +113,12 @@ impl RowWriter for DefaultRowWriter { self.row.push(ColumnValues::Null); } + // The hook fires before the value is pushed, so `row.len()` is the index the + // value is about to occupy. + fn write_variant_base_type(&mut self, _col: usize, base: TdsDataType) { + self.variant_bases.push((self.row.len(), base)); + } + fn write_bool(&mut self, _col: usize, val: bool) { self.row.push(ColumnValues::Bit(val)); } diff --git a/mssql-tds/tests/test_client_read_apis.rs b/mssql-tds/tests/test_client_read_apis.rs index 67aed0fd..c63f54b9 100644 --- a/mssql-tds/tests/test_client_read_apis.rs +++ b/mssql-tds/tests/test_client_read_apis.rs @@ -896,7 +896,7 @@ mod client_based_iterators { let c2 = client.read_row_column(1).await?; assert!(matches!( &c2, - CursorColumn::Value(ColumnValues::String(s)) if s.to_utf8_string() == "row1-c2" + CursorColumn::Value { value: ColumnValues::String(s), .. } if s.to_utf8_string() == "row1-c2" )); assert!(matches!( client.read_row_column(3).await?, @@ -924,7 +924,7 @@ mod client_based_iterators { let c2b = client.read_row_column(1).await?; assert!(matches!( &c2b, - CursorColumn::Value(ColumnValues::String(s)) if s.to_utf8_string() == "row2-c2" + CursorColumn::Value { value: ColumnValues::String(s), .. } if s.to_utf8_string() == "row2-c2" )); assert!(matches!( client.read_row_column(3).await?, @@ -987,7 +987,10 @@ mod client_based_iterators { assert!(client.next_row_cursor().await?); assert_eq!( client.read_row_column(0).await?, - CursorColumn::Value(ColumnValues::Null) + CursorColumn::Value { + value: ColumnValues::Null, + variant_base: None + } ); assert!(matches!( client.read_row_column(1).await?, @@ -1014,7 +1017,10 @@ mod client_based_iterators { assert!(client.next_row_cursor().await?); assert_eq!( client.read_row_column(0).await?, - CursorColumn::Value(ColumnValues::Null) + CursorColumn::Value { + value: ColumnValues::Null, + variant_base: None + } ); assert!(matches!( client.read_row_column(1).await?, @@ -1097,7 +1103,13 @@ mod client_based_iterators { assert!(first_row_c2.chunks_exact(2).all(|c| c == [b'X', 0])); let c4 = client.read_row_column(3).await?; - assert_eq!(c4, CursorColumn::Value(ColumnValues::Int(24))); + assert_eq!( + c4, + CursorColumn::Value { + value: ColumnValues::Int(24), + variant_base: None + } + ); // Row 2. assert!(client.next_row_cursor().await?); @@ -1122,7 +1134,13 @@ mod client_based_iterators { assert!(second_row_c2.chunks_exact(2).all(|c| c == [b'Y', 0])); let c4b = client.read_row_column(3).await?; - assert_eq!(c4b, CursorColumn::Value(ColumnValues::Int(34))); + assert_eq!( + c4b, + CursorColumn::Value { + value: ColumnValues::Int(34), + variant_base: None + } + ); assert!(!client.next_row_cursor().await?); } @@ -1210,7 +1228,10 @@ mod client_based_iterators { // Cursor re-parked: a valid pull still returns the column value. assert_eq!( client.read_row_column(0).await?, - CursorColumn::Value(ColumnValues::Int(10)) + CursorColumn::Value { + value: ColumnValues::Int(10), + variant_base: None + } ); assert!(!client.next_row_cursor().await?); @@ -1245,7 +1266,10 @@ mod client_based_iterators { // the row stays paused (not fully consumed). assert_eq!( client.read_row_column(1).await?, - CursorColumn::Value(ColumnValues::Int(20)) + CursorColumn::Value { + value: ColumnValues::Int(20), + variant_base: None + } ); // c1 (0-based 0) is now behind the cursor: forward-only violation. @@ -1287,7 +1311,10 @@ mod client_based_iterators { // Read the last column (0-based 1); the cursor advances to idle. assert_eq!( client.read_row_column(1).await?, - CursorColumn::Value(ColumnValues::Int(20)) + CursorColumn::Value { + value: ColumnValues::Int(20), + variant_base: None + } ); // Out-of-range and backward pulls both collapse to RowEnded once idle. From 4710622be493af6ba01ddfbaa42aafeb82590285 Mon Sep 17 00:00:00 2001 From: David Engel Date: Thu, 13 Aug 2026 18:39:55 +0000 Subject: [PATCH 04/11] odbc: accept the zero-length SQL_C_BINARY probe mssql-python cannot read a sql_variant column without this. It calls SQLGetData(col, SQL_C_BINARY, NULL, 0, &indicator) first -- to detect NULL and to make the underlying type available to SQLColAttribute -- and on failure logs and yields None for the column. So without the probe the SQL_CA_SS_VARIANT_TYPE support added in the previous commit is unreachable, and variant columns come back as None with no error surfaced to the user. A zero-length read is a length/NULL enquiry rather than a data read, so it is admitted while binary delivery stays unimplemented: asking for binary data with a real buffer is still HYC00. The probe reports the available byte count and leaves the value resident, since the caller reads it for real afterwards. Lengths are exact where the binary encoding follows from the value and SQL_NO_TOTAL otherwise, rather than inventing a number this driver could not honour. This is a deliberate slice of AB#47239 (restore SQL_C_BINARY as a target) taken here because P2 ships dead code without it; the remaining binary delivery work stays on that item. 533 tests pass, workspace clippy and fmt clean. --- mssql-odbc/src/api/get_data.rs | 111 +++++++++++++++++++++++++++++++-- 1 file changed, 105 insertions(+), 6 deletions(-) diff --git a/mssql-odbc/src/api/get_data.rs b/mssql-odbc/src/api/get_data.rs index 7fa7e6a8..1324ce62 100644 --- a/mssql-odbc/src/api/get_data.rs +++ b/mssql-odbc/src/api/get_data.rs @@ -6,9 +6,9 @@ use tracing::{debug, error}; use super::odbc_types::{ - SQL_C_CHAR, SQL_C_GUID, SQL_C_WCHAR, SQL_ERROR, SQL_INVALID_HANDLE, SQL_NO_DATA, SQL_NO_TOTAL, - SQL_NULL_DATA, SQL_SUCCESS, SQL_SUCCESS_WITH_INFO, SqlHandle, SqlLen, SqlPointer, SqlReturn, - SqlSmallInt, SqlUSmallInt, + SQL_C_BINARY, SQL_C_CHAR, SQL_C_GUID, SQL_C_WCHAR, SQL_ERROR, SQL_INVALID_HANDLE, SQL_NO_DATA, + SQL_NO_TOTAL, SQL_NULL_DATA, SQL_SUCCESS, SQL_SUCCESS_WITH_INFO, SqlHandle, SqlLen, SqlPointer, + SqlReturn, SqlSmallInt, SqlUSmallInt, }; use super::sqlstate::*; use crate::api::odbc_types::SqlWChar; @@ -235,8 +235,13 @@ fn write_captured_column( // native); callers that need ANSI must transcode. SQL_C_WCHAR is UTF-16LE on // both drivers. // Check target type first — an unsupported type must not consume last_captured so the app can retry. + // A zero-length SQL_C_BINARY read is a length/NULL probe rather than a data + // read. mssql-python issues one on every sql_variant column to detect NULL + // and to make the underlying type available to SQLColAttribute, so the probe + // is admitted here; delivering binary data is still AB#47239. + let binary_probe = target_type == SQL_C_BINARY && buffer_length == 0; let typed_target = is_typed_c_target(target_type); - if !typed_target && target_type != SQL_C_CHAR && target_type != SQL_C_WCHAR { + if !typed_target && target_type != SQL_C_CHAR && target_type != SQL_C_WCHAR && !binary_probe { post_sql_error( stmt_state, SQLSTATE_HYC00, @@ -287,6 +292,13 @@ fn write_captured_column( // Fixed / typed C targets deliver the whole value in one call through the // shared conversion core; only the character targets chunk. + if binary_probe { + // Report what is available and leave the value resident — the caller + // reads it for real on a following call. + unsafe { write_if_some(strlen_or_ind_ptr, binary_length(value)) }; + return SQL_SUCCESS; + } + if typed_target { let converted = unsafe { convert_typed_c(value, target_type, target_value_ptr, strlen_or_ind_ptr) }; @@ -303,8 +315,8 @@ fn write_captured_column( Ok(t) => t, Err(TextError::Malformed) => { // Leave the value resident so the column stays re-readable. There is no - // raw-bytes fallback today: SQL_C_BINARY is rejected by the target gate - // above. + // raw-bytes fallback today: SQL_C_BINARY only answers the zero-length + // probe, it does not deliver data (AB#47239). error!("SQLGetData: column payload could not be decoded as text"); post_diag(stmt_state, ERR_INVALID_CHARACTER_VALUE); return SQL_ERROR; @@ -902,6 +914,26 @@ enum TextError { } /// `true` for the C targets served by the shared conversion core in one call. +/// Byte count a value would occupy in its `SQL_C_BINARY` form, for the length +/// probe. `SQL_NO_TOTAL` where the binary encoding is not fixed by the value +/// alone — this driver does not deliver binary data yet (AB#47239), so there is +/// no length to promise for those. +fn binary_length(value: &ColumnValues) -> SqlLen { + let len = match value { + ColumnValues::Bytes(b) => b.len(), + ColumnValues::String(s) => s.bytes.len(), + ColumnValues::Xml(x) => x.bytes.len(), + ColumnValues::Json(j) => j.bytes.len(), + ColumnValues::Bit(_) | ColumnValues::TinyInt(_) => 1, + ColumnValues::SmallInt(_) => 2, + ColumnValues::Int(_) | ColumnValues::Real(_) | ColumnValues::SmallMoney(_) => 4, + ColumnValues::BigInt(_) | ColumnValues::Float(_) | ColumnValues::Money(_) => 8, + ColumnValues::Uuid(_) => 16, + _ => return SQL_NO_TOTAL, + }; + SqlLen::try_from(len).unwrap_or(SqlLen::MAX) +} + fn is_typed_c_target(target_type: SqlSmallInt) -> bool { is_integer_c_target(target_type) || is_float_c_target(target_type) @@ -1380,6 +1412,73 @@ mod tests { s.last_captured = Some((1, value)); } + /// A zero-length SQL_C_BINARY read reports the available length and leaves + /// the value resident, so the caller can still read it for real afterwards. + /// This is the probe mssql-python issues on every sql_variant column. + #[test] + fn get_data_binary_probe_reports_length_without_consuming() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_captured(&h, ColumnValues::Int(7)); + + let mut ind: SqlLen = 0; + let ret = + unsafe { sql_get_data(h.stmt, 1, SQL_C_BINARY, std::ptr::null_mut(), 0, &mut ind) }; + assert_eq!(ret, SQL_SUCCESS); + assert_eq!(ind, 4); + + // The value survived the probe. + let mut out: i32 = 0; + let ret = unsafe { + sql_get_data( + h.stmt, + 1, + crate::api::odbc_types::SQL_C_SLONG, + (&mut out as *mut i32).cast(), + 4, + &mut ind, + ) + }; + assert_eq!(ret, SQL_SUCCESS); + assert_eq!(out, 7); + } + + #[test] + fn get_data_binary_probe_reports_null() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_captured(&h, ColumnValues::Null); + + let mut ind: SqlLen = 0; + let ret = + unsafe { sql_get_data(h.stmt, 1, SQL_C_BINARY, std::ptr::null_mut(), 0, &mut ind) }; + assert_eq!(ret, SQL_SUCCESS); + assert_eq!(ind, SQL_NULL_DATA); + } + + /// Only the zero-length probe is supported; asking for binary data is still + /// unimplemented. + #[test] + fn get_data_binary_with_buffer_is_not_implemented() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_captured(&h, ColumnValues::Int(7)); + + let mut buf = [0u8; 8]; + let mut ind: SqlLen = 0; + let ret = unsafe { + sql_get_data( + h.stmt, + 1, + SQL_C_BINARY, + buf.as_mut_ptr() as SqlPointer, + buf.len() as SqlLen, + &mut ind, + ) + }; + assert_eq!(ret, SQL_ERROR); + let sh = unsafe { handle_from_raw::(h.stmt) }; + let s = sh.inner.lock().unwrap(); + assert_eq!(s.diag_records.last().unwrap().sql_state, SQLSTATE_HYC00); + } + #[test] fn get_data_typed_integer_target() { let h = TestHandles::with_env_dbc_stmt(); From 92f83c9b99c57c38bc797d816e4c27ff86ccadd5 Mon Sep 17 00:00:00 2001 From: David Engel Date: Thu, 13 Aug 2026 18:41:05 +0000 Subject: [PATCH 05/11] docs: record P2's variant dependency chain and the two new divergences The sql_variant path needs three things (SQL_SS_VARIANT from SQLDescribeCol, the zero-length SQL_C_BINARY probe, then SQL_CA_SS_VARIANT_TYPE), and a missing link shows up as silently empty data rather than an error, so the chain is written down rather than left to be rediscovered. Adds the two divergences the variant work introduced: numerics reporting SQL_C_CHAR instead of SQL_C_NUMERIC, which follows from the SQL_NUMERIC_STRUCT non-goal, and HY113 for a non-variant column. --- mssql-odbc/docs/typed-columnar-fetch-plan.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/mssql-odbc/docs/typed-columnar-fetch-plan.md b/mssql-odbc/docs/typed-columnar-fetch-plan.md index 700b4c1e..49611bac 100644 --- a/mssql-odbc/docs/typed-columnar-fetch-plan.md +++ b/mssql-odbc/docs/typed-columnar-fetch-plan.md @@ -103,6 +103,8 @@ These were found by reading `Sql/Ntdbms/sqlncli/odbc/sqlccnvt.cpp` while reviewi | `T` separator, `HH:MM` without seconds, unpadded fields such as `2023-6-5` | rejected (fixed-length token grammar) | accepted | Permissive. Low risk, same task. | | A time-only value into `SQL_C_TYPE_TIMESTAMP` | fills in the current date and succeeds, per Appendix D | `22018` from a character source, `07006` from a `time` column | Gap — Task [47247](https://sqlclientdrivers.visualstudio.com/mssql-rs/_workitems/edit/47247). Needs a platform-specific local-date helper, so it is not a one-line fix. | | Any source into `SQL_C_NUMERIC` | converts, per Appendix D | `HYC00` | **Deliberate, and permanent.** Decimal is delivered as character data, which is what mssql-python requests, so `SQL_NUMERIC_STRUCT` is not scheduled to become supported. Anchored by `UnsupportedCTypeReturnsHyc00ThenValueReadable`. | +| `SQL_CA_SS_VARIANT_TYPE` for a variant holding `decimal` / `numeric` / `money` | `SQL_C_NUMERIC` | `SQL_C_CHAR` | **Deliberate**, and follows from the row above: reporting `SQL_C_NUMERIC` would make the caller request a `SQL_NUMERIC_STRUCT` this driver refuses. Character is how those values are actually delivered. | +| `SQL_CA_SS_VARIANT_TYPE` on a column that is not `sql_variant` | `HY113` (`IDS_S1_113`) | `HY113` | Matches. The `S1` prefix in msodbcsql's identifier is the ODBC 2.x spelling of `HY`, cf. `IDS_S1_C00` = `HYC00`. | ### P2 — SQLColAttributeW — Task [46579](https://sqlclientdrivers.visualstudio.com/mssql-rs/_workitems/edit/46579) @@ -110,6 +112,14 @@ These were found by reading `Sql/Ntdbms/sqlncli/odbc/sqlccnvt.cpp` while reviewi - Plus common descriptor fields (type / concise type, length, octet length, precision, scale, name, unsigned, nullable, display size) reusing the `SQLDescribeColW` metadata mapping. - Export `SQLColAttributeW` (driver-load requires the pointer non-null). +Reading a `sql_variant` column takes three things, not one, and mssql-python needs all of them before it will produce a value — on any failure it logs and yields `None` for the column, so a missing link shows up as silently empty data rather than an error: + +1. `SQLDescribeCol` must report `SQL_SS_VARIANT`. mssql-python branches on that exact type; while the column was reported as `SQL_VARCHAR` it never entered the variant path at all. +2. `SQLGetData(col, SQL_C_BINARY, NULL, 0, &indicator)` must succeed. This is a length/NULL probe, not a data read; it is admitted while binary delivery stays unimplemented (a real buffer is still `HYC00`, tracked as Task [47239](https://sqlclientdrivers.visualstudio.com/mssql-rs/_workitems/edit/47239)). +3. `SQLColAttribute(SQL_CA_SS_VARIANT_TYPE)` returns the C type of the value just probed. + +The underlying type is a property of the **value**, not the column — a variant column can hold a different type in every row — so it is carried up from the decoder rather than derived from metadata: `RowWriter` gained a defaulted `write_variant_base_type`, `CursorColumn::Value` carries the base type alongside the value, and `StmtState` clears it with the rest of the row-stream state. `ColumnValues` is deliberately untouched, which is what keeps this change out of the Python and Node bindings. + ### P3 — SQLBindCol + block SQLFetchScroll — Task [46580](https://sqlclientdrivers.visualstudio.com/mssql-rs/_workitems/edit/46580) - `SQLBindCol`: store per-column binding (col, target C type, buffer ptr, buffer len, indicator ptr); support unbind (null ptr) and `SQLFreeStmt(SQL_UNBIND)`. @@ -149,7 +159,7 @@ Both of those landed with the fetch rework in [#153](https://github.com/microsof | P0 — Prerequisites & plumbing | 46577 | Implemented (build + clippy clean, 332 tests pass) | | P1 — Typed SQLGetData | 46578 | Implemented (int/float/guid/date-time C targets + char/wchar rendering; 491 tests pass). Chunked retrieval and incremental PLP streaming are owned by #153 (merged), on top of which the typed targets are dispatched; missing source-type conversions tracked as P1a; `SQL_C_BINARY` and binary→char hex are **not** implemented (see the P1 section); `sql_variant` underlying-type resolution deferred to P2. | | P1a — Mandatory source-type conversions | 47107 | Implemented (decimal, money and character sources into the numeric and date/time C targets; `01S07` on lossy numeric conversion, `22018` on an invalid character literal). | -| P2 — SQLColAttributeW | 46579 | Not started | +| P2 — SQLColAttributeW | 46579 | Implemented (common descriptor fields + `SQL_CA_SS_VARIANT_TYPE`, plus the `SQL_SS_VARIANT` type mapping and the zero-length `SQL_C_BINARY` probe the variant path depends on). Binary *delivery* remains unimplemented (Task 47239). | | P3 — SQLBindCol + SQLFetchScroll | 46580 | Not started | | P4 — Exports & driver-load compat | 46581 | Not started | | P5 — Testing & end-to-end | 46582 | Not started | From 15ea6f8db6419a73535d7200b2e2b021d37b7adf Mon Sep 17 00:00:00 2001 From: David Engel Date: Thu, 13 Aug 2026 19:00:51 +0000 Subject: [PATCH 06/11] e2e: cover SQLColAttributeW against a live server Unit tests can only build `int` column metadata (int_columns is the only constructor available outside the decoder), so the per-type mapping tables -- concise type, type name, numeric radix, precision/scale, unsigned -- and the whole sql_variant path had no coverage at all. This adds it where those can actually run. The variant test walks the full sequence an application uses: SQLDescribeCol reports SQL_SS_VARIANT, a zero-length SQL_C_BINARY probe primes the value, then SQL_CA_SS_VARIANT_TYPE returns its underlying C type. It reads two rows holding different base types in the same column, which is what pins the type to the value rather than the column -- the property the whole plumbing exists for. Cross-checked every hand-written SQL_DESC_* constant in odbc_types.rs against the real ODBC headers with static_assert; all 26 match. --- mssql-odbc/tests/e2e/CMakeLists.txt | 1 + .../tests/e2e/tests/col_attribute_test.cpp | 220 ++++++++++++++++++ 2 files changed, 221 insertions(+) create mode 100644 mssql-odbc/tests/e2e/tests/col_attribute_test.cpp diff --git a/mssql-odbc/tests/e2e/CMakeLists.txt b/mssql-odbc/tests/e2e/CMakeLists.txt index e93fe0c3..8f8fe978 100644 --- a/mssql-odbc/tests/e2e/CMakeLists.txt +++ b/mssql-odbc/tests/e2e/CMakeLists.txt @@ -117,6 +117,7 @@ add_odbc_test(free_handle_test tests/free_handle_test.cpp) add_odbc_test(set_env_attr_test tests/set_env_attr_test.cpp) add_odbc_test(get_diag_rec_test tests/get_diag_rec_test.cpp) add_odbc_test(describe_col_test tests/describe_col_test.cpp) +add_odbc_test(col_attribute_test tests/col_attribute_test.cpp) add_odbc_test(num_result_cols_test tests/num_result_cols_test.cpp) add_odbc_test(get_diag_field_test tests/get_diag_field_test.cpp) add_odbc_test(driver_connect_test tests/driver_connect_test.cpp) diff --git a/mssql-odbc/tests/e2e/tests/col_attribute_test.cpp b/mssql-odbc/tests/e2e/tests/col_attribute_test.cpp new file mode 100644 index 00000000..f7e4dcd1 --- /dev/null +++ b/mssql-odbc/tests/e2e/tests/col_attribute_test.cpp @@ -0,0 +1,220 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// col_attribute_test.cpp – E2E tests for SQLColAttributeW. +// +// Unit tests can only build `int` column metadata, so the per-type mapping +// tables (concise type, type name, radix, and the sql_variant underlying type) +// are only meaningfully exercised here, against a live server. +// +// Verifies: +// 1. NullHandle - SQL_NULL_HSTMT → SQL_INVALID_HANDLE +// 2. FreshStatementReturnsSequenceError- no active stmt → HY010 +// 3. InvalidColumnOrdinal - column 0 and past-end → 07009 +// 4. UnknownFieldIdentifier - unreported field id → HY091 +// 5. DescCountIgnoresColumnNumber - SQL_DESC_COUNT describes the result set +// 6. ConciseTypePerColumnType - int/varchar/nvarchar/decimal concise types +// 7. TypeNameAndRadix - SQL_DESC_TYPE_NAME, SQL_DESC_NUM_PREC_RADIX +// 8. PrecisionScaleAndNullable - DECIMAL(10,2), NOT NULL vs NULL +// 9. UnsignedOnlyForTinyint - tinyint unsigned, int signed +// 10. NameIsReportedInBytes - SQL_DESC_NAME length is a byte count +// 11. NameTruncationReturnsInfo - short buffer → SUCCESS_WITH_INFO + 01004 +// 12. VariantTypeOnNonVariantColumn - HY113 +// 13. VariantUnderlyingTypeAfterProbe - probe then SQL_CA_SS_VARIANT_TYPE +// 14. VariantTypeBeforeProbeIsSequenceError - attribute before the value is read + +#include "odbc_test_fixture.h" + +#include +#include + +// SQL Server-specific identifiers not in standard . +#ifndef SQL_CA_SS_VARIANT_TYPE +#define SQL_CA_SS_VARIANT_TYPE (1215) +#endif +#ifndef SQL_SS_VARIANT +#define SQL_SS_VARIANT (-150) +#endif + +class ColAttributeLiveTest : public ODBCTest {}; + +// Reads a numeric attribute, asserting the call succeeded. +static SQLLEN NumericAttr(SQLHSTMT stmt, SQLUSMALLINT col, SQLUSMALLINT field) { + SQLLEN value = -1; + SQLRETURN rc = SQLColAttribute(stmt, col, field, nullptr, 0, nullptr, &value); + EXPECT_TRUE(SQL_SUCCEEDED(rc)) << "field " << field; + return value; +} + +TEST(ColAttributeTest, NullHandle) { + SQLLEN value = 0; + SQLRETURN rc = SQLColAttribute( + SQL_NULL_HSTMT, 1, SQL_DESC_CONCISE_TYPE, nullptr, 0, nullptr, &value); + EXPECT_EQ(SQL_INVALID_HANDLE, rc); +} + +TEST_F(ColAttributeLiveTest, FreshStatementReturnsSequenceError) { + SQLLEN value = 0; + SQLRETURN rc = + SQLColAttribute(stmt_, 1, SQL_DESC_CONCISE_TYPE, nullptr, 0, nullptr, &value); + EXPECT_EQ(SQL_ERROR, rc); + EXPECT_SQLSTATE(SQL_HANDLE_STMT, stmt_, "HY010"); +} + +TEST_F(ColAttributeLiveTest, InvalidColumnOrdinal) { + ExecDirect("SELECT CAST(1 AS INT) AS c1"); + for (SQLUSMALLINT col : {static_cast(0), static_cast(2)}) { + SQLLEN value = 0; + SQLRETURN rc = + SQLColAttribute(stmt_, col, SQL_DESC_CONCISE_TYPE, nullptr, 0, nullptr, &value); + EXPECT_EQ(SQL_ERROR, rc) << "column " << col; + EXPECT_SQLSTATE(SQL_HANDLE_STMT, stmt_, "07009"); + } + SQLCloseCursor(stmt_); +} + +// An identifier this driver does not report is rejected rather than answered +// with a silent zero. msodbcsql reports a wider set, so only compare our leg. +TEST_F(ColAttributeLiveTest, UnknownFieldIdentifier) { + SKIP_IF_COMPARING_MSODBCSQL(); + ExecDirect("SELECT CAST(1 AS INT) AS c1"); + SQLLEN value = 0; + SQLRETURN rc = SQLColAttribute(stmt_, 1, 9999, nullptr, 0, nullptr, &value); + EXPECT_EQ(SQL_ERROR, rc); + EXPECT_SQLSTATE(SQL_HANDLE_STMT, stmt_, "HY091"); + SQLCloseCursor(stmt_); +} + +// SQL_DESC_COUNT describes the result set, so it answers for a column number +// that would otherwise be out of range. +TEST_F(ColAttributeLiveTest, DescCountIgnoresColumnNumber) { + ExecDirect("SELECT 1 AS a, 2 AS b, 3 AS c"); + EXPECT_EQ(3, NumericAttr(stmt_, 1, SQL_DESC_COUNT)); + EXPECT_EQ(3, NumericAttr(stmt_, 99, SQL_DESC_COUNT)); + SQLCloseCursor(stmt_); +} + +TEST_F(ColAttributeLiveTest, ConciseTypePerColumnType) { + ExecDirect( + "SELECT CAST(1 AS INT) AS i, CAST('a' AS VARCHAR(10)) AS v," + " CAST(N'b' AS NVARCHAR(10)) AS n, CAST(1.5 AS DECIMAL(10,2)) AS d"); + EXPECT_EQ(SQL_INTEGER, NumericAttr(stmt_, 1, SQL_DESC_CONCISE_TYPE)); + EXPECT_EQ(SQL_VARCHAR, NumericAttr(stmt_, 2, SQL_DESC_CONCISE_TYPE)); + EXPECT_EQ(SQL_WVARCHAR, NumericAttr(stmt_, 3, SQL_DESC_CONCISE_TYPE)); + EXPECT_EQ(SQL_DECIMAL, NumericAttr(stmt_, 4, SQL_DESC_CONCISE_TYPE)); + SQLCloseCursor(stmt_); +} + +TEST_F(ColAttributeLiveTest, TypeNameAndRadix) { + ExecDirect("SELECT CAST(1 AS INT) AS i, CAST(1.5 AS FLOAT) AS f," + " CAST('a' AS VARCHAR(10)) AS v"); + + SQLTCHAR name[64] = {}; + SQLSMALLINT nameLen = 0; + SQLRETURN rc = + SQLColAttribute(stmt_, 1, SQL_DESC_TYPE_NAME, name, sizeof(name), &nameLen, nullptr); + ASSERT_SQL_OK(rc, SQL_HANDLE_STMT, stmt_); + EXPECT_EQ("int", ODBCTestUtils::ToNarrow(SqlTString(name))); + + // Exact numerics are base 10, approximate are base 2, non-numerics have none. + EXPECT_EQ(10, NumericAttr(stmt_, 1, SQL_DESC_NUM_PREC_RADIX)); + EXPECT_EQ(2, NumericAttr(stmt_, 2, SQL_DESC_NUM_PREC_RADIX)); + EXPECT_EQ(0, NumericAttr(stmt_, 3, SQL_DESC_NUM_PREC_RADIX)); + SQLCloseCursor(stmt_); +} + +TEST_F(ColAttributeLiveTest, PrecisionScaleAndNullable) { + ExecDirect("SELECT CAST(1.5 AS DECIMAL(10,2)) AS d," + " CAST(NULL AS INT) AS n"); + EXPECT_EQ(10, NumericAttr(stmt_, 1, SQL_DESC_PRECISION)); + EXPECT_EQ(2, NumericAttr(stmt_, 1, SQL_DESC_SCALE)); + EXPECT_EQ(SQL_NULLABLE, NumericAttr(stmt_, 2, SQL_DESC_NULLABLE)); + SQLCloseCursor(stmt_); +} + +// `tinyint` is the only unsigned integer SQL Server exposes. +TEST_F(ColAttributeLiveTest, UnsignedOnlyForTinyint) { + ExecDirect("SELECT CAST(1 AS TINYINT) AS t, CAST(1 AS INT) AS i"); + EXPECT_EQ(SQL_TRUE, NumericAttr(stmt_, 1, SQL_DESC_UNSIGNED)); + EXPECT_EQ(SQL_FALSE, NumericAttr(stmt_, 2, SQL_DESC_UNSIGNED)); + SQLCloseCursor(stmt_); +} + +// The wide entry point reports string lengths in bytes, not characters. +TEST_F(ColAttributeLiveTest, NameIsReportedInBytes) { + ExecDirect("SELECT 1 AS abcd"); + SQLTCHAR name[32] = {}; + SQLSMALLINT nameLen = 0; + SQLRETURN rc = + SQLColAttribute(stmt_, 1, SQL_DESC_NAME, name, sizeof(name), &nameLen, nullptr); + ASSERT_SQL_OK(rc, SQL_HANDLE_STMT, stmt_); + EXPECT_EQ("abcd", ODBCTestUtils::ToNarrow(SqlTString(name))); + EXPECT_EQ(static_cast(4 * sizeof(SQLTCHAR)), nameLen); + SQLCloseCursor(stmt_); +} + +TEST_F(ColAttributeLiveTest, NameTruncationReturnsInfo) { + ExecDirect("SELECT 1 AS averylongcolumnname"); + SQLTCHAR name[3] = {}; + SQLSMALLINT nameLen = 0; + SQLRETURN rc = + SQLColAttribute(stmt_, 1, SQL_DESC_NAME, name, sizeof(name), &nameLen, nullptr); + EXPECT_EQ(SQL_SUCCESS_WITH_INFO, rc); + EXPECT_SQLSTATE(SQL_HANDLE_STMT, stmt_, "01004"); + SQLCloseCursor(stmt_); +} + +// The variant attribute is rejected outright on a column that is not a +// sql_variant, rather than reporting a type the caller would then trust. +TEST_F(ColAttributeLiveTest, VariantTypeOnNonVariantColumn) { + ExecDirect("SELECT CAST(1 AS INT) AS c1"); + SQLLEN value = 0; + SQLRETURN rc = + SQLColAttribute(stmt_, 1, SQL_CA_SS_VARIANT_TYPE, nullptr, 0, nullptr, &value); + EXPECT_EQ(SQL_ERROR, rc); + EXPECT_SQLSTATE(SQL_HANDLE_STMT, stmt_, "HY113"); + SQLCloseCursor(stmt_); +} + +// The full sequence an application uses to read a sql_variant: describe the +// column, probe it with a zero-length SQL_C_BINARY read, then ask for the +// underlying C type. The underlying type belongs to the value, so it tracks the +// row rather than the column. +TEST_F(ColAttributeLiveTest, VariantUnderlyingTypeAfterProbe) { + ExecDirect( + "SELECT CAST(42 AS SQL_VARIANT) AS v" + " UNION ALL SELECT CAST(CAST('abc' AS VARCHAR(10)) AS SQL_VARIANT)"); + + SQLSMALLINT dataType = 0; + SQLRETURN rc = SQLDescribeCol( + stmt_, 1, nullptr, 0, nullptr, &dataType, nullptr, nullptr, nullptr); + ASSERT_SQL_OK(rc, SQL_HANDLE_STMT, stmt_); + EXPECT_EQ(SQL_SS_VARIANT, dataType); + + ASSERT_SQL_OK(SQLFetch(stmt_), SQL_HANDLE_STMT, stmt_); + SQLLEN indicator = 0; + ASSERT_SQL_OK(SQLGetData(stmt_, 1, SQL_C_BINARY, nullptr, 0, &indicator), + SQL_HANDLE_STMT, stmt_); + EXPECT_NE(SQL_NULL_DATA, indicator); + EXPECT_EQ(SQL_C_SLONG, NumericAttr(stmt_, 1, SQL_CA_SS_VARIANT_TYPE)); + + // Second row holds a different base type in the same column. + ASSERT_SQL_OK(SQLFetch(stmt_), SQL_HANDLE_STMT, stmt_); + ASSERT_SQL_OK(SQLGetData(stmt_, 1, SQL_C_BINARY, nullptr, 0, &indicator), + SQL_HANDLE_STMT, stmt_); + EXPECT_EQ(SQL_C_CHAR, NumericAttr(stmt_, 1, SQL_CA_SS_VARIANT_TYPE)); + + SQLCloseCursor(stmt_); +} + +// Without the probe there is no value to report a type for; msodbcsql relies on +// the same ordering, so this only pins our diagnostic. +TEST_F(ColAttributeLiveTest, VariantTypeBeforeProbeIsSequenceError) { + SKIP_IF_COMPARING_MSODBCSQL(); + ExecDirect("SELECT CAST(42 AS SQL_VARIANT) AS v"); + ASSERT_SQL_OK(SQLFetch(stmt_), SQL_HANDLE_STMT, stmt_); + SQLLEN value = 0; + SQLRETURN rc = + SQLColAttribute(stmt_, 1, SQL_CA_SS_VARIANT_TYPE, nullptr, 0, nullptr, &value); + EXPECT_EQ(SQL_ERROR, rc); + EXPECT_SQLSTATE(SQL_HANDLE_STMT, stmt_, "HY010"); + SQLCloseCursor(stmt_); +} From a8627f3c41d636468a52d9b53e32deae1b007a36 Mon Sep 17 00:00:00 2001 From: David Engel Date: Thu, 13 Aug 2026 19:17:07 +0000 Subject: [PATCH 07/11] e2e: connect the col_attribute fixture and key the probe on length, not null Two defects the local run caught, neither of which unit tests could: - ColAttributeLiveTest had an empty body, so it never called Connect(). Every live test failed with SQL_INVALID_HANDLE on a null stmt_ rather than testing anything. Matches the SetUp() the other live fixtures use. - The variant probe passed a null TargetValuePtr, which the unixODBC Driver Manager rejects with HY009 before the driver sees the call. mssql-python can pass NULL because it dlopen's the driver directly and bypasses the DM; through a Driver Manager the probe has to use a real pointer with a zero buffer length. The driver keys the probe on the length, so both forms work there. All 19 e2e binaries pass against SQL Server 2025 in Docker, including the full variant chain over two rows carrying different base types. --- .../tests/e2e/tests/col_attribute_test.cpp | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/mssql-odbc/tests/e2e/tests/col_attribute_test.cpp b/mssql-odbc/tests/e2e/tests/col_attribute_test.cpp index f7e4dcd1..743bcd60 100644 --- a/mssql-odbc/tests/e2e/tests/col_attribute_test.cpp +++ b/mssql-odbc/tests/e2e/tests/col_attribute_test.cpp @@ -34,7 +34,16 @@ #define SQL_SS_VARIANT (-150) #endif -class ColAttributeLiveTest : public ODBCTest {}; +class ColAttributeLiveTest : public ODBCTest { +protected: + void SetUp() override { + ODBCTest::SetUp(); + if (!ODBCTestConfig::Instance().HasConnection()) { + FAIL() << "No connection configured – set ODBC_TEST_SERVER or ODBC_TEST_CONNSTR"; + } + Connect(); + } +}; // Reads a numeric attribute, asserting the call succeeded. static SQLLEN NumericAttr(SQLHSTMT stmt, SQLUSMALLINT col, SQLUSMALLINT field) { @@ -190,15 +199,20 @@ TEST_F(ColAttributeLiveTest, VariantUnderlyingTypeAfterProbe) { EXPECT_EQ(SQL_SS_VARIANT, dataType); ASSERT_SQL_OK(SQLFetch(stmt_), SQL_HANDLE_STMT, stmt_); + // The probe is keyed on a zero buffer length, not on a null pointer: + // mssql-python passes NULL here, but it dlopen's the driver directly, while + // these tests go through the Driver Manager, which rejects a null + // TargetValuePtr with HY009 before the driver ever sees the call. + SQLCHAR probe = 0; SQLLEN indicator = 0; - ASSERT_SQL_OK(SQLGetData(stmt_, 1, SQL_C_BINARY, nullptr, 0, &indicator), + ASSERT_SQL_OK(SQLGetData(stmt_, 1, SQL_C_BINARY, &probe, 0, &indicator), SQL_HANDLE_STMT, stmt_); EXPECT_NE(SQL_NULL_DATA, indicator); EXPECT_EQ(SQL_C_SLONG, NumericAttr(stmt_, 1, SQL_CA_SS_VARIANT_TYPE)); // Second row holds a different base type in the same column. ASSERT_SQL_OK(SQLFetch(stmt_), SQL_HANDLE_STMT, stmt_); - ASSERT_SQL_OK(SQLGetData(stmt_, 1, SQL_C_BINARY, nullptr, 0, &indicator), + ASSERT_SQL_OK(SQLGetData(stmt_, 1, SQL_C_BINARY, &probe, 0, &indicator), SQL_HANDLE_STMT, stmt_); EXPECT_EQ(SQL_C_CHAR, NumericAttr(stmt_, 1, SQL_CA_SS_VARIANT_TYPE)); From 41b4009a8ab725c72cfb44c4b4a635c161bbbac0 Mon Sep 17 00:00:00 2001 From: David Engel Date: Thu, 13 Aug 2026 20:38:38 +0000 Subject: [PATCH 08/11] e2e: skip the non-variant attribute check on the msodbcsql leg msodbcsql returns SUCCESS for SQL_CA_SS_VARIANT_TYPE on a column that is not a sql_variant, so asserting HY113 on both legs failed the parity comparison. Its SQL_CA_SS_VARIANT_TYPE case sets `wError = IDS_S1_113` and then plain `break`s, while the adjacent SQL_CA_SS_VARIANT_SERVER_TYPE case does `SETRC_SERR_GOTO(retcode, ErrorRet)` with that same error -- so the diagnostic it prepares is never returned. That asymmetry reads as an oversight rather than a contract, so this driver keeps HY113: telling the caller it asked the wrong question is more useful than answering it. Recorded as a deliberate divergence rather than silently matching. Verified locally against SQL Server 2025 in Docker with both drivers installed: 20 parity, 0 divergences, 0 shared failures. --- mssql-odbc/docs/typed-columnar-fetch-plan.md | 2 +- mssql-odbc/tests/e2e/tests/col_attribute_test.cpp | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/mssql-odbc/docs/typed-columnar-fetch-plan.md b/mssql-odbc/docs/typed-columnar-fetch-plan.md index 49611bac..86a62f43 100644 --- a/mssql-odbc/docs/typed-columnar-fetch-plan.md +++ b/mssql-odbc/docs/typed-columnar-fetch-plan.md @@ -104,7 +104,7 @@ These were found by reading `Sql/Ntdbms/sqlncli/odbc/sqlccnvt.cpp` while reviewi | A time-only value into `SQL_C_TYPE_TIMESTAMP` | fills in the current date and succeeds, per Appendix D | `22018` from a character source, `07006` from a `time` column | Gap — Task [47247](https://sqlclientdrivers.visualstudio.com/mssql-rs/_workitems/edit/47247). Needs a platform-specific local-date helper, so it is not a one-line fix. | | Any source into `SQL_C_NUMERIC` | converts, per Appendix D | `HYC00` | **Deliberate, and permanent.** Decimal is delivered as character data, which is what mssql-python requests, so `SQL_NUMERIC_STRUCT` is not scheduled to become supported. Anchored by `UnsupportedCTypeReturnsHyc00ThenValueReadable`. | | `SQL_CA_SS_VARIANT_TYPE` for a variant holding `decimal` / `numeric` / `money` | `SQL_C_NUMERIC` | `SQL_C_CHAR` | **Deliberate**, and follows from the row above: reporting `SQL_C_NUMERIC` would make the caller request a `SQL_NUMERIC_STRUCT` this driver refuses. Character is how those values are actually delivered. | -| `SQL_CA_SS_VARIANT_TYPE` on a column that is not `sql_variant` | `HY113` (`IDS_S1_113`) | `HY113` | Matches. The `S1` prefix in msodbcsql's identifier is the ODBC 2.x spelling of `HY`, cf. `IDS_S1_C00` = `HYC00`. | +| `SQL_CA_SS_VARIANT_TYPE` on a column that is not `sql_variant` | `SQL_SUCCESS` | `HY113` | **Deliberate.** msodbcsql prepares `IDS_S1_113` and then `break`s without returning it, where the adjacent `SQL_CA_SS_VARIANT_SERVER_TYPE` case does `SETRC_SERR_GOTO` with the same error — so its success looks like an oversight rather than a contract. Telling the caller it asked the wrong question is more useful than answering it. | ### P2 — SQLColAttributeW — Task [46579](https://sqlclientdrivers.visualstudio.com/mssql-rs/_workitems/edit/46579) diff --git a/mssql-odbc/tests/e2e/tests/col_attribute_test.cpp b/mssql-odbc/tests/e2e/tests/col_attribute_test.cpp index 743bcd60..7e6950d4 100644 --- a/mssql-odbc/tests/e2e/tests/col_attribute_test.cpp +++ b/mssql-odbc/tests/e2e/tests/col_attribute_test.cpp @@ -173,7 +173,14 @@ TEST_F(ColAttributeLiveTest, NameTruncationReturnsInfo) { // The variant attribute is rejected outright on a column that is not a // sql_variant, rather than reporting a type the caller would then trust. +// +// msodbcsql-specific: msodbcsql returns SUCCESS here. Its `SQL_CA_SS_VARIANT_TYPE` +// case sets `wError = IDS_S1_113` and then plain `break`s, where the neighbouring +// `SQL_CA_SS_VARIANT_SERVER_TYPE` case does `SETRC_SERR_GOTO(retcode, ErrorRet)` +// with the same error — so the diagnostic it prepares is never actually returned. +// Recorded in the divergence table in docs/typed-columnar-fetch-plan.md. TEST_F(ColAttributeLiveTest, VariantTypeOnNonVariantColumn) { + SKIP_IF_COMPARING_MSODBCSQL(); ExecDirect("SELECT CAST(1 AS INT) AS c1"); SQLLEN value = 0; SQLRETURN rc = From 63ad6d853b45cb44accb8177926f10735a077c8f Mon Sep 17 00:00:00 2001 From: David Engel Date: Thu, 13 Aug 2026 21:13:09 +0000 Subject: [PATCH 09/11] odbc: handle a NULL sql_variant, and key the base type to its column Two review findings, both real. - read_sql_variant_with_base read the base-type and property bytes unconditionally. A NULL variant is a zero length and nothing else, so those two reads consumed the following column's bytes. The base type is now Option, None for NULL, and the caller skips the hook rather than reporting a type that was never sent. This was pre-existing in read_sql_variant; the new cursor path inherited it. Not reachable through a normal SQL Server response, which carries NULLs in the NBCROW null bitmap and skips the column entirely, so the regression test is a decoder unit test rather than e2e: it places a sentinel after the variant and asserts it survives. Without the guard it fails with "Invalid TDS Type 171" -- 171 being that sentinel, consumed and misread as a base type. - last_variant_base was not associated with a column, so probing one variant column and then asking SQL_CA_SS_VARIANT_TYPE for a different one returned the first column's type instead of the sequence error. It now carries the 1-based column index and only answers for that column. 558 mssql-odbc and 1648 mssql-tds unit tests pass; e2e reports 20 parity, 0 divergences against msodbcsql on SQL Server 2025. --- mssql-odbc/src/api/col_attribute.rs | 8 +++- mssql-odbc/src/api/get_data.rs | 2 +- mssql-odbc/src/handles/stmt.rs | 7 +-- .../tests/e2e/tests/col_attribute_test.cpp | 47 +++++++++++++++++++ mssql-tds/src/datatypes/decoder.rs | 43 +++++++++++++++-- 5 files changed, 98 insertions(+), 9 deletions(-) diff --git a/mssql-odbc/src/api/col_attribute.rs b/mssql-odbc/src/api/col_attribute.rs index ec6884d6..f9bd79e2 100644 --- a/mssql-odbc/src/api/col_attribute.rs +++ b/mssql-odbc/src/api/col_attribute.rs @@ -150,7 +150,13 @@ fn sql_col_attribute_w_safe( post_diag(&mut stmt_state, ERR_NOT_VARIANT_COLUMN); return SQL_ERROR; } - let Some(base) = stmt_state.last_variant_base else { + // The base type belongs to the value that was probed, so it only answers + // for the column it came from. + let base = stmt_state + .last_variant_base + .filter(|(col, _)| *col == column_number as usize) + .map(|(_, base)| base); + let Some(base) = base else { // Callers probe the column with SQLGetData first; that read is what // supplies the base type. post_diag(&mut stmt_state, ERR_FUNCTION_SEQUENCE); diff --git a/mssql-odbc/src/api/get_data.rs b/mssql-odbc/src/api/get_data.rs index 1324ce62..cfe0c660 100644 --- a/mssql-odbc/src/api/get_data.rs +++ b/mssql-odbc/src/api/get_data.rs @@ -456,7 +456,7 @@ fn resume_row_to_column( }) => { if let Ok(mut stmt_state) = stmt.inner.lock() { stmt_state.last_captured = Some((column_number, value)); - stmt_state.last_variant_base = variant_base; + stmt_state.last_variant_base = variant_base.map(|base| (column_number, base)); stmt_state.row_exhausted = false; stmt_state.partial_text_offset = None; return SQL_SUCCESS; diff --git a/mssql-odbc/src/handles/stmt.rs b/mssql-odbc/src/handles/stmt.rs index 63eeef13..2bead1e4 100644 --- a/mssql-odbc/src/handles/stmt.rs +++ b/mssql-odbc/src/handles/stmt.rs @@ -94,9 +94,10 @@ pub(crate) struct StmtState { pub(crate) row_positioned: bool, /// The column value captured by the most recent resume_row_to_column call, with its 1-based column index. pub(crate) last_captured: Option<(usize, ColumnValues)>, - /// Base type of `last_captured` when that column is `sql_variant`. Set per - /// value, since a variant column can hold a different type in every row. - pub(crate) last_variant_base: Option, + /// Base type of `last_captured` when that column is `sql_variant`, with its + /// 1-based column index. Set per value, since a variant column can hold a + /// different type in every row. + pub(crate) last_variant_base: Option<(usize, TdsDataType)>, /// `true` when the last resume consumed the row's final column /// (`CursorColumn::RowEnded`). Distinguishes "row exhausted" from "decoder /// paused at a PLP column" when `last_captured` is `None` (see diff --git a/mssql-odbc/tests/e2e/tests/col_attribute_test.cpp b/mssql-odbc/tests/e2e/tests/col_attribute_test.cpp index 7e6950d4..6521100e 100644 --- a/mssql-odbc/tests/e2e/tests/col_attribute_test.cpp +++ b/mssql-odbc/tests/e2e/tests/col_attribute_test.cpp @@ -226,6 +226,53 @@ TEST_F(ColAttributeLiveTest, VariantUnderlyingTypeAfterProbe) { SQLCloseCursor(stmt_); } +// A NULL sql_variant is just a zero length on the wire, with no base type or +// property byte following it. Reading those anyway would consume the next +// column's bytes, so the column after the variant is what actually proves it. +TEST_F(ColAttributeLiveTest, NullVariantDoesNotDisturbTheFollowingColumn) { + ExecDirect("SELECT CAST(NULL AS SQL_VARIANT) AS v, CAST(12345 AS INT) AS following"); + + ASSERT_SQL_OK(SQLFetch(stmt_), SQL_HANDLE_STMT, stmt_); + + SQLCHAR probe = 0; + SQLLEN indicator = 0; + ASSERT_SQL_OK(SQLGetData(stmt_, 1, SQL_C_BINARY, &probe, 0, &indicator), + SQL_HANDLE_STMT, stmt_); + EXPECT_EQ(SQL_NULL_DATA, indicator); + + SQLINTEGER following = 0; + ASSERT_SQL_OK(SQLGetData(stmt_, 2, SQL_C_SLONG, &following, sizeof(following), &indicator), + SQL_HANDLE_STMT, stmt_); + EXPECT_EQ(12345, following); + + SQLCloseCursor(stmt_); +} + +// The base type belongs to the value that was probed, so probing one variant +// column must not answer for another. +TEST_F(ColAttributeLiveTest, VariantTypeIsPerColumn) { + SKIP_IF_COMPARING_MSODBCSQL(); + ExecDirect("SELECT CAST(42 AS SQL_VARIANT) AS a," + " CAST(CAST('x' AS VARCHAR(5)) AS SQL_VARIANT) AS b"); + + ASSERT_SQL_OK(SQLFetch(stmt_), SQL_HANDLE_STMT, stmt_); + SQLCHAR probe = 0; + SQLLEN indicator = 0; + ASSERT_SQL_OK(SQLGetData(stmt_, 1, SQL_C_BINARY, &probe, 0, &indicator), + SQL_HANDLE_STMT, stmt_); + EXPECT_EQ(SQL_C_SLONG, NumericAttr(stmt_, 1, SQL_CA_SS_VARIANT_TYPE)); + + // Column 2 has not been probed, so it has no type to report yet -- it must + // not inherit column 1's. + SQLLEN value = 0; + SQLRETURN rc = + SQLColAttribute(stmt_, 2, SQL_CA_SS_VARIANT_TYPE, nullptr, 0, nullptr, &value); + EXPECT_EQ(SQL_ERROR, rc); + EXPECT_SQLSTATE(SQL_HANDLE_STMT, stmt_, "HY010"); + + SQLCloseCursor(stmt_); +} + // Without the probe there is no value to report a type for; msodbcsql relies on // the same ordering, so this only pins our diagnostic. TEST_F(ColAttributeLiveTest, VariantTypeBeforeProbeIsSequenceError) { diff --git a/mssql-tds/src/datatypes/decoder.rs b/mssql-tds/src/datatypes/decoder.rs index f7530ee3..f658064c 100644 --- a/mssql-tds/src/datatypes/decoder.rs +++ b/mssql-tds/src/datatypes/decoder.rs @@ -520,15 +520,21 @@ impl GenericDecoder { /// As [`Self::read_sql_variant`], but also returns the base type the variant /// declared on the wire. The decoded value alone cannot always recover it — - /// `varchar` and `nvarchar` both arrive as [`ColumnValues::String`]. + /// `varchar` and `nvarchar` both arrive as [`ColumnValues::String`]. The base + /// is `None` for a NULL variant, which carries no type on the wire. async fn read_sql_variant_with_base( &self, reader: &mut T, - ) -> TdsResult<(TdsDataType, ColumnValues)> + ) -> TdsResult<(Option, ColumnValues)> where T: TdsPacketReader + Send + Sync, { let length = reader.read_uint32().await?; + // A NULL variant is the zero length and nothing else: reading a base type + // and property byte here would consume the next token's bytes. + if length == 0 { + return Ok((None, ColumnValues::Null)); + } let variant_base_type = reader.read_byte().await?; let tds_type = TdsDataType::try_from(variant_base_type)?; let variant_prop_bytes = reader.read_byte().await?; @@ -568,7 +574,7 @@ impl GenericDecoder { ))); } }; - Ok((tds_type, col_value)) + Ok((Some(tds_type), col_value)) } async fn decode_zero_propbyte_variant( @@ -1393,7 +1399,10 @@ impl GenericDecoder { // always recover it. TdsDataType::SsVariant => { let (base, value) = self.read_sql_variant_with_base(reader).await?; - writer.write_variant_base_type(col, base); + // A NULL variant has no base type to report. + if let Some(base) = base { + writer.write_variant_base_type(col, base); + } write_column_value(writer, col, value); } @@ -3452,6 +3461,32 @@ mod test { assert_eq!(val, ColumnValues::TinyInt(42)); } + /// A NULL `sql_variant` is a zero length and nothing else. Reading a + /// base type and property byte anyway would consume the following + /// column's bytes, so the sentinel after it is what proves the guard. + /// + /// Not reachable through a normal SQL Server response, which carries + /// NULLs in the NBCROW null bitmap and skips the column entirely, so it + /// has to be exercised here rather than end to end. + #[tokio::test] + async fn decode_into_null_sql_variant_leaves_following_bytes() { + let md = fixed_metadata(TdsDataType::SsVariant, 0); + let mut reader = ByteReader::new(vec![0, 0, 0, 0, 0xAB, 0xCD]); + let decoder = GenericDecoder::default(); + let mut writer = DefaultRowWriter::new(1); + decoder + .decode_into(&mut reader, &md, 0, &mut writer) + .await + .unwrap(); + + assert_eq!(writer.take_row()[0], ColumnValues::Null); + assert_eq!( + reader.read_byte().await.unwrap(), + 0xAB, + "the NULL variant consumed the following column's bytes" + ); + } + #[tokio::test] async fn decode_into_int2() { let md = fixed_metadata(TdsDataType::Int2, 2); From 280e3170883a7db0949b8f2d1877341ec53b719c Mon Sep 17 00:00:00 2001 From: David Engel Date: Thu, 13 Aug 2026 23:35:59 +0000 Subject: [PATCH 10/11] odbc: advertise SQLColAttribute in SQLGetFunctions The Windows Driver Manager answers IM001 "Driver does not support this function" without ever calling the driver unless the function is advertised through SQLGetFunctions, so every SQLColAttribute test failed on the Windows leg while passing on Linux, where unixODBC dispatches regardless. The one Windows test that passed was the NULL-variant case, which is the only one that never calls SQLColAttribute. This is the same trap as AB#46973, which caught SQLGetTypeInfo and SQLSetStmtAttr; adding the matching guard test alongside those two, so exporting an entry point without registering it fails a unit test rather than only the Windows CI leg. 559 mssql-odbc and 1655 mssql-tds unit tests pass; e2e reports 20 parity, 0 divergences against msodbcsql. --- mssql-odbc/src/api/get_functions.rs | 30 +++++++++++++++++++++-------- mssql-odbc/src/api/odbc_types.rs | 1 + 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/mssql-odbc/src/api/get_functions.rs b/mssql-odbc/src/api/get_functions.rs index 82b592ea..abffd3ad 100644 --- a/mssql-odbc/src/api/get_functions.rs +++ b/mssql-odbc/src/api/get_functions.rs @@ -8,14 +8,14 @@ use tracing::{debug, error}; use crate::api::odbc_types::{ SQL_API_ALL_FUNCTIONS, SQL_API_ALL_FUNCTIONS_SIZE, SQL_API_ODBC3_ALL_FUNCTIONS, SQL_API_SQLALLOCHANDLE, SQL_API_SQLBINDPARAMETER, SQL_API_SQLCANCEL, SQL_API_SQLCLOSECURSOR, - SQL_API_SQLCONNECT, SQL_API_SQLDESCRIBECOL, SQL_API_SQLDISCONNECT, SQL_API_SQLDRIVERCONNECT, - SQL_API_SQLENDTRAN, SQL_API_SQLEXECDIRECT, SQL_API_SQLEXECUTE, SQL_API_SQLFETCH, - SQL_API_SQLFREEHANDLE, SQL_API_SQLFREESTMT, SQL_API_SQLGETCONNECTATTR, SQL_API_SQLGETDATA, - SQL_API_SQLGETDIAGFIELD, SQL_API_SQLGETDIAGREC, SQL_API_SQLGETENVATTR, SQL_API_SQLGETFUNCTIONS, - SQL_API_SQLGETINFO, SQL_API_SQLGETSTMTATTR, SQL_API_SQLGETTYPEINFO, SQL_API_SQLMORERESULTS, - SQL_API_SQLNUMRESULTCOLS, SQL_API_SQLPREPARE, SQL_API_SQLROWCOUNT, SQL_API_SQLSETCONNECTATTR, - SQL_API_SQLSETENVATTR, SQL_API_SQLSETSTMTATTR, SQL_ERROR, SQL_FALSE, SQL_INVALID_HANDLE, - SQL_SUCCESS, SQL_TRUE, SqlHandle, SqlReturn, SqlUSmallInt, + SQL_API_SQLCOLATTRIBUTE, SQL_API_SQLCONNECT, SQL_API_SQLDESCRIBECOL, SQL_API_SQLDISCONNECT, + SQL_API_SQLDRIVERCONNECT, SQL_API_SQLENDTRAN, SQL_API_SQLEXECDIRECT, SQL_API_SQLEXECUTE, + SQL_API_SQLFETCH, SQL_API_SQLFREEHANDLE, SQL_API_SQLFREESTMT, SQL_API_SQLGETCONNECTATTR, + SQL_API_SQLGETDATA, SQL_API_SQLGETDIAGFIELD, SQL_API_SQLGETDIAGREC, SQL_API_SQLGETENVATTR, + SQL_API_SQLGETFUNCTIONS, SQL_API_SQLGETINFO, SQL_API_SQLGETSTMTATTR, SQL_API_SQLGETTYPEINFO, + SQL_API_SQLMORERESULTS, SQL_API_SQLNUMRESULTCOLS, SQL_API_SQLPREPARE, SQL_API_SQLROWCOUNT, + SQL_API_SQLSETCONNECTATTR, SQL_API_SQLSETENVATTR, SQL_API_SQLSETSTMTATTR, SQL_ERROR, SQL_FALSE, + SQL_INVALID_HANDLE, SQL_SUCCESS, SQL_TRUE, SqlHandle, SqlReturn, SqlUSmallInt, }; use crate::error::free_errors; use crate::handles::{DbcHandle, HandleType, handle_from_raw}; @@ -167,6 +167,7 @@ fn supported_function_ids() -> &'static [SqlUSmallInt] { SQL_API_SQLPREPARE, SQL_API_SQLBINDPARAMETER, SQL_API_SQLENDTRAN, + SQL_API_SQLCOLATTRIBUTE, ] } @@ -240,6 +241,19 @@ mod tests { } } + // Same trap as AB#46973: SQLColAttributeW is exported and implemented, but + // the Windows Driver Manager answers IM001 without ever calling the driver + // unless it is advertised here. unixODBC dispatches regardless, so this only + // shows up on the Windows leg. + #[test] + fn col_attribute_reports_true() { + let h = TestHandles::with_env_dbc(); + let mut supported: SqlUSmallInt = SQL_FALSE; + let ret = unsafe { sql_get_functions(h.dbc, SQL_API_SQLCOLATTRIBUTE, &mut supported) }; + assert_eq!(ret, SQL_SUCCESS); + assert_eq!(supported, SQL_TRUE); + } + #[test] fn unsupported_function_reports_false() { let h = TestHandles::with_env_dbc(); diff --git a/mssql-odbc/src/api/odbc_types.rs b/mssql-odbc/src/api/odbc_types.rs index 8fc27c50..0852c161 100644 --- a/mssql-odbc/src/api/odbc_types.rs +++ b/mssql-odbc/src/api/odbc_types.rs @@ -140,6 +140,7 @@ pub const SQL_API_ODBC3_ALL_FUNCTIONS_SIZE: usize = 250; pub const SQL_API_SQLCONNECT: SqlUSmallInt = 7; pub const SQL_API_SQLCANCEL: SqlUSmallInt = 5; pub const SQL_API_SQLDESCRIBECOL: SqlUSmallInt = 8; +pub const SQL_API_SQLCOLATTRIBUTE: SqlUSmallInt = 6; pub const SQL_API_SQLDISCONNECT: SqlUSmallInt = 9; pub const SQL_API_SQLEXECDIRECT: SqlUSmallInt = 11; pub const SQL_API_SQLEXECUTE: SqlUSmallInt = 12; From fa0c127978dc96ee9805369cac8ca59ffbd24b2c Mon Sep 17 00:00:00 2001 From: David Engel Date: Fri, 14 Aug 2026 16:22:36 +0000 Subject: [PATCH 11/11] test: cover the attribute, variant and probe mapping tables Coverage on the new code sat at 84% because the unit tests could only build `int` column metadata, leaving the per-type mapping tables in SQLColAttributeW reachable only from the C++ e2e suite. The metadata fields are public, so a column produced by `int_columns` can be retyped in place. That unlocks direct tests for the whole of `type_name`, `num_prec_radix`, `octet_length`, `precision` and `is_unsigned`, plus the `sql_variant` success path, without a live server. `TypeInfo::partial_len` and `var_len_precision_scale` cover the PLP and declared-precision branches the same way. Also adds a table test for the C type each `sql_variant` base maps to, one for the byte count a zero-length SQL_C_BINARY probe reports, one for the base types a DefaultRowWriter keys to their column, and one decoding a non-NULL variant end to end so the reported base type is checked against the wire header rather than the decoded value. col_attribute.rs goes from 72% to 99.7% line coverage and 100% of its functions. Patch coverage across the change is 98.7%; what remains is the live-cursor path in tds_client, which needs a connection, and a poisoned-mutex branch. No production code changes. --- mssql-odbc/src/api/col_attribute.rs | 314 +++++++++++++++++++++++++- mssql-odbc/src/api/get_data.rs | 38 ++++ mssql-tds/src/datatypes/decoder.rs | 23 ++ mssql-tds/src/datatypes/row_writer.rs | 25 ++ 4 files changed, 398 insertions(+), 2 deletions(-) diff --git a/mssql-odbc/src/api/col_attribute.rs b/mssql-odbc/src/api/col_attribute.rs index f9bd79e2..1c425598 100644 --- a/mssql-odbc/src/api/col_attribute.rs +++ b/mssql-odbc/src/api/col_attribute.rs @@ -409,12 +409,12 @@ fn type_name(meta: &ColumnMetadata) -> &'static str { mod tests { use std::ptr; - use mssql_tds::test_client_support::int_columns; - use super::*; use crate::api::odbc_types::{SQL_INTEGER, SQL_NULLABLE}; use crate::api::sqlstate::ERR_INVALID_DESCRIPTOR_FIELD; use crate::test_support::TestHandles; + use mssql_tds::datatypes::sqldatatypes::TypeInfo; + use mssql_tds::test_client_support::int_columns; /// A statement positioned on a result set of `n` nullable `int` columns. fn stmt_with_int_columns(h: &TestHandles, n: usize) { @@ -639,6 +639,316 @@ mod tests { ); } + /// Retypes column `col` (1-based) in place. `int_columns` is the only + /// metadata constructor available here, and the fields are public, so this + /// is how the per-type mapping tables get exercised without a live server. + fn retype_column(h: &TestHandles, col: usize, data_type: TdsDataType, length: usize) { + let stmt_handle = unsafe { handle_from_raw::(h.stmt) }; + let mut s = stmt_handle.inner.lock().unwrap(); + let meta = &mut s.column_metadata[col - 1]; + meta.data_type = data_type; + meta.type_info.tds_type = data_type; + meta.type_info.length = length; + } + + /// Every numeric attribute this driver reports, on one `int` column. + #[test] + fn every_numeric_attribute_answers_for_an_int_column() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 1); + + assert_eq!(numeric(&h, 1, SQL_DESC_LENGTH), 10); + assert_eq!(numeric(&h, 1, SQL_DESC_DISPLAY_SIZE), 10); + assert_eq!(numeric(&h, 1, SQL_DESC_OCTET_LENGTH), 4); + assert_eq!(numeric(&h, 1, SQL_DESC_PRECISION), 10); + assert_eq!(numeric(&h, 1, SQL_DESC_SCALE), 0); + assert_eq!(numeric(&h, 1, SQL_DESC_CASE_SENSITIVE), 0); + assert_eq!(numeric(&h, 1, SQL_DESC_FIXED_PREC_SCALE), 0); + assert_eq!( + numeric(&h, 1, SQL_DESC_UPDATABLE), + SQL_ATTR_READWRITE_UNKNOWN + ); + assert_eq!(numeric(&h, 1, SQL_DESC_AUTO_UNIQUE_VALUE), 0); + assert_eq!(numeric(&h, 1, SQL_DESC_SEARCHABLE), SQL_PRED_SEARCHABLE); + } + + /// A column with no name reports `SQL_UNNAMED`; `int_columns` names them. + #[test] + fn unnamed_column_is_reported() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 1); + { + let stmt_handle = unsafe { handle_from_raw::(h.stmt) }; + let mut s = stmt_handle.inner.lock().unwrap(); + s.column_metadata[0].column_name.clear(); + } + assert_eq!(numeric(&h, 1, SQL_DESC_UNNAMED), SQL_UNNAMED); + } + + /// A non-nullable column reports `SQL_NO_NULLS`. `int_columns` sets the + /// nullable flag, so clear it. + #[test] + fn not_nullable_column_is_reported() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 1); + { + let stmt_handle = unsafe { handle_from_raw::(h.stmt) }; + let mut s = stmt_handle.inner.lock().unwrap(); + s.column_metadata[0].flags &= !0x01; + } + assert_eq!( + numeric(&h, 1, SQL_DESC_NULLABLE), + SqlLen::from(SQL_NO_NULLS) + ); + } + + #[test] + fn type_name_and_radix_track_the_column_type() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 1); + + // (type, wire length, expected type name, expected radix) + let cases: &[(TdsDataType, usize, &str, SqlLen)] = &[ + (TdsDataType::Int1, 1, "tinyint", 10), + (TdsDataType::Int2, 2, "smallint", 10), + (TdsDataType::Int8, 8, "bigint", 10), + (TdsDataType::Flt4, 4, "real", 2), + (TdsDataType::Flt8, 8, "float", 2), + (TdsDataType::MoneyN, 8, "money", 10), + (TdsDataType::Guid, 16, "uniqueidentifier", 0), + (TdsDataType::BigVarChar, 10, "varchar", 0), + (TdsDataType::NVarChar, 20, "nvarchar", 0), + (TdsDataType::SsVariant, 8, "sql_variant", 0), + ]; + for (ty, len, name, radix) in cases { + retype_column(&h, 1, *ty, *len); + assert_eq!(numeric(&h, 1, SQL_DESC_NUM_PREC_RADIX), *radix, "{ty:?}"); + assert_eq!( + numeric(&h, 1, SQL_DESC_OCTET_LENGTH), + *len as SqlLen, + "{ty:?}" + ); + + let mut buf = [0u16; 32]; + let mut written: SqlSmallInt = 0; + let rc = unsafe { + sql_col_attribute_w( + h.stmt, + 1, + SQL_DESC_TYPE_NAME, + buf.as_mut_ptr() as SqlPointer, + (buf.len() * 2) as SqlSmallInt, + &mut written, + ptr::null_mut(), + ) + }; + assert_eq!(rc, SQL_SUCCESS, "{ty:?}"); + let got = String::from_utf16_lossy(&buf[..(written as usize) / 2]); + assert_eq!(got, *name, "{ty:?}"); + } + } + + /// `tinyint` is the only unsigned integer, and it is the one type this + /// driver deliberately reports as unsigned. + #[test] + fn unsigned_is_reported_only_for_tinyint() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 1); + retype_column(&h, 1, TdsDataType::Int1, 1); + assert_eq!(numeric(&h, 1, SQL_DESC_UNSIGNED), 1); + retype_column(&h, 1, TdsDataType::IntN, 1); + assert_eq!(numeric(&h, 1, SQL_DESC_UNSIGNED), 1); + retype_column(&h, 1, TdsDataType::IntN, 4); + assert_eq!(numeric(&h, 1, SQL_DESC_UNSIGNED), 0); + } + + /// The C type reported for each base type a `sql_variant` can carry. + /// Exercised directly because a variant's base type is a property of the + /// value, which unit tests cannot produce. + #[test] + fn variant_c_type_covers_the_base_types() { + let cases: &[(TdsDataType, SqlSmallInt)] = &[ + (TdsDataType::Int1, SQL_C_TINYINT), + (TdsDataType::Int2, SQL_C_SSHORT), + (TdsDataType::Int4, SQL_C_SLONG), + (TdsDataType::Int8, SQL_C_SBIGINT), + (TdsDataType::Bit, SQL_C_BIT), + (TdsDataType::Flt4, SQL_C_FLOAT), + (TdsDataType::Flt8, SQL_C_DOUBLE), + // The exact numerics are advertised as character data because + // SQL_NUMERIC_STRUCT is a permanent non-goal. + (TdsDataType::Numeric, SQL_C_CHAR), + (TdsDataType::MoneyN, SQL_C_CHAR), + (TdsDataType::DateN, SQL_C_TYPE_DATE), + (TdsDataType::TimeN, SQL_C_SS_TIME2), + (TdsDataType::DateTimeN, SQL_C_TYPE_TIMESTAMP), + (TdsDataType::DateTime2N, SQL_C_TYPE_TIMESTAMP), + (TdsDataType::DateTimeOffsetN, SQL_C_SS_TIMESTAMPOFFSET), + (TdsDataType::BigVarChar, SQL_C_CHAR), + (TdsDataType::NVarChar, SQL_C_WCHAR), + (TdsDataType::BigVarBinary, SQL_C_BINARY), + (TdsDataType::Guid, SQL_C_GUID), + // A variant cannot carry these, so character is the fallback. + (TdsDataType::Xml, SQL_C_CHAR), + ]; + for (base, expected) in cases { + assert_eq!(variant_c_type(*base), *expected, "{base:?}"); + } + } + + /// The success path: a variant column whose value has been probed reports + /// that value's underlying C type. + #[test] + fn variant_type_is_reported_after_the_value_is_probed() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 2); + retype_column(&h, 1, TdsDataType::SsVariant, 8); + { + let stmt_handle = unsafe { handle_from_raw::(h.stmt) }; + let mut s = stmt_handle.inner.lock().unwrap(); + s.last_variant_base = Some((1, TdsDataType::NVarChar)); + } + assert_eq!( + numeric(&h, 1, SQL_CA_SS_VARIANT_TYPE), + SqlLen::from(SQL_C_WCHAR) + ); + } + + /// A base type captured for one column must not answer for another. + #[test] + fn variant_type_does_not_leak_across_columns() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 2); + retype_column(&h, 1, TdsDataType::SsVariant, 8); + retype_column(&h, 2, TdsDataType::SsVariant, 8); + { + let stmt_handle = unsafe { handle_from_raw::(h.stmt) }; + let mut s = stmt_handle.inner.lock().unwrap(); + s.last_variant_base = Some((1, TdsDataType::Int4)); + } + let mut out: SqlLen = 0; + let rc = unsafe { + sql_col_attribute_w( + h.stmt, + 2, + SQL_CA_SS_VARIANT_TYPE, + ptr::null_mut(), + 0, + ptr::null_mut(), + &mut out, + ) + }; + assert_eq!(rc, SQL_ERROR); + let sh = unsafe { handle_from_raw::(h.stmt) }; + let s = sh.inner.lock().unwrap(); + assert_eq!( + s.diag_records.last().unwrap().sql_state, + ERR_FUNCTION_SEQUENCE.state + ); + } + + /// Every arm of the type-name table. Driven directly because a name is a + /// pure function of the metadata and needs no live result set. + #[test] + fn type_name_covers_every_supported_type() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 1); + + let cases: &[(TdsDataType, usize, &str)] = &[ + (TdsDataType::Int1, 1, "tinyint"), + (TdsDataType::Int2, 2, "smallint"), + (TdsDataType::Int4, 4, "int"), + (TdsDataType::Int8, 8, "bigint"), + (TdsDataType::IntN, 1, "tinyint"), + (TdsDataType::IntN, 2, "smallint"), + (TdsDataType::IntN, 4, "int"), + (TdsDataType::IntN, 8, "bigint"), + // A length the server never sends still has to name something. + (TdsDataType::IntN, 3, "int"), + (TdsDataType::Bit, 1, "bit"), + (TdsDataType::BitN, 1, "bit"), + (TdsDataType::Flt4, 4, "real"), + (TdsDataType::Flt8, 8, "float"), + (TdsDataType::FltN, 4, "real"), + (TdsDataType::FltN, 8, "float"), + (TdsDataType::Decimal, 9, "decimal"), + (TdsDataType::DecimalN, 9, "decimal"), + (TdsDataType::Numeric, 9, "numeric"), + (TdsDataType::NumericN, 9, "numeric"), + (TdsDataType::Money, 8, "money"), + (TdsDataType::MoneyN, 8, "money"), + (TdsDataType::Money4, 4, "smallmoney"), + (TdsDataType::DateN, 3, "date"), + (TdsDataType::TimeN, 5, "time"), + (TdsDataType::DateTime, 8, "datetime"), + (TdsDataType::DateTimeN, 8, "datetime"), + (TdsDataType::DateTim4, 4, "smalldatetime"), + (TdsDataType::DateTime2N, 8, "datetime2"), + (TdsDataType::DateTimeOffsetN, 10, "datetimeoffset"), + (TdsDataType::Char, 10, "char"), + (TdsDataType::BigChar, 10, "char"), + (TdsDataType::VarChar, 10, "varchar"), + (TdsDataType::BigVarChar, 10, "varchar"), + (TdsDataType::Text, 16, "text"), + (TdsDataType::NChar, 20, "nchar"), + (TdsDataType::NVarChar, 20, "nvarchar"), + (TdsDataType::NText, 16, "ntext"), + (TdsDataType::Binary, 8, "binary"), + (TdsDataType::BigBinary, 8, "binary"), + (TdsDataType::VarBinary, 8, "varbinary"), + (TdsDataType::BigVarBinary, 8, "varbinary"), + (TdsDataType::Image, 16, "image"), + (TdsDataType::Guid, 16, "uniqueidentifier"), + (TdsDataType::Xml, 0, "xml"), + (TdsDataType::Json, 0, "json"), + (TdsDataType::Vector, 0, "vector"), + (TdsDataType::SsVariant, 8, "sql_variant"), + (TdsDataType::Udt, 0, "udt"), + (TdsDataType::Void, 0, "unknown"), + ]; + for (ty, len, name) in cases { + retype_column(&h, 1, *ty, *len); + let stmt_handle = unsafe { handle_from_raw::(h.stmt) }; + let s = stmt_handle.inner.lock().unwrap(); + assert_eq!(type_name(&s.column_metadata[0]), *name, "{ty:?} len {len}"); + } + } + + /// A `varchar(max)` streams as PLP, which has no fixed octet length, so + /// the driver reports zero rather than the sentinel wire length. + #[test] + fn plp_column_reports_zero_octet_length() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 1); + { + let stmt_handle = unsafe { handle_from_raw::(h.stmt) }; + let mut s = stmt_handle.inner.lock().unwrap(); + let meta = &mut s.column_metadata[0]; + meta.data_type = TdsDataType::BigVarChar; + meta.type_info = TypeInfo::partial_len(TdsDataType::BigVarChar, 0xFFFF, None) + .expect("varchar(max) is a PLP type"); + } + assert_eq!(numeric(&h, 1, SQL_DESC_OCTET_LENGTH), 0); + } + + /// A `decimal` carries its own precision on the wire, which takes + /// precedence over the display size fallback. + #[test] + fn decimal_reports_its_declared_precision() { + let h = TestHandles::with_env_dbc_stmt(); + stmt_with_int_columns(&h, 1); + { + let stmt_handle = unsafe { handle_from_raw::(h.stmt) }; + let mut s = stmt_handle.inner.lock().unwrap(); + let meta = &mut s.column_metadata[0]; + meta.data_type = TdsDataType::DecimalN; + meta.type_info = TypeInfo::var_len_precision_scale(TdsDataType::DecimalN, 9, 18, 4) + .expect("decimal carries precision and scale"); + } + assert_eq!(numeric(&h, 1, SQL_DESC_PRECISION), 18); + assert_eq!(numeric(&h, 1, SQL_DESC_SCALE), 4); + } + /// mssql-python passes a null string buffer and reads only the numeric /// attribute, so a null `character_attribute_ptr` must not fault. #[test] diff --git a/mssql-odbc/src/api/get_data.rs b/mssql-odbc/src/api/get_data.rs index cfe0c660..5f727d59 100644 --- a/mssql-odbc/src/api/get_data.rs +++ b/mssql-odbc/src/api/get_data.rs @@ -1412,6 +1412,44 @@ mod tests { s.last_captured = Some((1, value)); } + /// The byte count a probe reports for each value kind. Variable-length + /// values report their real size; fixed-width values report their wire + /// width; anything without a defined binary form reports SQL_NO_TOTAL so + /// the caller falls back to reading without a size hint. + #[test] + fn binary_length_covers_the_value_kinds() { + use mssql_tds::datatypes::column_values::SqlXml; + use mssql_tds::datatypes::sql_string::SqlString; + + let cases: &[(ColumnValues, SqlLen)] = &[ + (ColumnValues::Bytes(vec![1, 2, 3]), 3), + // A SqlString holds the bytes as they came off the wire, so a + // four-character UTF-16 string is eight bytes. + ( + ColumnValues::String(SqlString::from_utf8_string("abcd".to_string())), + 8, + ), + ( + ColumnValues::Xml(SqlXml { + bytes: vec![0x41, 0x42], + }), + 2, + ), + (ColumnValues::Bit(true), 1), + (ColumnValues::TinyInt(1), 1), + (ColumnValues::SmallInt(1), 2), + (ColumnValues::Int(1), 4), + (ColumnValues::Real(1.0), 4), + (ColumnValues::BigInt(1), 8), + (ColumnValues::Float(1.0), 8), + (ColumnValues::Uuid(uuid::Uuid::nil()), 16), + (ColumnValues::Null, SQL_NO_TOTAL), + ]; + for (value, expected) in cases { + assert_eq!(binary_length(value), *expected, "{value:?}"); + } + } + /// A zero-length SQL_C_BINARY read reports the available length and leaves /// the value resident, so the caller can still read it for real afterwards. /// This is the probe mssql-python issues on every sql_variant column. diff --git a/mssql-tds/src/datatypes/decoder.rs b/mssql-tds/src/datatypes/decoder.rs index f658064c..e7b3f57a 100644 --- a/mssql-tds/src/datatypes/decoder.rs +++ b/mssql-tds/src/datatypes/decoder.rs @@ -3487,6 +3487,29 @@ mod test { ); } + /// A non-NULL variant reports its base type alongside the value, which + /// is the whole reason the base type is threaded through the writer: + /// an `int` and a `varchar` variant both decode to distinct + /// `ColumnValues`, but only the wire header says what the column was + /// declared to hold. + #[tokio::test] + async fn decode_into_sql_variant_reports_the_base_type() { + let md = fixed_metadata(TdsDataType::SsVariant, 0); + // 6-byte payload: base type INT4 (0x38), zero property bytes, then + // the four value bytes. 0xAB trails to prove nothing overreads. + let mut reader = ByteReader::new(vec![6, 0, 0, 0, 0x38, 0x00, 42, 0, 0, 0, 0xAB]); + let decoder = GenericDecoder::default(); + let mut writer = DefaultRowWriter::new(1); + decoder + .decode_into(&mut reader, &md, 0, &mut writer) + .await + .unwrap(); + + assert_eq!(writer.variant_base(0), Some(TdsDataType::Int4)); + assert_eq!(writer.take_row()[0], ColumnValues::Int(42)); + assert_eq!(reader.read_byte().await.unwrap(), 0xAB); + } + #[tokio::test] async fn decode_into_int2() { let md = fixed_metadata(TdsDataType::Int2, 2); diff --git a/mssql-tds/src/datatypes/row_writer.rs b/mssql-tds/src/datatypes/row_writer.rs index e96a68b7..c4b62f5e 100644 --- a/mssql-tds/src/datatypes/row_writer.rs +++ b/mssql-tds/src/datatypes/row_writer.rs @@ -425,4 +425,29 @@ mod tests { assert_eq!(row[6], ColumnValues::Bit(false)); assert_eq!(row[7], ColumnValues::Null); } + + /// A variant's base type is keyed to the position the value lands in, so a + /// row mixing variant and non-variant columns reports the right base for + /// each, and nothing for the others. + #[test] + fn default_row_writer_keys_variant_base_types_to_their_column() { + let mut writer = DefaultRowWriter::new(3); + + writer.write_i32(0, 1); + writer.write_variant_base_type(1, TdsDataType::NVarChar); + writer.write_string(1, SqlString::new(vec![0x41, 0x00], EncodingType::Utf16)); + writer.write_variant_base_type(2, TdsDataType::Int4); + writer.write_i32(2, 7); + writer.end_row(); + + assert_eq!(writer.variant_base(0), None); + assert_eq!(writer.variant_base(1), Some(TdsDataType::NVarChar)); + assert_eq!(writer.variant_base(2), Some(TdsDataType::Int4)); + // Out of range is simply "not a variant". + assert_eq!(writer.variant_base(9), None); + + // Taking the row clears the bases so the writer can be reused. + assert_eq!(writer.take_row().len(), 3); + assert_eq!(writer.variant_base(1), None); + } }