Skip to content

fix!: Add DATETIMEOFFSET handling in pyodbc for MSSQL #4930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

mattiasthalen
Copy link
Contributor

@mattiasthalen mattiasthalen commented Jul 7, 2025

This pull request introduces enhancements to MSSQL connection handling, particularly for the pyodbc driver, and updates dependency specifications for pyodbc across the project. The most significant changes include adding support for MSSQL's DATETIMEOFFSET type, improving test coverage for this feature, and ensuring the pyodbc version is explicitly specified to avoid compatibility issues.

MSSQL Connection Enhancements:

  • sqlmesh/core/config/connection.py: Added support for MSSQL's DATETIMEOFFSET type by implementing a custom output converter at the connection level for pyodbc. The converter handles binary data for the DATETIMEOFFSET SQL type (-155) and converts it into Python's datetime objects with timezone information.

Test Coverage Improvements:

  • tests/core/test_connection_config.py: Added tests to validate the DATETIMEOFFSET handling for pyodbc. These include scenarios for positive and negative timezone offsets, ensuring proper conversion of binary data to datetime objects. Additionally, confirmed that _cursor_init is no longer required for pymssql due to the connection-level setup.

Dependency Updates:

  • pyproject.toml: Updated the pyodbc dependency to require version >=5.0.0 across multiple optional dependency groups (azuresql-odbc, mssql-odbc, and dev) to ensure compatibility with the new features. [1] [2] [3]

@mattiasthalen mattiasthalen force-pushed the fix/mssql-pyodbc-datetimeoffset branch from fe30ea2 to a637f37 Compare July 7, 2025 23:20
@mattiasthalen mattiasthalen marked this pull request as ready for review July 7, 2025 23:20
@mattiasthalen mattiasthalen force-pushed the fix/mssql-pyodbc-datetimeoffset branch from 4ec0b84 to 7ed0734 Compare July 8, 2025 11:15
@mattiasthalen
Copy link
Contributor Author

@erindru hm, the tests fails when I run them on the main branch as well.

@georgesittas
Copy link
Contributor

@mattiasthalen try rebasing off of main, @themisvaltinos fixed CI.

@mattiasthalen mattiasthalen force-pushed the fix/mssql-pyodbc-datetimeoffset branch 2 times, most recently from 5affaba to 0c169be Compare July 8, 2025 16:55
Copy link
Contributor

@georgesittas georgesittas left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me, @mattiasthalen.

Just double-checking, but this is backwards-compatible, yes? In the sense that DATETIMEOFFSET would previously lead to an error, if pyodbc was used?

@georgesittas georgesittas requested a review from erindru July 8, 2025 17:56
@mattiasthalen
Copy link
Contributor Author

Yes, without it PYODBC would throw an error about data type -155 ☺️

@mattiasthalen mattiasthalen force-pushed the fix/mssql-pyodbc-datetimeoffset branch from 0c169be to 6043490 Compare July 9, 2025 06:34
@mattiasthalen mattiasthalen changed the title Fix: Add DATETIMEOFFSET handling for MSSQL cursor initialization fix(mssql): Add DATETIMEOFFSET handling in pyodbc for MSSQL Jul 9, 2025
Copy link
Contributor

@georgesittas georgesittas left a comment

Choose a reason for hiding this comment

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

I'll mark this as breaking due to the version constraint.

@georgesittas georgesittas changed the title fix(mssql): Add DATETIMEOFFSET handling in pyodbc for MSSQL fix!: Add DATETIMEOFFSET handling in pyodbc for MSSQL Jul 9, 2025
@mattiasthalen mattiasthalen force-pushed the fix/mssql-pyodbc-datetimeoffset branch from a7e6a20 to fab2de4 Compare July 9, 2025 15:34
@mattiasthalen mattiasthalen force-pushed the fix/mssql-pyodbc-datetimeoffset branch from fab2de4 to 774b6b3 Compare July 9, 2025 21:41
Copy link
Collaborator

@erindru erindru left a comment

Choose a reason for hiding this comment

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

Great work, thanks!

This ended up in a much simpler place than where we started

@erindru erindru merged commit 461b2a2 into TobikoData:main Jul 9, 2025
23 checks passed
@mattiasthalen mattiasthalen deleted the fix/mssql-pyodbc-datetimeoffset branch July 9, 2025 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants