odbc: SQLColAttributeW and the sql_variant underlying-type chain (P2) [AB#46579] by David-Engel · Pull Request #278 · microsoft/mssql-rs

David Engel (David-Engel) · 2026-08-13T19:01:30Z

What

Implements SQLColAttributeW (P2), plus the two things the sql_variant path turned out to depend on.

mssql-python resolves SQLColAttributeW at load time and includes the pointer in its non-null check, so without this export the driver does not load at all.

The variant chain

Reading a sql_variant column takes three things, not one, and mssql-python needs all of them. 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 — worth knowing when reviewing, because two of these links look unrelated to this work item:

SQLDescribeCol must report SQL_SS_VARIANT. We reported SQL_VARCHAR, and mssql-python branches on the exact type, so it never entered the variant path and treated every variant column as a string.
SQLGetData(col, SQL_C_BINARY, NULL, 0, &indicator) must succeed. This is a length/NULL probe, not a data read.
SQLColAttribute(SQL_CA_SS_VARIANT_TYPE) returns the C type of the value just probed.

How the underlying type is carried

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. The decoded ColumnValues cannot always recover it, since varchar and nvarchar both arrive as ColumnValues::String.

So it is carried up from the decoder:

RowWriter gains a defaulted write_variant_base_type, so PyRowWriter and the mssql-js writer compile unchanged.
decode_into gets an explicit SsVariant arm that reports the base type before writing the value.
CursorColumn::Value carries it alongside the value, mirroring how PlpStreaming already carries its collation. CursorColumn is used only by mssql-odbc, so widening it is contained.
StmtState holds it next to last_captured and clears it with the rest of the row-stream state.

ColumnValues is deliberately untouched. That is what keeps this change out of the Python and Node bindings — mssql-py-core matches on it in ~300 places.

Reviewer notes

Behaviour change: sql_variant columns now describe as SQL_SS_VARIANT (−150) instead of SQL_VARCHAR. Applications that keyed on the old value will see the new one, which is what msodbcsql reports.
Deliberate divergence: for a variant holding decimal/numeric/money, SQL_CA_SS_VARIANT_TYPE reports SQL_C_CHAR where msodbcsql reports SQL_C_NUMERIC. Reporting the latter would make the caller request a SQL_NUMERIC_STRUCT this driver refuses; character is how those values are actually delivered. Recorded in the divergence table.
Scope call: the SQL_C_BINARY probe arguably belongs to AB#47239. I took the zero-length form here because P2 ships unreachable code otherwise, and left binary delivery on that item — a real buffer is still HYC00. AB#47239 has been updated to say so.
HY113 for the not-a-variant case matches msodbcsql's IDS_S1_113; its S1 prefix is the ODBC 2.x spelling of HY (cf. IDS_S1_C00 = HYC00).

Testing

533 unit tests, workspace clippy and cargo fmt clean. The mssql-tds changes were also validated against SQL Server 2025 in Docker: 1647 unit tests and all 24 test_client_read_apis integration tests pass.

New e2e col_attribute_test.cpp, because unit tests can only build int column metadata — the per-type mapping tables and the entire variant path had no coverage otherwise. The variant test reads two rows holding different base types in the same column, which is what pins the type to the value rather than the column.

Every hand-written SQL_DESC_* constant was cross-checked against the real ODBC headers with static_assert; all 26 match.

AB#46579

Copilot

Pull request overview

Implements SQLColAttributeW and propagates per-value sql_variant base types from TDS decoding into the ODBC layer.

Changes:

Adds SQLColAttributeW descriptor handling and export.
Supports SQL_SS_VARIANT metadata and binary probes.
Adds unit and live-server coverage.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`mssql-tds/tests/test_client_read_apis.rs`	Updates cursor assertions.
`mssql-tds/src/datatypes/row_writer.rs`	Captures variant base types.
`mssql-tds/src/datatypes/decoder.rs`	Propagates variant base types.
`mssql-tds/src/connection/tds_client.rs`	Extends cursor values with variant metadata.
`mssql-odbc/tests/e2e/tests/col_attribute_test.cpp`	Adds live `SQLColAttributeW` tests.
`mssql-odbc/tests/e2e/CMakeLists.txt`	Registers the new test.
`mssql-odbc/src/handles/stmt.rs`	Stores per-row variant state.
`mssql-odbc/src/api/sqlstate.rs`	Adds descriptor diagnostics.
`mssql-odbc/src/api/odbc_types.rs`	Adds descriptor constants.
`mssql-odbc/src/api/mod.rs`	Registers the API module.
`mssql-odbc/src/api/get_data.rs`	Supports zero-length binary probes.
`mssql-odbc/src/api/exports.rs`	Exports `SQLColAttributeW`.
`mssql-odbc/src/api/describe_col.rs`	Reports `SQL_SS_VARIANT`.
`mssql-odbc/src/api/col_attribute.rs`	Implements descriptor attributes.
`mssql-odbc/docs/typed-columnar-fetch-plan.md`	Documents P2 completion and divergences.

Suppressed comments (3)

mssql-odbc/src/api/col_attribute.rs:211

SQL_DESC_TYPE cannot always equal SQL_DESC_CONCISE_TYPE. For date/time columns the verbose type must be SQL_DATETIME, while the concise field is SQL_TYPE_DATE, SQL_TYPE_TIMESTAMP, or the corresponding subtype. Returning the concise identifier for both breaks descriptor consumers; split these cases and add the verbose datetime mapping.

        // `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)),

mssql-odbc/src/api/col_attribute.rs:241

This always reports SQL_FALSE for identity columns even though ColumnMetadata::is_identity() exposes the TDS fIdentity flag. SQL_DESC_AUTO_UNIQUE_VALUE should reflect that flag so consumers can identify auto-increment columns.

        SQL_DESC_AUTO_UNIQUE_VALUE => Attr::Numeric(SqlLen::from(false)),

mssql-odbc/src/api/col_attribute.rs:214

SQL_DESC_DISPLAY_SIZE is not the same as column size. For example, this reports 10 for a signed INT, although displaying -2147483648 requires 11 characters; binary values require twice their octet length, and scaled numerics need sign/decimal-point space. Applications sizing character buffers from this result can truncate values. Give display size its own per-type mapping.

        SQL_DESC_LENGTH | SQL_DESC_DISPLAY_SIZE => {
            Attr::Numeric(SqlLen::try_from(column_size(meta)).unwrap_or(SqlLen::MAX))
        }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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.

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.

…YPE (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-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.

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.

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.

…ot 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.

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.

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<TdsDataType>, 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.

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.

github-actions · 2026-08-14T00:47:57Z

📊 Code Coverage Report

🔥 Diff Coverage 99%	🎯 Overall Coverage 91.6%	📦 Project: `mssql-tds + mssql-odbc + mssql-py-core` ℹ️ Note: diff coverage is reported, not enforced.

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

mssql-odbc/src/api/col_attribute.rs (99.7%): Missing lines 120-121
mssql-odbc/src/api/describe_col.rs (80.0%): Missing lines 216
mssql-odbc/src/api/exports.rs (100%)
mssql-odbc/src/api/get_data.rs (98.9%): Missing lines 926
mssql-odbc/src/api/get_functions.rs (100%)
mssql-odbc/src/handles/stmt.rs (100%)
mssql-tds/src/connection/tds_client.rs (100%)
mssql-tds/src/datatypes/decoder.rs (97.9%): Missing lines 1408
mssql-tds/src/datatypes/row_writer.rs (100%)

Summary

Total: 837 lines
Missing: 5 lines
Coverage: 99%

mssql-odbc/src/api/col_attribute.rs

  116     string_length_ptr: *mut SqlSmallInt,
  117     numeric_attribute_ptr: *mut SqlLen,
  118 ) -> SqlReturn {
  119     let Ok(mut stmt_state) = stmt.inner.lock() else {
! 120         error!("SQLColAttributeW: stmt mutex poisoned");
! 121         return SQL_ERROR;
  122     };
  123 
  124     free_errors(&mut stmt_state);

mssql-odbc/src/api/describe_col.rs

  212         TdsDataType::Xml | TdsDataType::Json => SQL_WLONGVARCHAR,
  213         // mssql-python keys its sql_variant handling off this exact type, so
  214         // reporting the column as character data hides the variant entirely.
  215         TdsDataType::SsVariant => SQL_SS_VARIANT,
! 216         TdsDataType::Vector | TdsDataType::Udt => SQL_VARCHAR,
  217         _ => SQL_UNKNOWN_TYPE,
  218     }
  219 }

mssql-odbc/src/api/get_data.rs

  922     let len = match value {
  923         ColumnValues::Bytes(b) => b.len(),
  924         ColumnValues::String(s) => s.bytes.len(),
  925         ColumnValues::Xml(x) => x.bytes.len(),
! 926         ColumnValues::Json(j) => j.bytes.len(),
  927         ColumnValues::Bit(_) | ColumnValues::TinyInt(_) => 1,
  928         ColumnValues::SmallInt(_) => 2,
  929         ColumnValues::Int(_) | ColumnValues::Real(_) | ColumnValues::SmallMoney(_) => 4,
  930         ColumnValues::BigInt(_) | ColumnValues::Float(_) | ColumnValues::Money(_) => 8,

mssql-tds/src/datatypes/decoder.rs

  1404                     writer.write_variant_base_type(col, base);
  1405                 }
  1406                 write_column_value(writer, col, value);
  1407             }
! 1408 
  1409             // === Fallback: rare types go through decode() → write_column_value() ===
  1410             _ => {
  1411                 let value = self.decode(reader, metadata).await?;
  1412                 write_column_value(writer, col, value);

🔗 Quick Links

View Azure DevOps Build · Coverage Report

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.

Saurabh Singh (saurabh500)

Requesting changes for the two blocking ODBC descriptor-size correctness issues. See the inline comments for details and the associated should-fix items.

Saurabh Singh (saurabh500) · 2026-08-14T23:11:45Z

+        // `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 => {


Severity: Blocking — SQL_DESC_DISPLAY_SIZE cannot share column_size. These are different ODBC attributes: int needs 11 display characters (sign plus 10 digits), uniqueidentifier needs 36, and binary values need two characters per byte. Returning 10, 16, and n respectively can make callers allocate undersized character buffers and truncate valid values. Please use a type-specific display-size mapping that matches msodbcsql.

Saurabh Singh (saurabh500) · 2026-08-14T23:11:45Z

+    Some(attr)
+}
+
+/// Storage size in bytes of the column's value on the wire.


Severity: Blocking — SQL_DESC_OCTET_LENGTH is the ODBC transfer/descriptor size, not the TDS wire payload length. For example, a TDS date payload is 3 bytes while the default ODBC SQL_DATE_STRUCT is 6 bytes, and timestamp structures require 16 bytes. Reporting the wire length can make applications allocate buffers that are too small. Please map this by ODBC SQL type and transfer representation.

Saurabh Singh (saurabh500) · 2026-08-14T23:11:45Z

+    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)),


Severity: Should fix — SQL_DESC_TYPE is the verbose type field. For date, time, and timestamp columns it must return SQL_DATETIME (with the subtype reported separately), while SQL_DESC_CONCISE_TYPE returns SQL_TYPE_DATE, SQL_TYPE_TIME, or SQL_TYPE_TIMESTAMP. Treating the two fields as identical breaks descriptor consumers.

Saurabh Singh (saurabh500) · 2026-08-14T23:11:45Z

+
+/// `SQL_DESC_PRECISION`: the number of significant digits for the exact and
+/// approximate numeric types, otherwise the column size.
+fn precision(meta: &ColumnMetadata) -> SqlSmallInt {


Severity: Should fix — Falling back to column_size does not implement SQL_DESC_PRECISION. Approximate numerics need binary precision (real = 24 and float = 53), and temporal types report fractional-second precision rather than display width. Please make this mapping type-specific instead of reusing the display-oriented size.

Saurabh Singh (saurabh500) · 2026-08-14T23:11:45Z

+    }
+}
+
+/// `tinyint` is the only unsigned integer SQL Server exposes.


Severity: Should fix — The ODBC rule is broader than “tinyint is unsigned”: SQL_DESC_UNSIGNED is SQL_TRUE for unsigned numeric columns or for nonnumeric columns, and SQL_FALSE for signed numeric columns. This helper therefore returns the wrong value for character, date/time, GUID, and other nonnumeric types.

Saurabh Singh (saurabh500) · 2026-08-14T23:11:45Z

+        // 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),


Severity: Should fix — SQL_PRED_SEARCHABLE claims full comparison and LIKE support for every type. That overstates capability for types that support only basic predicates and for types such as XML/image that are not fully searchable. Please derive SQL_PRED_NONE, SQL_PRED_CHAR, SQL_PRED_BASIC, or SQL_PRED_SEARCHABLE from the SQL type.

Saurabh Singh (saurabh500) · 2026-08-14T23:11:46Z

+        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 => {


Severity: Should fix — SQL_DESC_BASE_COLUMN_NAME is not the result-set label. For SELECT source_col AS alias, SQL_DESC_NAME/SQL_DESC_LABEL may return alias, but the base-column field must return source_col; if that provenance is unavailable, ODBC expects an empty string rather than the alias.

Saurabh Singh (saurabh500) · 2026-08-14T23:11:46Z

 pub enum CursorColumn {
    /// A fully decoded, materialized column value (non-PLP).
-    Value(ColumnValues),
+    Value {


Severity: Should fix — This changes the public CursorColumn::Value(ColumnValues) tuple variant into a struct variant, so downstream constructors and pattern matches no longer compile. If this break is intentional before the initial release, please document it in the changelog or migration notes; otherwise provide a compatibility-preserving way to expose variant_base.

Saurabh Singh (saurabh500) · 2026-08-14T23:11:46Z

            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"


Severity: Should fix — Every updated assertion here either ignores variant_base with .. or checks only None. Nothing directly proves that read_row_column returns Some(base) for a real sql_variant. Please add a non-first variant column with rows containing varchar, nvarchar, and NULL, asserting BigVarChar, NVarChar, and None; that pins output-position association and per-row clearing at this API boundary.

Saurabh Singh (saurabh500) · 2026-08-14T23:11:46Z

+// 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.


Severity: Should fix — Several critical descriptor mappings added in this PR are not exercised here. Please add live msodbcsql parity cases for display size (int, GUID, binary, decimal/float), transfer octet length (date/timestamp), verbose versus concise datetime type, nonnumeric SQL_DESC_UNSIGNED, identity/searchability, and aliased base-column names. The current unit tests execute these mapping arms but lock in values such as an int display size of 10, so they do not catch these contract errors.

David Engel (David-Engel) force-pushed the david/odbc-colattribute branch from ed30154 to 0d20de0 Compare August 13, 2026 20:38

David Engel (David-Engel) requested a balanced review from Copilot August 13, 2026 20:56

Copilot started reviewing on behalf of David Engel (David-Engel) August 13, 2026 20:57 View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Comment thread mssql-tds/src/datatypes/decoder.rs Outdated

Comment thread mssql-odbc/src/api/col_attribute.rs Outdated

David Engel (David-Engel) added 10 commits August 13, 2026 23:32

David Engel (David-Engel) force-pushed the david/odbc-colattribute branch from c438792 to 280e317 Compare August 13, 2026 23:36

David Engel (David-Engel) marked this pull request as ready for review August 14, 2026 16:03

David Engel (David-Engel) requested a review from a team as a code owner August 14, 2026 16:03

Saurabh Singh (saurabh500) requested changes Aug 14, 2026

View reviewed changes

Uh oh!

Conversation

David Engel (David-Engel) commented Aug 13, 2026 • edited by azure-boards Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

The variant chain

How the underlying type is carried

Reviewer notes

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Code Coverage Report

🔥 Diff Coverage

99%

🎯 Overall Coverage

91.6%

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

Summary

mssql-odbc/src/api/col_attribute.rs

mssql-odbc/src/api/describe_col.rs

mssql-odbc/src/api/get_data.rs

mssql-tds/src/datatypes/decoder.rs

🔗 Quick Links

Uh oh!

Saurabh Singh (saurabh500) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

David Engel (David-Engel) commented Aug 13, 2026 •

edited by azure-boards Bot

Loading

github-actions Bot commented Aug 14, 2026 •

edited

Loading