Skip to content

FIX: Stored datetime.time values have the microseconds attribute set to zero#479

Open
jahnvi480 wants to merge 4 commits intomainfrom
jahnvi/ghissue_203
Open

FIX: Stored datetime.time values have the microseconds attribute set to zero#479
jahnvi480 wants to merge 4 commits intomainfrom
jahnvi/ghissue_203

Conversation

@jahnvi480
Copy link
Contributor

@jahnvi480 jahnvi480 commented Mar 19, 2026

Work Item / Issue Reference

AB#38820

GitHub Issue: #203


Summary

This pull request introduces improvements to the handling of SQL TIME and TIME2 types 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:

  • Updated _map_sql_type in cursor.py to use SQL_TYPE_TIME and bind Python datetime.time values as text with microsecond precision, increasing column size and changing C-type to SQL_C_CHAR.
  • Normalized Python datetime.time values to ISO text format with microseconds when binding as text types, ensuring compatibility with SQL_C_CHAR/SQL_C_WCHAR.
  • In executemany, ensured that Python datetime.time values are converted to ISO text with microseconds when bound as text types.

C++ backend improvements for TIME/TIME2:

  • Added the ParseSqlTimeTextToPythonObject function to parse SQL TIME/TIME2 text into Python datetime.time objects, supporting microseconds.
  • Refactored SQLGetData_wrap and batch fetching logic to retrieve SQL_SS_TIME2 columns as text and convert them to Python datetime.time objects using the new parser.
  • Updated column binding and row size calculations to handle SQL_SS_TIME2 as text with a defined maximum length, ensuring correct buffer allocation.

Miscellaneous:

  • Added necessary includes and defined SQL_TIME_TEXT_MAX_LEN for improved text handling in C++.

Copilot AI review requested due to automatic review settings March 19, 2026 14:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.time parameters as text (SQL_C_CHAR/SQL_C_WCHAR) using isoformat(timespec="microseconds").
  • Fetch SQL_SS_TIME2 as text in both SQLGetData_wrap and batch fetch, converting to datetime.time via 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.

@github-actions github-actions bot added the pr-size: large Substantial code update label Mar 19, 2026
@github-actions
Copy link

github-actions bot commented Mar 19, 2026

📊 Code Coverage Report

🔥 Diff Coverage

79%


🎯 Overall Coverage

76%


📈 Total Lines Covered: 5716 out of 7429
📁 Project: mssql-python


Diff Coverage

Diff: main...HEAD, staged and unstaged changes

  • mssql_python/cursor.py (100%)
  • mssql_python/pybind/ddbc_bindings.cpp (77.6%): Missing lines 64-65,71-73,77-78,85-86,94-95,3330,3335-3339,3341-3343,3997,4014

Summary

  • Total: 105 lines
  • Missing: 22 lines
  • Coverage: 79%

mssql_python/pybind/ddbc_bindings.cpp

Lines 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

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-size: large Substantial code update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants