refactor(mssql): use pyodbc native driver, drop ibis dependency#2274
refactor(mssql): use pyodbc native driver, drop ibis dependency#2274goldmedal wants to merge 6 commits into
Conversation
The mssql connector now uses pyodbc directly with custom Arrow type
inference and a datetimeoffset output converter. The mssql extra
installs `pyodbc` instead of `ibis-framework[mssql]`.
Highlights
- `connector/mssql.py`: raw pyodbc cursor; sqlglot-based LIMIT/OFFSET
rewrite (`OFFSET 0 ROWS FETCH NEXT n ROWS ONLY`); Arrow schema built
from `cursor.description` + value sampling; dry_run falls back to
`sys.dm_exec_describe_first_result_set` for precise error messages.
- `model/data_source.py`: `_connect_mssql_pyodbc` builds an ODBC
connection string with proper `{...}` escaping; `mssql://` URL
parsing added; output converter for DATETIMEOFFSET (type code -155)
decodes the 20-byte payload into a tz-aware datetime.
- `pyproject.toml`: mssql extra -> `pyodbc>=5,<6`.
Tests
- `tests/connectors/test_mssql.py` covers int sizes (tinyint/smallint/
int/bigint), bit, varchar, decimal, datetime/datetime2,
datetimeoffset (utc + non-utc), uniqueidentifier and varbinary;
also exercises the URL connection path and pagination rewrite.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
dependencies
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughRefactors MSSQL from an Ibis-backed connector to a pyodbc-based implementation: updates optional dependency, adds URL/ODBC connection plumbing with datetimeoffset decoding, implements cursor execution with SQL pagination and Arrow schema/array construction, adds connection lifecycle handling, and adds unit and integration tests covering types, pagination, and error reporting. ChangesMSSQL Connector Refactor: Ibis to Native Pyodbc
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
core/wren/tests/connectors/test_mssql.py (2)
272-282: ⚡ Quick winPagination test misses the SQL Server
ORDER BYrequirement.
test_raw_cursor_sql_injects_fetch_nextasserts only thatFETCH NEXT 10 ROWS ONLYandOFFSET 0 ROWSappear in the rewritten string. SQL Server rejectsOFFSET … FETCH NEXT …unless anORDER BYclause is present in the sameSELECT, so the real invariant under test should be: "the rewritten SQL is executable on SQL Server".Strengthen the test to either (a) assert that an
ORDER BYis present in the rewrite when none was in the input, or (b) execute the rewritten SQL against themssql_containerfixture (e.g.,cursor.execute(rewritten)) to prove the server accepts it. Without one of these, this unit test cannot catch the regression where sqlglot or the rewrite dropsORDER BYsynthesis. See the related concern onMSSqlConnector._raw_cursor_sqlinmssql.py.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/wren/tests/connectors/test_mssql.py` around lines 272 - 282, Update the test test_raw_cursor_sql_injects_fetch_next to ensure the rewritten SQL is executable on SQL Server by verifying an ORDER BY is present or by executing the rewritten statement against the mssql_container; specifically call MSSqlConnector._raw_cursor_sql("SELECT * FROM orders", 10) (as currently done), then either assert that the returned string contains an ORDER BY clause (e.g., "order by") or use the mssql_container fixture to obtain a cursor and run cursor.execute(rewritten) to confirm the server accepts the OFFSET...FETCH NEXT syntax; this ensures MSSqlConnector._raw_cursor_sql (and any sqlglot rewrite) synthesizes ORDER BY when needed.
264-269: 💤 Low valueTighten the dry-run assertion so a generic
WrenErrordoesn't trivially satisfy it.
"dry run failed" in str(exc.value).lower()will match anyWrenErrorwhose message starts with the canned"The sql dry run failed. {error_message}."prefix — including the case where_describe_sql_for_error_messagereturned"Unknown reason"and the connector therefore did not wrap the original exception (Line 58-64 ofmssql.py: iferror_message == "Unknown reason"weraisethe originalExceptionandWrenErroris not raised at all).If the describe path ever silently returns
"Unknown reason"due to a regression (e.g., DMV permission change), this test still passes because pytest catches the un-wrapped pyodbc exception — except thatpytest.raises(WrenError)will actually fail in that case. So the assertion is OK on the happy path. Consider also asserting on the metadata to lock in the describe-path output, e.g.assert "not_a_column" in str(exc.value)orassert exc.value.metadata.get(DIALECT_SQL).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/wren/tests/connectors/test_mssql.py` around lines 264 - 269, The assertion in test_dry_run_invalid_column_returns_describe_error is too generic; tighten it so the test verifies the describe-path output rather than any WrenError prefix by asserting the error message contains the offending identifier (e.g., "not_a_column") and/or that exc.value.metadata contains the SQL under the DIALECT_SQL key; update the with pytest.raises(WrenError) block to assert "not_a_column" in str(exc.value).lower() and/or assert exc.value.metadata.get(DIALECT_SQL) is not None to ensure the connector.dry_run() raised the wrapped describe error.core/wren/src/wren/connector/mssql.py (5)
223-245: 💤 Low valueTINYINT (
internal_size == 1) should always beuint8in MSSQL.SQL Server
TINYINTis unsigned 0–255 by definition; it cannot store negative values. Falling back toint8when the sampled values happen to all be negative is impossible for a realTINYINTcolumn and produces a confusing type only when the cursor'sinternal_size==1comes from something else (which shouldn't happen for SQL Server). Consider dropping thenon_negativebranch here:- if internal_size == 1: - return pa.uint8() if non_negative else pa.int8() + if internal_size == 1: + return pa.uint8()Otherwise this is fine.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/wren/src/wren/connector/mssql.py` around lines 223 - 245, In _mssql_integer_arrow_type, remove the non_negative branch for the TINYINT case so that when internal_size == 1 the function always returns pa.uint8() (SQL Server TINYINT is unsigned); update the conditional handling in the function to unconditionally return pa.uint8() for internal_size == 1 and leave the rest of the size/precision logic unchanged (i.e., remove the pa.int8() fallback and any dependency on the values/non_negative check for the internal_size==1 path).
138-154: ⚡ Quick win
_describe_sql_for_error_messageuses the same connection that just failed and silently swallows the describe error.Two concerns:
- Reusing
self.connectionafter a query error is generally fine with pyodbc, but ifautocommit=Falseis ever in effect, the prior failure may have left an open implicit transaction that affects this describe call. Worth confirming the connection is inautocommit=Trueby default for this connector, or explicitly rolling back before describing.- The bare
except Exception: return "Unknown reason"hides real failures of the describe path (driver error, permission error on the DMV, etc.), which then surfaces to the user as the original exception with no diagnostic. Logging the describe-path exception vialoguru.logger.debug(or.warning) would preserve the existing UX while making misconfiguration debuggable.♻️ Suggested logging tweak
- except Exception: + except Exception as describe_error: + logger.debug( + f"sys.dm_exec_describe_first_result_set failed: {describe_error}" + ) return "Unknown reason"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/wren/src/wren/connector/mssql.py` around lines 138 - 154, The _describe_sql_for_error_message function currently reuses self.connection after a failed query and swallows any exception; change it to ensure the connection is safe for the describe call by either calling self.connection.rollback() (or verifying/setting self.connection.autocommit = True) before creating a cursor, and do not silence errors—catch exceptions from the describe path and log them with loguru.logger.debug or .warning (including the exception text and the SQL being described) before returning "Unknown reason"; keep references to sge.convert(sql).sql("mssql"), describe_sql, and closing(self.connection.cursor()) so you update only the transaction handling and add the logging instead of removing the existing describe logic.
247-285: ⚡ Quick winDecimal coercion path is unreachable.
_mssql_arrow_typealways returnspa.string()(notpa.decimal*) forPyDecimalcolumns (Line 202–203). As a result, thepa.types.is_decimal(arrow_type)branch at Line 261–270 of_build_mssql_columnis dead code — every decimal value gets routed through theis_stringbranch and is serialized viastr(value).Either (a) flip
_mssql_arrow_typeto returnpa.decimal128(...)/pa.decimal256(...)when precision/scale are known and keep this branch, or (b) drop the unreachable decimal branch to make the contract obvious. The PR's stated rationale ("Decimals serialise as strings to avoid arrow decimal precision pitfalls" in the test) suggests option (b).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/wren/src/wren/connector/mssql.py` around lines 247 - 285, The pa.types.is_decimal branch in _build_mssql_column is unreachable because _mssql_arrow_type returns pa.string() for PyDecimal columns; remove the decimal-specific branch and any PyDecimal-only coercion in _build_mssql_column so decimals are routed through the pa.types.is_string handling (serialised via str or json as the tests expect), and delete or stop referencing PyDecimal in that function (or remove the import if unused). This keeps the contract consistent with _mssql_arrow_type and the test expectation that decimals serialize as strings.
33-49: ⚖️ Poor tradeoffSchema inferred only from
rowsmay mis-type unsampled columns.
_build_mssql_arrow_schemainfers Arrow types fromcursor.descriptionplus the fetchedrows. Forquery(sql, limit=N)withNsmall (e.g., 1), or when all sampled values in a column areNone,_mssql_arrow_typefalls back through everyisinstance(sample, ...)branch and only matches ontype_code is X— which only works for the handful of Python types pyodbc actually returns (bool,int,float,bytes/bytearray,datetime.datetime,datetime.date,datetime.time). Columns whose pyodbc type code is none of those (e.g.,Decimal,uuid.UUID,str) and whose sampled values are allNonewill silently fall through to thepa.string()default at Line 207, which is acceptable for decimals/UUIDs but will coerce all-null numeric/temporal columns to string.Consider keying the type decision primarily off
type_code(withisinstanceas a tie-breaker), sincecursor.description[i][1]is authoritative and independent of which rows happened to be sampled.
174-207: 💤 Low valueType-dispatch order and
type_code is Xchecks need a closer look.A couple of subtle issues in
_mssql_arrow_type:
- Line 181:
type_code is boolworks only if pyodbc returns the actualboolclass forBITcolumns. In practice it does, but if any future driver/version returnsintforBIT, an all-nullBITcolumn would be classified asint(and the value-basedisinstance(sample, bool)short-circuit only catches non-null samples). Worth noting in a comment.- Lines 189/192:
dtlib.datetimeis checked beforedtlib.date, which is correct becausedatetimeis a subclass ofdate. Good — please keep this ordering invariant explicit with a brief comment so a future refactor doesn't reorder them.- Lines 196-201:
isinstance(sample, float) or type_code is floatis checked beforeisinstance(sample, int). Sincebool ⊂ int, the earlier bool guard at Line 181 is required to protect againstTrue/Falsefalling into the integer branch — also worth a brief comment.These are robustness/maintainability nits; the current behavior is correct for the tested matrix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/wren/src/wren/connector/mssql.py` around lines 174 - 207, Add brief explanatory comments inside MSSqlConnector._mssql_arrow_type around the type-dispatch checks to make the current fragile assumptions explicit: note that type_code checks (the local variable type_code) rely on driver-specific returned classes and may vary across drivers/versions, that the bool check (isinstance(sample, bool) or type_code is bool) must come before integer handling to prevent bool (a subclass of int) from being classified as integer, that datetime must remain before date (dtlib.datetime before dtlib.date) to preserve correct dispatch and reference MSSqlConnector._mssql_timezone_name usage, and that float is intentionally checked before int to preserve numeric precedence used by MSSqlConnector._mssql_integer_arrow_type; these comments should live next to the corresponding checks for sample/type_code to prevent accidental reordering.core/wren/src/wren/model/data_source.py (1)
444-471: 💤 Low valueAdd length validation to prevent silent garbage values on malformed DATETIMEOFFSET payloads.
The byte slicing here does not raise on short input—
int.from_bytes(b"", "little")returns0—so malformed payloads silently yielddatetime(0, 0, 0, …)which fails at the constructor with a crypticValueError: month must be in 1..12rather than a clear message about the payload size.Per pyodbc's DATETIMEOFFSET handling, the payload is always 20 bytes (SQL_SS_TIMESTAMPOFFSET_STRUCT), and the fraction field is documented as nanoseconds (0–999999999), so the
nanoseconds // 1000conversion is correct. Add a length check to catch future mismatches:Suggested fix
if value is None: return None + if len(value) != 20: + raise ValueError( + f"Unexpected MSSQL DATETIMEOFFSET payload length: {len(value)} (expected 20)" + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/wren/src/wren/model/data_source.py` around lines 444 - 471, The _decode_mssql_datetimeoffset function should validate the incoming bytes length before parsing to avoid silent garbage values; add an explicit check that value is either None or has length exactly 20 (SQL_SS_TIMESTAMPOFFSET_STRUCT) and raise a clear ValueError (e.g., "invalid DATETIMEOFFSET payload length: expected 20, got {len(value)}") when it is not, then proceed with the existing byte-slicing and conversion logic (including nanoseconds // 1000) so malformed payloads fail with a clear message rather than cryptic constructor errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/wren/src/wren/model/data_source.py`:
- Around line 386-431: In _connect_mssql_pyodbc: the current authentication
branch treats Trusted_Connection only when both user and password are None and
will call DataSourceExtension._escape_odbc_value(None) if one side is missing,
and it validates statement_timeout after opening the pyodbc connection which can
leak connections; fix by adding pre-flight validation: if both user and password
are None use Trusted_Connection=yes, if exactly one is None raise
WrenError(ErrorCode.INVALID_CONNECTION_INFO, "...") rejecting partial
credentials, and parse/cast/validate statement_timeout (convert to int or raise
WrenError) before calling pyodbc.connect so the connection is opened only after
inputs are validated; update construction to never call _escape_odbc_value on
None (use the validated user/password values).
- Around line 355-384: The URL components are not decoded consistently: only
password is unquoted. In get_mssql_connection_from_url decode parsed.username
and the database string (parsed.path.lstrip("/")) with
urllib.parse.unquote_plus, and apply unquote_plus to each value in kwargs after
building it from parse_qsl so query params are consistently decoded; then pass
those decoded user, database, password, driver and kwargs into
_connect_mssql_pyodbc as before.
In `@core/wren/tests/connectors/test_mssql.py`:
- Around line 285-305: The test constructs an mssql URL without encoding
credentials so special characters in the SqlServerContainer default
password/user can break parsing; update test_url_connection to URL-encode the
user and password when building the f"mssql://..." string (e.g., use
urllib.parse.quote or quote_plus) before passing the URL into
DataSource.mssql.get_connection_info/get_connection so the URL is unambiguous
and pairs with the decoding change in get_mssql_connection_from_url.
---
Nitpick comments:
In `@core/wren/src/wren/connector/mssql.py`:
- Around line 223-245: In _mssql_integer_arrow_type, remove the non_negative
branch for the TINYINT case so that when internal_size == 1 the function always
returns pa.uint8() (SQL Server TINYINT is unsigned); update the conditional
handling in the function to unconditionally return pa.uint8() for internal_size
== 1 and leave the rest of the size/precision logic unchanged (i.e., remove the
pa.int8() fallback and any dependency on the values/non_negative check for the
internal_size==1 path).
- Around line 138-154: The _describe_sql_for_error_message function currently
reuses self.connection after a failed query and swallows any exception; change
it to ensure the connection is safe for the describe call by either calling
self.connection.rollback() (or verifying/setting self.connection.autocommit =
True) before creating a cursor, and do not silence errors—catch exceptions from
the describe path and log them with loguru.logger.debug or .warning (including
the exception text and the SQL being described) before returning "Unknown
reason"; keep references to sge.convert(sql).sql("mssql"), describe_sql, and
closing(self.connection.cursor()) so you update only the transaction handling
and add the logging instead of removing the existing describe logic.
- Around line 247-285: The pa.types.is_decimal branch in _build_mssql_column is
unreachable because _mssql_arrow_type returns pa.string() for PyDecimal columns;
remove the decimal-specific branch and any PyDecimal-only coercion in
_build_mssql_column so decimals are routed through the pa.types.is_string
handling (serialised via str or json as the tests expect), and delete or stop
referencing PyDecimal in that function (or remove the import if unused). This
keeps the contract consistent with _mssql_arrow_type and the test expectation
that decimals serialize as strings.
- Around line 174-207: Add brief explanatory comments inside
MSSqlConnector._mssql_arrow_type around the type-dispatch checks to make the
current fragile assumptions explicit: note that type_code checks (the local
variable type_code) rely on driver-specific returned classes and may vary across
drivers/versions, that the bool check (isinstance(sample, bool) or type_code is
bool) must come before integer handling to prevent bool (a subclass of int) from
being classified as integer, that datetime must remain before date
(dtlib.datetime before dtlib.date) to preserve correct dispatch and reference
MSSqlConnector._mssql_timezone_name usage, and that float is intentionally
checked before int to preserve numeric precedence used by
MSSqlConnector._mssql_integer_arrow_type; these comments should live next to the
corresponding checks for sample/type_code to prevent accidental reordering.
In `@core/wren/src/wren/model/data_source.py`:
- Around line 444-471: The _decode_mssql_datetimeoffset function should validate
the incoming bytes length before parsing to avoid silent garbage values; add an
explicit check that value is either None or has length exactly 20
(SQL_SS_TIMESTAMPOFFSET_STRUCT) and raise a clear ValueError (e.g., "invalid
DATETIMEOFFSET payload length: expected 20, got {len(value)}") when it is not,
then proceed with the existing byte-slicing and conversion logic (including
nanoseconds // 1000) so malformed payloads fail with a clear message rather than
cryptic constructor errors.
In `@core/wren/tests/connectors/test_mssql.py`:
- Around line 272-282: Update the test test_raw_cursor_sql_injects_fetch_next to
ensure the rewritten SQL is executable on SQL Server by verifying an ORDER BY is
present or by executing the rewritten statement against the mssql_container;
specifically call MSSqlConnector._raw_cursor_sql("SELECT * FROM orders", 10) (as
currently done), then either assert that the returned string contains an ORDER
BY clause (e.g., "order by") or use the mssql_container fixture to obtain a
cursor and run cursor.execute(rewritten) to confirm the server accepts the
OFFSET...FETCH NEXT syntax; this ensures MSSqlConnector._raw_cursor_sql (and any
sqlglot rewrite) synthesizes ORDER BY when needed.
- Around line 264-269: The assertion in
test_dry_run_invalid_column_returns_describe_error is too generic; tighten it so
the test verifies the describe-path output rather than any WrenError prefix by
asserting the error message contains the offending identifier (e.g.,
"not_a_column") and/or that exc.value.metadata contains the SQL under the
DIALECT_SQL key; update the with pytest.raises(WrenError) block to assert
"not_a_column" in str(exc.value).lower() and/or assert
exc.value.metadata.get(DIALECT_SQL) is not None to ensure the
connector.dry_run() raised the wrapped describe error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c5b0cee5-91ff-4890-9a17-6ed87aa18eca
⛔ Files ignored due to path filters (1)
core/wren/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
core/wren/pyproject.tomlcore/wren/src/wren/connector/mssql.pycore/wren/src/wren/model/data_source.pycore/wren/tests/conftest.pycore/wren/tests/connectors/test_mssql.py
Previously only the password got unquote_plus() — username, the database path, and the query-string handling left other components encoded, so URL-encoded specials (e.g. '@' in a username, spaces in a database name) reached the ODBC driver still percent-encoded. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous branch silently mishandled asymmetric credentials: user=None with password set crashed in password.get_secret_value(), and user set with password=None emitted UID= without PWD=, producing an incomplete ODBC connection string. Now: both absent falls back to Trusted_Connection=yes, exactly one absent raises INVALID_CONNECTION_INFO, both present uses normal SQL auth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
statement_timeout was cast to int only after pyodbc.connect() returned, so a non-numeric value (e.g. "5s") leaked the just-opened connection when int() raised ValueError. Validate and cast up-front; raise INVALID_CONNECTION_INFO before any connect call. Also adds unit tests with mocked pyodbc covering all three review findings: URL component decoding, asymmetric auth rejection, and the pre-connect timeout validation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SQL Server TINYINT is an unsigned 8-bit integer (0..255), but the arrow-type helper was branching on the sampled value sign and could fall back to int8. Map internal_size == 1 directly to pa.uint8() so the schema reflects the driver-declared type regardless of the rows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The DATETIMEOFFSET output converter assumed pyodbc would always hand it a 20-byte payload. Truncated or malformed buffers fell through and surfaced as a cryptic "month must be in 1..12" ValueError from datetime(). Reject non-20-byte payloads up front with a clear message that points at the actual length mismatch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Follow-up on the CodeRabbit nitpicks bundled in review 4287043625: Addressed
Skipped (with rationale)
|
|
Superseded by #2313 — these seven native-driver refactors were consolidated into a single feature branch to resolve shared-file conflicts (data_source.py, pyproject.toml, uv.lock, factory.py, etc.) once instead of seven times. |
1 participant
Summary
The mssql connector now uses pyodbc directly with custom Arrow type inference and a datetimeoffset output converter. The mssql extra installs
pyodbcinstead ofibis-framework[mssql].Highlights
connector/mssql.py: raw pyodbc cursor; sqlglot-based LIMIT/OFFSET rewrite (OFFSET 0 ROWS FETCH NEXT n ROWS ONLY); Arrow schema built fromcursor.description+ value sampling;dry_runfalls back tosys.dm_exec_describe_first_result_setfor precise error messages.model/data_source.py:_connect_mssql_pyodbcbuilds an ODBC connection string with proper{...}escaping;mssql://URL parsing added; output converter for DATETIMEOFFSET (type code-155) decodes the 20-byte little-endian payload into a tz-awaredatetime.pyproject.toml: mssql extra ->pyodbc>=5,<6.Test plan
pip install -e ".[mssql]"no longer pullsibis-frameworkdry_runreturns a helpful error viasys.dm_exec_describe_first_result_setwhen a column is unknowntests/connectors/test_mssql.pycovers int sizes (tinyint/smallint/int/bigint), bit, varchar, decimal, datetime/datetime2, datetimeoffset, uniqueidentifier, varbinary; URL connection path; pagination rewritejust lintclean; existing unit tests unaffectedSummary by CodeRabbit
New Features
Bug Fixes
Tests
Chores