Repository navigation
FIX: Inconsistent retrieval of CP1252 encoded data in VARCHAR columns - Windows vs. Linux #468 - #495
Conversation
There was a problem hiding this comment.
Pull request overview
This PR changes the default decoding path for SQL CHAR/VARCHAR (ODBC SQL_CHAR) columns to fetch as wide characters (SQL_C_WCHAR) and decode as UTF-16LE, eliminating Windows-vs-Linux inconsistencies when server code pages contain bytes that are invalid UTF-8.
Changes:
- Update connection defaults so
SQL_CHARdecoding usesencoding="utf-16le"withctype=SQL_WCHAR. - Plumb
charCtypethrough cursor fetch APIs into the C++ bindings, enabling wide-char fetch forSQL_CHARcolumns. - Expand/adjust encoding tests to validate new defaults and reproduce the CP1252 byte behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
mssql_python/connection.py |
Changes default decoding settings for SQL_CHAR to UTF-16LE/SQL_WCHAR. |
mssql_python/cursor.py |
Passes updated decoding settings (encoding + ctype) into the native fetch functions. |
mssql_python/pybind/ddbc_bindings.cpp |
Adds charCtype plumb-through and implements SQL_C_WCHAR paths for SQLGetData and bound-column fetching. |
mssql_python/pybind/ddbc_bindings.h |
Extends per-column metadata and adds a wide-char branch in ProcessChar. |
tests/test_013_encoding_decoding.py |
Updates default expectations and adds Windows-focused regression tests for the CP1252 byte case. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sumit Sarabhai (sumitmsft)
left a comment
There was a problem hiding this comment.
Left some minor comments. Otherwise, I am ok with this change
There was a problem hiding this comment.
approving with test addition suggestions below.
logic lgtm. fix for #468 reproduces clean against sql2022, new TestIssue531Utf8CollationVarchar covers the SQL_C_WCHAR-default path on linux/macOS, all 185 encoding tests pass.
one ask before merge: the test coverage can be improved.
I went through each missing line. four python tests would close most of the gap:
- fetchone with NULL + empty VARCHAR. covers null/empty arms in the new CHAR-via-WCHAR block.
cursor.execute("SELECT CAST(NULL AS VARCHAR(10)), CAST('' AS VARCHAR(10))")
row = cursor.fetchone()
assert row[0] is None and row[1] == ''- fetchone with VARCHAR(MAX) and NVARCHAR(MAX) carrying 50k+ non-ASCII. exercises wide-char LOB streaming in ProcessChar and SQLGetData_wrap. large LOBs also surface SQL_NO_TOTAL on the first chunk, picks up that branch incidentally.
payload = "caf \u00ad " * 6250 # ~50k
cursor.execute("CREATE TABLE #t (v VARCHAR(MAX) COLLATE Latin1_General_100_CI_AS_SC_UTF8, n NVARCHAR(MAX))")
cursor.execute("INSERT INTO #t VALUES (?, ?)", payload, payload)
cursor.execute("SELECT v, n FROM #t")
assert cursor.fetchone() == (payload, payload)- fetchone on a TEXT column. legacy LOB, deterministically returns SQL_NO_TOTAL on the first chunk. cheapest way to actually exercise that branch.
cursor.execute("CREATE TABLE #t (v TEXT)")
cursor.execute("INSERT INTO #t VALUES (?)", "A" * 100_000)
cursor.execute("SELECT v FROM #t")
assert cursor.fetchone()[0] == "A" * 100_000- fetchall on VARCHAR(N) with UTF-8 collation at size boundary. catches off-by-one in the wide-char path with non-BMP chars.
cursor.execute("CREATE TABLE #t (v VARCHAR(4) COLLATE Latin1_General_100_CI_AS_SC_UTF8)")
cursor.execute("INSERT INTO #t VALUES (N'\U0001F600')")
assert cursor.fetchall()[0][0] == '\U0001F600'the decode-failure-to-bytes branch and the throw paths can't easily be hit from python. fine as a follow-up, or at minimum mark them as defensive in a code comment so a future refactor doesn't quietly strip them.
Hi Gaurav Sharma (@bewithgaurav) , this suggested test does not improve code coverage ( ~1%). The lines that are flagged as not covered are hard to reach with current test framework. We need to add dummy/test code within the production code to create this scenario (one option). I tried that and it was looking pretty big change on the current coding framework. |
|
I hit the same error using Can you add tests for these cases too, as the |
### Work Item / Issue Reference > [AB#45159](https://sqlclientdrivers.visualstudio.com/mssql-python/_sprints/taskboard/mssql-python%20Team/mssql-python/Rubidium/May%202026?workitem=45159) ------------------------------------------------------------------- ### Summary **Enhancements** - #548 — manylinux_2_28 build targets for RHEL 8 / glibc 2.28 - #542 — macOS universal2 wheel for Python 3.10 - #526 — UTF-16 string handling via simdutf - #528 — Optimized execute() hot path - #567 — Azure Linux installation docs **Bug Fixes** - #562 — Login failures now raise mssql_python exception instead of RuntimeError - #568 — GIL released during blocking SQLSetConnectAttr calls - #541 — GIL released during blocking ODBC statement/fetch/transaction calls - #560 — executemany RuntimeError when decimals change signs - #495 — Inconsistent CP1252 VARCHAR retrieval Windows vs Linux - #559 — BulkCopy empty string in NVARCHAR(MAX)/VARCHAR(MAX) (via mssql_py_core 0.1.4)
### Work Item / Issue Reference <!-- IMPORTANT: Please follow the PR template guidelines below. For mssql-python maintainers: Insert your ADO Work Item ID below For external contributors: Insert Github Issue number below Only one reference is required - either GitHub issue OR ADO Work Item. --> <!-- mssql-python maintainers: ADO Work Item --> > [AB#44922](https://sqlclientdrivers.visualstudio.com/c6d89619-62de-46a0-8b46-70b92a84d85e/_workitems/edit/44922) <!-- External contributors: GitHub Issue --> > GitHub Issue: #553 ------------------------------------------------------------------- ### Summary <!-- Insert your summary of changes below. Minimum 10 characters required. --> Due to #495, we can now request SQL_CHAR data as SQL_C_WCHAR, i.e. utf16le strings. Doing this for the arrow path ensures that arrow methods always return correct data no matter the encoding settings / locale / operating system. There does not seem to be any significant negative performance impact. <!-- ### PR Title Guide > For feature requests FEAT: (short-description) > For non-feature requests like test case updates, config updates , dependency updates etc CHORE: (short-description) > For Fix requests FIX: (short-description) > For doc update requests DOC: (short-description) > For Formatting, indentation, or styling update STYLE: (short-description) > For Refactor, without any feature changes REFACTOR: (short-description) > For performance improvements PERF: (short-description) > For release related changes, without any feature changes RELEASE: #<RELEASE_VERSION> (short-description) ### Contribution Guidelines External contributors: - Create a GitHub issue first: https://github.com/microsoft/mssql-python/issues/new - Link the GitHub issue in the "GitHub Issue" section above - Follow the PR title format and provide a meaningful summary mssql-python maintainers: - Create an ADO Work Item following internal processes - Link the ADO Work Item in the "ADO Work Item" section above - Follow the PR title format and provide a meaningful summary --> --------- Co-authored-by: subrata-ms <141804867+subrata-ms@users.noreply.github.com> Co-authored-by: gargsaumya <saumyagarg.100@gmail.com>

Work Item / Issue Reference
Summary
This pull request updates the default handling of SQL
CHAR/VARCHARcolumns to use UTF-16 (wide character) encoding instead of UTF-8, primarily to address encoding mismatches on Windows and ensure consistent Unicode decoding. The changes span the connection, cursor, and C++ binding layers, and update related tests to reflect the new default behavior.Default Encoding and Decoding Changes:
The default decoding for SQL
CHARcolumns is now set to use"utf-16le"encoding and theSQL_WCHARctype, replacing the previous"utf-8"/SQL_CHARdefaults. This avoids issues where Windows ODBC drivers return raw bytes in the server's native code page, which may not decode as UTF-8. (mssql_python/connection.py, mssql_python/connection.pyR264-R271)All cursor fetch methods (
fetchone,fetchmany,fetchall) are updated to request UTF-16 decoding and pass the correct ctype when fetchingCHARdata, ensuring consistent behavior across platforms. (mssql_python/cursor.py, [1] [2] [3]C++ Binding and Processing Updates:
ColumnInfoExtstruct now tracks whether wide character (UTF-16) fetching is used for a column, and theProcessCharfunction is updated to handle both wide and narrow character paths, decoding appropriately based on the new setting. (mssql_python/pybind/ddbc_bindings.h, [1] [2] [3]Test Adjustments:
"utf-16le"andSQL_WCHARas the default decoding settings forSQL_CHARcolumns, and to validate the new default behavior. (tests/test_013_encoding_decoding.py, [1] [2] [3]