Skip to content

Commit 1da4fa9

Browse files
authored
FIX: Handle empty data + tests (#212)
### Work Item / Issue Reference <!-- IMPORTANT: Please follow the PR template guidelines below. For mssql-python maintainers: Insert your ADO Work Item ID below (e.g. AB#37452) For external contributors: Insert Github Issue number below (e.g. #149) Only one reference is required - either GitHub issue OR ADO Work Item. --> <!-- mssql-python maintainers: ADO Work Item --> > AB#<WORK_ITEM_ID> <!-- External contributors: GitHub Issue --> > GitHub Issue: #205 ------------------------------------------------------------------- ### Summary <!-- Insert your summary of changes below. Minimum 10 characters required. --> This pull request improves the handling of empty string and binary values in the MSSQL Python bindings, ensuring that empty data is correctly distinguished from NULL values and does not cause assertion failures. It also adds comprehensive tests to verify correct behavior for these edge cases. **Improvements to empty value handling:** * Updated `SQLGetData_wrap` in `ddbc_bindings.cpp` to append an empty string (`""`) or empty bytes (`b""`) when the returned data length is zero, instead of causing assertion failures or misinterpreting the value as NULL. This change applies to both string and binary column types. [[1]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1R1703-R1705) [[2]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1R1762-R1764) [[3]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1R1961-R1963) * Changed assertions in `FetchBatchData` to allow zero-length data, ensuring that empty values are handled gracefully instead of triggering errors. **Testing improvements:** * Added new tests in `tests/test_004_cursor.py` to verify correct handling of empty strings and binary data, including distinguishing empty values from NULLs, batch fetching, and various edge cases for empty strings. These tests ensure that the fixes work as intended and prevent regressions. **Minor code cleanup:** * Minor formatting and comment improvements in `ddbc_bindings.cpp` for better code clarity. <!-- ### 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 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 -->
1 parent 4b42f8c commit 1da4fa9

File tree

2 files changed

+454
-15
lines changed

2 files changed

+454
-15
lines changed

mssql_python/pybind/ddbc_bindings.cpp

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1778,11 +1778,19 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
17781778
}
17791779
} else if (dataLen == SQL_NULL_DATA) {
17801780
row.append(py::none());
1781-
} else {
1782-
assert(dataLen == SQL_NO_TOTAL);
1781+
} else if (dataLen == 0) {
1782+
// Handle zero-length (non-NULL) data
1783+
row.append(std::string(""));
1784+
} else if (dataLen == SQL_NO_TOTAL) {
1785+
// This means the length of the data couldn't be determined
17831786
LOG("SQLGetData couldn't determine the length of the data. "
1784-
"Returning NULL value instead. Column ID - {}", i);
1785-
row.append(py::none());
1787+
"Returning NULL value instead. Column ID - {}, Data Type - {}", i, dataType);
1788+
} else if (dataLen < 0) {
1789+
// This is unexpected
1790+
LOG("SQLGetData returned an unexpected negative data length. "
1791+
"Raising exception. Column ID - {}, Data Type - {}, Data Length - {}",
1792+
i, dataType, dataLen);
1793+
ThrowStdException("SQLGetData returned an unexpected negative data length");
17861794
}
17871795
} else {
17881796
LOG("Error retrieving data for column - {}, data type - {}, SQLGetData return "
@@ -1834,11 +1842,15 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
18341842
}
18351843
} else if (dataLen == SQL_NULL_DATA) {
18361844
row.append(py::none());
1837-
} else {
1838-
assert(dataLen == SQL_NO_TOTAL);
1839-
LOG("SQLGetData couldn't determine the length of the data. "
1840-
"Returning NULL value instead. Column ID - {}", i);
1841-
row.append(py::none());
1845+
} else if (dataLen == 0) {
1846+
// Handle zero-length (non-NULL) data
1847+
row.append(py::str(""));
1848+
} else if (dataLen < 0) {
1849+
// This is unexpected
1850+
LOG("SQLGetData returned an unexpected negative data length. "
1851+
"Raising exception. Column ID - {}, Data Type - {}, Data Length - {}",
1852+
i, dataType, dataLen);
1853+
ThrowStdException("SQLGetData returned an unexpected negative data length");
18421854
}
18431855
} else {
18441856
LOG("Error retrieving data for column - {}, data type - {}, SQLGetData return "
@@ -2030,11 +2042,15 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
20302042
}
20312043
} else if (dataLen == SQL_NULL_DATA) {
20322044
row.append(py::none());
2033-
} else {
2034-
assert(dataLen == SQL_NO_TOTAL);
2035-
LOG("SQLGetData couldn't determine the length of the data. "
2036-
"Returning NULL value instead. Column ID - {}", i);
2037-
row.append(py::none());
2045+
} else if (dataLen == 0) {
2046+
// Empty bytes
2047+
row.append(py::bytes(""));
2048+
} else if (dataLen < 0) {
2049+
// This is unexpected
2050+
LOG("SQLGetData returned an unexpected negative data length. "
2051+
"Raising exception. Column ID - {}, Data Type - {}, Data Length - {}",
2052+
i, dataType, dataLen);
2053+
ThrowStdException("SQLGetData returned an unexpected negative data length");
20382054
}
20392055
} else {
20402056
LOG("Error retrieving data for column - {}, data type - {}, SQLGetData return "
@@ -2317,8 +2333,30 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum
23172333
"Column ID - {}", col);
23182334
row.append(py::none());
23192335
continue;
2336+
} else if (dataLen == SQL_NULL_DATA) {
2337+
LOG("Column data is NULL. Appending None to the result row. Column ID - {}", col);
2338+
row.append(py::none());
2339+
continue;
2340+
} else if (dataLen == 0) {
2341+
// Handle zero-length (non-NULL) data
2342+
if (dataType == SQL_CHAR || dataType == SQL_VARCHAR || dataType == SQL_LONGVARCHAR) {
2343+
row.append(std::string(""));
2344+
} else if (dataType == SQL_WCHAR || dataType == SQL_WVARCHAR || dataType == SQL_WLONGVARCHAR) {
2345+
row.append(std::wstring(L""));
2346+
} else if (dataType == SQL_BINARY || dataType == SQL_VARBINARY || dataType == SQL_LONGVARBINARY) {
2347+
row.append(py::bytes(""));
2348+
} else {
2349+
// For other datatypes, 0 length is unexpected. Log & append None
2350+
LOG("Column data length is 0 for non-string/binary datatype. Appending None to the result row. Column ID - {}", col);
2351+
row.append(py::none());
2352+
}
2353+
continue;
2354+
} else if (dataLen < 0) {
2355+
// Negative value is unexpected, log column index, SQL type & raise exception
2356+
LOG("Unexpected negative data length. Column ID - {}, SQL Type - {}, Data Length - {}", col, dataType, dataLen);
2357+
ThrowStdException("Unexpected negative data length, check logs for details");
23202358
}
2321-
assert(dataLen > 0 && "Must be > 0 since SQL_NULL_DATA & SQL_NO_DATA is already handled");
2359+
assert(dataLen > 0 && "Data length must be > 0");
23222360

23232361
switch (dataType) {
23242362
case SQL_CHAR:

0 commit comments

Comments
 (0)