FIX: Initialize numeric array indicators in executemany - #702
Open
gargsaumya wants to merge 5 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a native executemany() array-binding correctness bug where fixed-width numeric parameters could leave earlier ODBC indicator slots uninitialized if a None (NULL) appeared later in the batch, leading to sporadic “0 rows inserted” outcomes with no exception.
Changes:
- Initialize/allocate
strLenOrIndArrayupfront for fixed-width numeric array bindings (e.g.,SQL_C_LONG,SQL_C_DOUBLE,SQL_C_TINYINT,SQL_C_SHORT) and explicitly set each slot to0orSQL_NULL_DATA. - Add a regression test that repeatedly inserts batches containing “late” NULLs across multiple numeric types (TINYINT/SMALLINT/INT/FLOAT) and asserts both rowcount and round-tripped values.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/test_004_cursor.py | Adds regression coverage for executemany() numeric batches where NULLs occur late in the parameter arrays. |
| mssql_python/pybind/ddbc_bindings.cpp | Ensures indicator arrays for fixed-width numeric parameter arrays are always allocated and fully initialized before SQLBindParameter/SQLExecuteMany. |
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql_python/pybind/ddbc_bindings.cppLines 2253-2261 2253 LOG("BindParameterArray: Binding SQL_C_DOUBLE array - "
2254 "param_index=%d, count=%zu",
2255 paramIndex, paramSetSize);
2256 double* dataArray = AllocateParamBufferArray<double>(tempBuffers, paramSetSize);
! 2257 strLenOrIndArray = AllocateParamBufferArray<SQLLEN>(tempBuffers, paramSetSize);
2258 for (size_t i = 0; i < paramSetSize; ++i) {
2259 if (columnValues[i].is_none()) {
2260 dataArray[i] = 0;
2261 strLenOrIndArray[i] = SQL_NULL_DATA;Lines 2260-2268 2260 dataArray[i] = 0;
2261 strLenOrIndArray[i] = SQL_NULL_DATA;
2262 } else {
2263 dataArray[i] = columnValues[i].cast<double>();
! 2264 strLenOrIndArray[i] = 0;
2265 }
2266 }
2267 LOG("BindParameterArray: SQL_C_DOUBLE bound - "
2268 "param_index=%d",Lines 2309-2317 2309 "array - param_index=%d, count=%zu",
2310 paramIndex, paramSetSize);
2311 unsigned char* dataArray =
2312 AllocateParamBufferArray<unsigned char>(tempBuffers, paramSetSize);
! 2313 strLenOrIndArray = AllocateParamBufferArray<SQLLEN>(tempBuffers, paramSetSize);
2314 for (size_t i = 0; i < paramSetSize; ++i) {
2315 if (columnValues[i].is_none()) {
2316 dataArray[i] = 0;
2317 strLenOrIndArray[i] = SQL_NULL_DATA;📋 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: 59.9%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 76.3%
mssql_python.__init__.py: 77.6%
mssql_python.row.py: 77.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 81.4%
mssql_python.pybind.connection.connection.cpp: 83.7%
mssql_python.logging.py: 85.5%🔗 Quick Links
|
subrata-ms
approved these changes
Aug 5, 2026
subrata-ms
left a comment
Contributor
There was a problem hiding this comment.
Looks good to me. Only concern is the code coverage showing pretty less. Probably you need to modify the existing test to cover the new AllocateParamBufferArray allocation call.
sumitmsft
approved these changes
Aug 5, 2026
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.
Work Item / Issue Reference
Summary
Root cause
The affected numeric binding paths allocated the ODBC indicator array only after encountering the first NULL. Earlier rows then contained uninitialized indicators, which could be interpreted as data-at-execution markers and cause a batch to insert zero rows without an exception.
Validation