FIX: Stored datetime.time values have the microseconds attribute set to zero#479
Open
FIX: Stored datetime.time values have the microseconds attribute set to zero#479
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes microsecond loss when round-tripping SQL Server TIME/TIME2 values by switching Python datetime.time binding to a text C-type with microsecond precision and updating the C++ fetch/batch-fetch paths to parse TIME2 from text back into datetime.time.
Changes:
- Bind Python
datetime.timeparameters as text (SQL_C_CHAR/SQL_C_WCHAR) usingisoformat(timespec="microseconds"). - Fetch
SQL_SS_TIME2as text in bothSQLGetData_wrapand batch fetch, converting todatetime.timevia a new parser. - Add a regression test asserting
TIME(6)preserves microseconds on insert/select.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/test_004_cursor.py | Adds a regression test for TIME microsecond round-trip; minor formatting updates to SQL strings. |
| mssql_python/pybind/ddbc_bindings.cpp | Adds a TIME text parser and changes SQL_SS_TIME2 retrieval/binding to use SQL_C_CHAR buffers. |
| mssql_python/cursor.py | Changes TIME parameter mapping and normalizes TIME values to microsecond ISO text in execute/executemany binding. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql_python/pybind/ddbc_bindings.cppLines 60-69 60 }
61
62 inline py::object ParseSqlTimeTextToPythonObject(const char* timeText, SQLLEN timeTextLen) {
63 if (!timeText || timeTextLen <= 0) {
! 64 return py::none();
! 65 }
66
67 size_t len;
68 if (timeTextLen == SQL_NO_TOTAL) {
69 // When the driver reports SQL_NO_TOTAL, the buffer may not be null-terminated.Lines 67-82 67 size_t len;
68 if (timeTextLen == SQL_NO_TOTAL) {
69 // When the driver reports SQL_NO_TOTAL, the buffer may not be null-terminated.
70 // Bound the scan to the maximum expected TIME/TIME2 text length.
! 71 const void* nul = std::memchr(timeText, '\0', SQL_TIME_TEXT_MAX_LEN - 1);
! 72 len = nul ? static_cast<size_t>(static_cast<const char*>(nul) - timeText)
! 73 : static_cast<size_t>(SQL_TIME_TEXT_MAX_LEN - 1);
74 } else {
75 len = static_cast<size_t>(timeTextLen);
76 if (len > SQL_TIME_TEXT_MAX_LEN - 1) {
! 77 len = SQL_TIME_TEXT_MAX_LEN - 1;
! 78 }
79 }
80
81 std::string value(timeText, len);Lines 81-90 81 std::string value(timeText, len);
82
83 size_t start = value.find_first_not_of(" \t\r\n");
84 if (start == std::string::npos) {
! 85 return py::none();
! 86 }
87 size_t end = value.find_last_not_of(" \t\r\n");
88 value = value.substr(start, end - start + 1);
89
90 size_t firstColon = value.find(':');Lines 90-99 90 size_t firstColon = value.find(':');
91 size_t secondColon =
92 (firstColon == std::string::npos) ? std::string::npos : value.find(':', firstColon + 1);
93 if (firstColon == std::string::npos || secondColon == std::string::npos) {
! 94 ThrowStdException("Failed to parse TIME/TIME2 value: missing ':' separators");
! 95 }
96
97 int hour = std::stoi(value.substr(0, firstColon));
98 int minute = std::stoi(value.substr(firstColon + 1, secondColon - firstColon - 1));Lines 3326-3347 3326 &timeDataLen);
3327 if (SQL_SUCCEEDED(ret)) {
3328 if (timeDataLen == SQL_NULL_DATA) {
3329 // Normal NULL value: append None without logging an error.
! 3330 row.append(py::none());
3331 } else {
3332 row.append(ParseSqlTimeTextToPythonObject(timeTextBuffer, timeDataLen));
3333 }
3334 } else {
! 3335 LOG("SQLGetData: Error retrieving SQL_SS_TIME2 for column "
! 3336 "%d - SQLRETURN=%d",
! 3337 i, ret);
! 3338 row.append(py::none());
! 3339 }
3340 break;
! 3341 }
! 3342 case SQL_TIME:
! 3343 case SQL_TYPE_TIME: {
3344 SQL_TIME_STRUCT timeValue;
3345 ret =
3346 SQLGetData_ptr(hStmt, i, SQL_C_TYPE_TIME, &timeValue, sizeof(timeValue), NULL);
3347 if (SQL_SUCCEEDED(ret)) {Lines 3993-4001 3993 PyList_SET_ITEM(row, col - 1, dateObj);
3994 break;
3995 }
3996 case SQL_TIME:
! 3997 case SQL_TYPE_TIME: {
3998 PyObject* timeObj =
3999 PythonObjectCache::get_time_class()(buffers.timeBuffers[col - 1][i].hour,
4000 buffers.timeBuffers[col - 1][i].minute,
4001 buffers.timeBuffers[col - 1][i].second)Lines 4010-4018 4010 SQLLEN timeDataLen = buffers.indicators[col - 1][i];
4011 py::object timeObj = ParseSqlTimeTextToPythonObject(rawData, timeDataLen);
4012 PyList_SET_ITEM(row, col - 1, timeObj.release().ptr());
4013 break;
! 4014 }
4015 case SQL_SS_TIMESTAMPOFFSET: {
4016 SQLULEN rowIdx = i;
4017 const DateTimeOffset& dtoValue = buffers.datetimeoffsetBuffers[col - 1][rowIdx];
4018 SQLLEN indicator = buffers.indicators[col - 1][rowIdx];📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.hpp: 58.8%
mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 67.8%
mssql_python.pybind.ddbc_bindings.cpp: 69.2%
mssql_python.row.py: 70.5%
mssql_python.pybind.connection.connection.cpp: 75.3%
mssql_python.__init__.py: 77.1%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.connection.py: 85.2%🔗 Quick Links
|
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
This pull request introduces improvements to the handling of SQL
TIMEandTIME2types in the Python MSSQL driver, ensuring consistent conversion between database and Python representations, especially for microsecond precision. The changes span both Python and C++ code, enhancing parameter binding, data retrieval, and batch fetching for these types.Enhancements to SQL TIME/TIME2 handling:
_map_sql_typeincursor.pyto useSQL_TYPE_TIMEand bind Pythondatetime.timevalues as text with microsecond precision, increasing column size and changing C-type toSQL_C_CHAR.datetime.timevalues to ISO text format with microseconds when binding as text types, ensuring compatibility with SQL_C_CHAR/SQL_C_WCHAR.executemany, ensured that Pythondatetime.timevalues are converted to ISO text with microseconds when bound as text types.C++ backend improvements for TIME/TIME2:
ParseSqlTimeTextToPythonObjectfunction to parse SQL TIME/TIME2 text into Pythondatetime.timeobjects, supporting microseconds.SQLGetData_wrapand batch fetching logic to retrieve SQL_SS_TIME2 columns as text and convert them to Pythondatetime.timeobjects using the new parser.Miscellaneous:
SQL_TIME_TEXT_MAX_LENfor improved text handling in C++.