Reach mssql-python parity in the ODBC driver - #175
Draft
Saurabh Singh (saurabh500) wants to merge 12 commits into
Draft
Reach mssql-python parity in the ODBC driver#175Saurabh Singh (saurabh500) wants to merge 12 commits into
Saurabh Singh (saurabh500) wants to merge 12 commits into
Conversation
Implements the driver entry points, conversions and lifecycle semantics the mssql-python pybind layer depends on when it loads the driver directly with no Driver Manager: the 15 missing exports, connection attributes and transactions, a shared SQL-to-C conversion layer, column-wise block fetch, catalog functions, descriptor fields, bounded read-ahead so a second statement can run while a cursor is open, and cascading handle frees. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e698c659-8b42-43f7-99ab-46f534b2489b
Implements SQLParamData/SQLPutData for data-at-execution parameters, column-wise parameter arrays for executemany, chunked SQLGetData with offset tracking, and SQL Server-specific type reporting for sql_variant and UDT columns. Character columns with a collation-derived encoding are now passed through in their original code page for SQL_C_CHAR targets. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e698c659-8b42-43f7-99ab-46f534b2489b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e698c659-8b42-43f7-99ab-46f534b2489b
…cute errors Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e698c659-8b42-43f7-99ab-46f534b2489b
…o values Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e698c659-8b42-43f7-99ab-46f534b2489b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e698c659-8b42-43f7-99ab-46f534b2489b
Statement-wise navigation stopped on the COUNT-flagged DONEINPROC that a SET assignment emits, leaving a following PRINT unread, so server messages never reached the client. The unicode string cap used a shift that both truncated at 255 characters and read as a bug; it is a byte count of a u16 length. A NULL decimal parameter carries no precision of its own. When a prepared plan is built from a parameter-array row whose value is NULL, the default declaration was reused for every later row and rejected wider values, so callers can now declare the precision and scale explicitly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e698c659-8b42-43f7-99ab-46f534b2489b
The TDS time fields count 100-nanosecond ticks, not nanoseconds, so every time, datetime2, and datetimeoffset value was scaled 100x. Money and smallmoney went through f64 and lost precision; they now keep their scaled integer representation. SQL_C_TINYINT is unsigned, ODBC 2.x applications bind dates and times with the older type codes, a UDT column reports its own length rather than zero, and NULL decimal parameters carry the application's precision and scale. SQLDescribeParam was a stub returning a fixed guess; it now asks the server through sp_describe_undeclared_parameters and caches the answer. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e698c659-8b42-43f7-99ab-46f534b2489b
Bound columns were strided by the application's buffer length, which ODBC ignores for fixed-width C types. An application passing the whole array size there corrupted memory beyond the first row. SQLGetData reported a NULL as success with an untouched buffer when no indicator was supplied, so callers read stale memory instead of seeing the required 22002. It now fails as the specification requires. An error arriving after the first row of a rowset was discarded once the partial rowset was returned, and the next call reported an invalid cursor instead of the real failure. The diagnostic is now replayed. The connection stayed claimed until the last result set was read, and a parameter array left each row's cursor open; both are now released, except for the final row, whose OUTPUT rows the caller still expects. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e698c659-8b42-43f7-99ab-46f534b2489b
Applications write procedure calls as {CALL p(?)}, which SQL Server cannot
parse; escape sequences are now translated before the batch is sent, and a
call's parenthesised argument list becomes an EXEC argument list.
Catalog functions returned the ODBC 2.x column names the system procedures
emit, dropped every row when no index name was supplied, quoted a table
type list as a single element, and failed outright on a catalog that does
not exist rather than returning no rows.
Also maps APP in the connection string to the application name and derives
the SQLSTATE class from the server's severity when the error number is
unknown.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e698c659-8b42-43f7-99ab-46f534b2489b
Records every defect the mssql-python integration suite found in this driver, what caused it, and how many tests it accounted for. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e698c659-8b42-43f7-99ab-46f534b2489b
Saurabh Singh (saurabh500)
changed the title
The suite includes windows.h and binds the driver with LoadLibraryW, but CMakeLists.txt added it unconditionally, so the Linux and Linux ARM builds failed to compile it. It now lives on its own, with every case disabled, so it can be enabled one case at a time as the API work lands. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e698c659-8b42-43f7-99ab-46f534b2489b
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mssql-odbcbuildsmsodbcsql18.dllas a drop-in replacement for the Microsoft ODBC Driver 18 for SQL Server. mssql-python ships that driver in its wheel and loads it directly withLoadLibraryW— no Driver Manager — so the driver alone owns the whole ODBC contract. That makes mssql-python's integration suite a good conformance test for this crate.This PR runs that suite against the Rust driver and fixes everything it found.
Full gap analysis:
mssql-odbc/docs/mssql-python-gap-analysis.mdResult
msodbcsql18.dll(C++, shipped)msodbcsql18.dll(this crate), beforemsodbcsql18.dll(this crate), afterThe two remaining failures are identical under both drivers: one exceeds the Windows
MAX_PATHlimit and makes no ODBC call, and the other goes through mssql-python's ownmssql_py_corePyO3 extension, which opens its own TDS connection and never reaches the driver.test_004_cursor.py, the largest file, went from an access violation part-way through the run to 518 passed, 0 failed.What was wrong
29 distinct defects, lettered A–AD in the report. The ones that mattered most:
BufferLengthfor fixed-width C types, and mssql-python exploits that by passing the whole array's size there. The driver used it as the per-row stride and wrote past the end of the buffer — non-deterministic heap corruption that killed the run at a different test each time.SQL_ERRORwith22002whenSQLGetDatahits a NULL and the caller passed no indicator. Returning success with an untouched buffer meantfetchone()handed back stale stack memory for every NULL fixed-width column.fetchall()was fine because it binds real indicator arrays — that split is what found it.{CALL p(?)}reached the server as-is.SQLDescribeParamwas a stub returning a fixed guess for every parameter.Also fixed: data-at-execution parameters, parameter arrays,
sql_variant, UDT and spatial types, ANSI code page encoding, LOB lengths, SQLSTATE mapping, connection lifetime,PRINToutput, money precision, and NULL decimal declarations.Tests
mssql-odbcunit tests pass, including new coverage for the escape translator, catalog argument rendering, and NULL decimal declaration.mssql-odbc/tests/e2e/tests/mssql_python_parity_test.cpp— 20 cases covering transactions, block fetch, concurrent cursors,SQLGetDataconversions, catalog functions, and parameter binding.cargo bfmtandcargo bclippyare clean.mssql-tdscertificate/TLS test failures on this machine are pre-existing and reproduce without these changes.Related Issues
The parity test suite that came out of this work is in #177. The gap analysis it produced is in
mssql-odbc/docs/mssql-python-gap-analysis.md.