Skip to content

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

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

FIX: Stored datetime.time values have the microseconds attribute set to zero#479
jahnvi480 wants to merge 6 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 significant improvements to how SQL TIME/TIME2 values are handled in the MSSQL Python driver, transitioning from native C-type bindings to text-based representations. The changes ensure correct parsing, binding, and conversion between SQL TIME values and Python datetime.time objects, addressing edge cases and improving compatibility.

SQL TIME/TIME2 Handling Improvements

  • Changed TIME/TIME2 parameter binding in mssql_python/cursor.py to use SQL_TYPE_TIME and text C-types (SQL_C_CHAR), and normalized Python datetime.time values to ISO text format with microseconds.
  • Updated C++ bindings in ddbc_bindings.cpp to bind and fetch TIME/TIME2 columns as text buffers instead of native structs, and introduced a robust parser (ParseSqlTimeTextToPythonObject) for converting SQL time text to Python objects.
  • Adjusted row size calculations and buffer allocations for TIME/TIME2 columns to use the new maximum text length constant (SQL_TIME_TEXT_MAX_LEN).

Testing and Utilities

  • Exposed the time-text parser as a test helper in the Python module, allowing for unit testing of TIME/TIME2 parsing edge cases.

Miscellaneous

  • Improved table cleanup in tests by switching to drop_table_if_exists for reliability.

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

98%


🎯 Overall Coverage

77%


📈 Total Lines Covered: 5734 out of 7395
📁 Project: mssql-python


Diff Coverage

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

  • mssql_python/cursor.py (100%)
  • mssql_python/pybind/ddbc_bindings.cpp (98.8%): Missing lines 3335

Summary

  • Total: 91 lines
  • Missing: 1 line
  • Coverage: 98%

mssql_python/pybind/ddbc_bindings.cpp

Lines 3331-3339

  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                 }


📋 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: 70.4%
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

@github-actions github-actions bot added pr-size: medium Moderate update size and removed pr-size: large Substantial code update labels Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-size: medium Moderate update size

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants