Skip to content

Conversation

gargsaumya
Copy link
Contributor

@gargsaumya gargsaumya commented Sep 25, 2025

Work Item / Issue Reference

AB#34944

GitHub Issue: #<ISSUE_NUMBER>


Summary

This pull request adds comprehensive support for the SQL Server DATETIMEOFFSET type in both the Python and C++ layers of the codebase. It introduces new constants, enhances the datetime parsing logic to handle timezone-aware datetimes, and updates the parameter binding and data fetching logic to correctly handle DATETIMEOFFSET values. The changes ensure accurate round-trip of timezone-aware datetime values between Python and SQL Server.

Support for DATETIMEOFFSET type:

  • Added SQL_DATETIMEOFFSET and SQL_C_SS_TIMESTAMPOFFSET constants to both mssql_python/constants.py and the C++ bindings (ddbc_bindings.cpp). [1] [2]
  • Introduced a new DateTimeOffset C++ struct and integrated it into buffer management for column data. [1] [2] [3]
  • Updated the C++ parameter binding logic to correctly marshal Python timezone-aware datetime objects as DATETIMEOFFSET, including array binding for executemany. [1] [2] [3]
  • Implemented fetching and conversion logic for DATETIMEOFFSET columns, ensuring Python receives timezone-aware datetime objects in UTC. [1] [2] [3]

Datetime parsing and mapping improvements:

  • Enhanced _parse_datetime in mssql_python/cursor.py to support parsing timezone-aware datetime strings, improving compatibility with DATETIMEOFFSET.
  • Updated SQL type mapping logic to distinguish between naive and timezone-aware datetime objects, mapping the latter to DATETIMEOFFSET.

Refactoring and cleanup:

  • Removed the now-unused _select_best_sample_value static method from mssql_python/cursor.py as type inference is now handled differently.
  • Adjusted type inference during executemany to use the new _compute_column_type method, aligning with the improved datetime handling.

@Copilot Copilot AI review requested due to automatic review settings September 25, 2025 10:55
Copy link
Contributor

@Copilot 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 adds comprehensive support for the SQL Server DATETIMEOFFSET type in both Python and C++ layers, enabling timezone-aware datetime handling in executemany() operations and regular queries. It introduces new SQL constants, enhances datetime parsing logic to handle timezone information, and implements proper parameter binding and data fetching for DATETIMEOFFSET values.

  • Adds SQL_DATETIMEOFFSET and SQL_C_SS_TIMESTAMPOFFSET constants to support the new SQL type
  • Implements timezone-aware datetime parsing and type mapping logic
  • Removes unused helper method and refactors executemany type inference logic

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
mssql_python/constants.py Adds SQL constants for DATETIMEOFFSET support
mssql_python/cursor.py Enhances datetime parsing and type mapping, removes unused method
mssql_python/pybind/ddbc_bindings.cpp Implements C++ DateTimeOffset struct, parameter binding, and data fetching
tests/test_004_cursor.py Adds comprehensive tests for DATETIMEOFFSET functionality
tests/test_003_connection.py Removes unused import statements

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@sumitmsft sumitmsft left a comment

Choose a reason for hiding this comment

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

Left a few comments

@microsoft microsoft deleted a comment from Copilot AI Sep 29, 2025
@microsoft microsoft deleted a comment from Copilot AI Sep 29, 2025
@github-actions github-actions bot added the pr-size: medium Moderate update size label Sep 29, 2025
@gargsaumya gargsaumya force-pushed the saumya/datetimeoffset-executemany branch from e1253f2 to 5f45001 Compare September 29, 2025 11:52
@gargsaumya gargsaumya force-pushed the saumya/datetimeoffset-executemany branch from 978fe43 to 41c1592 Compare September 29, 2025 12:10
@gargsaumya gargsaumya force-pushed the saumya/datetimeoffset-executemany branch from 41c1592 to 93df1a2 Compare September 29, 2025 12:21
Copy link

github-actions bot commented Sep 29, 2025

📊 Code Coverage Report

🔥 Diff Coverage

85%


🎯 Overall Coverage

73%


📈 Total Lines Covered: 4052 out of 5513
📁 Project: mssql-python


Diff Coverage

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

  • mssql_python/cursor.py (100%)
  • mssql_python/pybind/ddbc_bindings.cpp (85.5%): Missing lines 1950-1951,1954-1955,1959-1961,1986,3227-3228,3230

Summary

  • Total: 77 lines
  • Missing: 11 lines
  • Coverage: 85%

mssql_python/pybind/ddbc_bindings.cpp

Lines 1946-1965

  1946                     for (size_t i = 0; i < paramSetSize; ++i) {
  1947                         const py::handle& param = columnValues[i];
  1948 
  1949                         if (param.is_none()) {
! 1950                             std::memset(&dtoArray[i], 0, sizeof(DateTimeOffset));
! 1951                             strLenOrIndArray[i] = SQL_NULL_DATA;
  1952                         } else {
  1953                             if (!py::isinstance(param, datetimeType)) {
! 1954                                 ThrowStdException(MakeParamMismatchErrorStr(info.paramCType, paramIndex));
! 1955                             }
  1956 
  1957                             py::object tzinfo = param.attr("tzinfo");
  1958                             if (tzinfo.is_none()) {
! 1959                                 ThrowStdException("Datetime object must have tzinfo for SQL_C_SS_TIMESTAMPOFFSET at paramIndex " +
! 1960                                     std::to_string(paramIndex));
! 1961                             }
  1962 
  1963                             // Populate the C++ struct directly from the Python datetime object.
  1964                             dtoArray[i].year   = static_cast<SQLSMALLINT>(param.attr("year").cast<int>());
  1965                             dtoArray[i].month  = static_cast<SQLUSMALLINT>(param.attr("month").cast<int>());

Lines 1982-1990

  1982                     }
  1983                     dataPtr = dtoArray;
  1984                     bufferLength = sizeof(DateTimeOffset);
  1985                     break;
! 1986                 }
  1987                 case SQL_C_NUMERIC: {
  1988                     SQL_NUMERIC_STRUCT* numericArray = AllocateParamBufferArray<SQL_NUMERIC_STRUCT>(tempBuffers, paramSetSize);
  1989                     strLenOrIndArray = AllocateParamBufferArray<SQLLEN>(tempBuffers, paramSetSize);
  1990                     for (size_t i = 0; i < paramSetSize; ++i) {

Lines 3223-3234

  3223                         );
  3224                         py_dt = py_dt.attr("astimezone")(datetime.attr("timezone").attr("utc"));
  3225                         row.append(py_dt);
  3226                     } else {
! 3227                         row.append(py::none());
! 3228                     }
  3229                     break;
! 3230                 }
  3231                 case SQL_GUID: {
  3232                     SQLGUID* guidValue = &buffers.guidBuffers[col - 1][i];
  3233                     uint8_t reordered[16];
  3234                     reordered[0] = ((char*)&guidValue->Data1)[3];


📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)
mssql_python.pybind.ddbc_bindings.cpp: 67.4%
mssql_python.pybind.connection.connection.cpp: 68.3%
mssql_python.ddbc_bindings.py: 68.5%
mssql_python.pybind.connection.connection_pool.cpp: 78.9%
mssql_python.cursor.py: 79.1%
mssql_python.connection.py: 81.7%
mssql_python.helpers.py: 84.7%
mssql_python.auth.py: 85.3%
mssql_python.type.py: 86.8%
mssql_python.pooling.py: 88.8%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

@gargsaumya gargsaumya merged commit 7b91e8f into main Sep 29, 2025
21 checks passed
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.

3 participants