Uh oh!
There was an error while loading. Please reload this page.
FIX: Inconsistent retrieval of CP1252 encoded data in VARCHAR columns - Windows vs. Linux #468 - #495
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql_python/pybind/ddbc_bindings.cppLines 2062-2070 2062size_t chunkBytes = DAE_CHUNK_SIZE;
2063while (offset < totalBytes) {
2064size_t len = std::min(chunkBytes, totalBytes - offset);
2065 ! 2066 rc = putData((SQLPOINTER)(dataPtr + offset), static_cast<SQLLEN>(len));
2067if (!SQL_SUCCEEDED(rc)) {
2068LOG("SQLExecute: SQLPutData failed for "2069"SQL_C_CHAR chunk - offset=%zu",
2070 offset, totalBytes, len, rc);Lines 3412-3425 3412"length=%lu",
3413 i, (unsignedlong)numCharsInData);
3414 } else {
3415// Buffer too small, fallback to streaming
! 3416LOG("SQLGetData: CHAR column %d (WCHAR path) data "
! 3417"truncated, using streaming LOB",
! 3418 i);
! 3419 row.append(FetchLobColumnData(hStmt, i, SQL_C_WCHAR, true, false,
! 3420"utf-16le"));
! 3421 }
3422 } elseif (dataLen == SQL_NULL_DATA) {
3423LOG("SQLGetData: Column %d is NULL (CHAR via WCHAR)", i);
3424 row.append(py::none());
3425 } elseif (dataLen == 0) {Lines 3428-3447 3428// Driver cannot report total length up front; this is3429// NOT a NULL value. Fall back to streaming via3430// FetchLobColumnData (repeated SQLGetData chunks) so3431// we don't silently lose data.
! 3432LOG("SQLGetData: SQL_NO_TOTAL for column %d (CHAR via WCHAR), "
! 3433"streaming via FetchLobColumnData",
! 3434 i);
! 3435 row.append(
! 3436FetchLobColumnData(hStmt, i, SQL_C_WCHAR, true, false, "utf-16le"));
! 3437 } elseif (dataLen < 0) {
! 3438LOG("SQLGetData: Unexpected negative data length "
! 3439"for column %d - dataType=%d, dataLen=%ld",
! 3440 i, dataType, (long)dataLen);
! 3441ThrowStdException("SQLGetData returned an unexpected negative "
! 3442"data length");
! 3443 }
3444 } else {
3445// Surface driver errors instead of silently returning NULL.3446// Returning py::none() here would be indistinguishable from3447// a genuine SQL NULL value to the Python caller and is aLines 3445-3458 3445// Surface driver errors instead of silently returning NULL.3446// Returning py::none() here would be indistinguishable from3447// a genuine SQL NULL value to the Python caller and is a3448// data-integrity risk.
! 3449LOG_ERROR("SQLGetData: Error retrieving data for column %d "
! 3450"(CHAR via WCHAR) - SQLRETURN=%d",
! 3451 i, ret);
! 3452ThrowStdException("SQLGetData failed for CHAR/VARCHAR column "
! 3453"fetched as SQL_C_WCHAR");
! 3454 }
3455 } else {
3456// Allocate columnSize * 4 + 1 on ALL platforms (no #if guard).3457//3458// Why this differs from SQLBindColums / FetchBatchData:Lines 3517-3529 3517// Driver cannot report total length up front; this is3518// NOT a NULL value. Fall back to streaming via3519// FetchLobColumnData (repeated SQLGetData chunks) so3520// we don't silently lose data.
! 3521LOG("SQLGetData: SQL_NO_TOTAL for column %d (SQL_CHAR), "
! 3522"streaming via FetchLobColumnData",
3523 i);
! 3524 row.append(FetchLobColumnData(hStmt, i, SQL_C_CHAR, false, false,
! 3525 effectiveCharEnc));
3526 } elseif (dataLen < 0) {
3527LOG("SQLGetData: Unexpected negative data length "3528"for column %d - dataType=%d, dataLen=%ld",
3529 i, dataType, (long)dataLen);Lines 3534-3545 3534// Surface driver errors instead of silently returning NULL.3535// Returning py::none() here would be indistinguishable from3536// a genuine SQL NULL value to the Python caller and is a3537// data-integrity risk.
! 3538LOG_ERROR("SQLGetData: Error retrieving data for column %d "
! 3539"(SQL_CHAR) - SQLRETURN=%d",
! 3540 i, ret);
! 3541ThrowStdException("SQLGetData failed for SQL_CHAR/VARCHAR column");
3542 }
3543 }
3544break;
3545 }Lines 3602-3614 3602// Driver cannot report total length up front; this is3603// NOT a NULL value. Fall back to streaming via3604// FetchLobColumnData (repeated SQLGetData chunks) so3605// we don't silently lose data.
! 3606LOG("SQLGetData: SQL_NO_TOTAL for column %d (NVARCHAR), "
! 3607"streaming via FetchLobColumnData",
3608 i);
! 3609 row.append(
! 3610FetchLobColumnData(hStmt, i, SQL_C_WCHAR, true, false, "utf-16le"));
3611 } elseif (dataLen < 0) {
3612LOG("SQLGetData: Unexpected negative data length "3613"for column %d (NVARCHAR) - dataLen=%ld",
3614 i, (long)dataLen);Lines 3619-3630 3619// Surface driver errors instead of silently returning NULL.3620// Returning py::none() here would be indistinguishable from3621// a genuine SQL NULL value to the Python caller and is a3622// data-integrity risk.
! 3623LOG_ERROR("SQLGetData: Error retrieving data for column %d "
! 3624"(NVARCHAR) - SQLRETURN=%d",
! 3625 i, ret);
! 3626ThrowStdException("SQLGetData failed for NVARCHAR column");
3627 }
3628 }
3629break;
3630 }Lines 4996-5006 4996 arrowColumnProducer->ptrValueBuffer = arrowColumnProducer->bitVal.get();
4997break;
4998default:
4999 std::ostringstream errorString;
! 5000 errorString << "Unsupported data type for Arrow batch fetch for column - "
! 5001 << columnName.c_str() << ", Type - " << dataType << ", column ID - "
! 5002 << (i + 1);
5003LOG(errorString.str().c_str());
5004ThrowStdException(errorString.str());
5005break;
5006 }Lines 5256-5265 5256 buffers.datetimeoffsetBuffers[idxCol].data(),
5257sizeof(DateTimeOffset),
5258 buffers.indicators[idxCol].data());
5259if (!SQL_SUCCEEDED(ret)) {
! 5260LOG("Error fetching SS_TIMESTAMPOFFSET data for column %d",
! 5261 idxCol + 1);
5262return ret;
5263 }
5264break;
5265 }Lines 5307-5317 5307 nullCounts[idxCol] += 1;
5308continue;
5309 } elseif (indicator < 0) {
5310// Negative value is unexpected, log column index, SQL type & raise exception
! 5311LOG("Unexpected negative data length. Column ID - %d, SQL Type - %d, Data "
! 5312"Length - %lld",
! 5313 idxCol + 1, dataType, (longlong)indicator);
5314ThrowStdException("Unexpected negative data length.");
5315 }
5316auto dataLen = static_cast<uint64_t>(indicator);Lines 5610-5619 5610 arrowSchemaBatchCapsule =
5611py::capsule(arrowSchemaBatch.get(), "arrow_schema", [](void* ptr) {
5612auto arrowSchema = static_cast<ArrowSchema*>(ptr);
5613if (arrowSchema->release) {
! 5614 arrowSchema->release(arrowSchema);
! 5615 }
5616delete arrowSchema;
5617 });
5618 } catch (...) {
5619 arrowSchemaBatch->release(arrowSchemaBatch.get());mssql_python/pybind/ddbc_bindings.h📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 66.5%
mssql_python.row.py: 70.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 74.3%
mssql_python.pybind.connection.connection.cpp: 76.2%
mssql_python.__init__.py: 77.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.connection.py: 85.2%🔗 Quick Links
|
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Sumit Sarabhai (sumitmsft)
left a comment
There was a problem hiding this comment.
Left some minor comments. Otherwise, I am ok with this change
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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()
assertrow[0] isNoneandrow[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# ~50kcursor.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")
assertcursor.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")
assertcursor.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')")
assertcursor.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.
Subrata (subrata-ms)
commented
May 12, 2026
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 |
Subrata (subrata-ms)
commented
May 13, 2026
Yannick Einsweiler (@yan-hic) , Thanks for the comment. However, arrow related fix will be tracked under below bug - #553 |
Uh oh!
There was an error while loading. Please reload this page.
### 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]