From f8e455677f64eb558a9ecc6c0ff3a8721cbaca9e Mon Sep 17 00:00:00 2001 From: Jahnvi Thakkar Date: Thu, 17 Sep 2026 19:55:55 +0530 Subject: [PATCH] REFACTOR: Optimize checked temporal fetch construction Use direct CPython date/time/datetime construction for exact cached standard types, preserving substituted constructors and exception behavior. Cover row-wise and batch fetch contracts in isolated subprocesses. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 4 + mssql_python/pybind/ddbc_bindings.cpp | 38 ++-- mssql_python/pybind/fetch_temporal.hpp | 59 ++++++ tests/test_038_fetch_temporal.py | 262 +++++++++++++++++++++++++ 4 files changed, 340 insertions(+), 23 deletions(-) create mode 100644 mssql_python/pybind/fetch_temporal.hpp create mode 100644 tests/test_038_fetch_temporal.py diff --git a/CHANGELOG.md b/CHANGELOG.md index aab046b3a..efab12f4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), does not change the default provider or ship any Rust driver binaries. ### Changed +- DATE, TIME, and TIMESTAMP fetch conversion uses checked CPython constructors + for the standard datetime types, while preserving cached substitute constructors, + their positional arguments and exceptions, and fractional-second truncation. + DATETIMEOFFSET, UUID, and Decimal conversion are unchanged. - `mssql-python` now depends on `mssql-python-rs==0.1.0` for `mssql_py_core` instead of embedding files owned by that separately published distribution. - **GH-769 deprecation policy:** The misplaced `GetInfoConstants` members diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 35a3b6da4..db1447081 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -12,6 +12,7 @@ #include "param_detect.hpp" #include "py_ref.hpp" #include "py_type_cache.hpp" +#include "fetch_temporal.hpp" #include "utf_utils.h" #include // std::min @@ -3758,8 +3759,8 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p ret = SQLGetData_ptr(hStmt, i, SQL_C_TYPE_DATE, &dateValue, sizeof(dateValue), NULL); if (SQL_SUCCEEDED(ret)) { - row.append(PyTypeCache::get_date_class_obj()(dateValue.year, dateValue.month, - dateValue.day)); + row.append( + FetchTemporal::date(dateValue.year, dateValue.month, dateValue.day)); } else { row.append(py::none()); } @@ -3771,7 +3772,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p SQLLEN indicator = 0; ret = SQLGetData_ptr(hStmt, i, SQL_C_SS_TIME2, &t2, sizeof(t2), &indicator); if (SQL_SUCCEEDED(ret) && indicator != SQL_NULL_DATA) { - row.append(PyTypeCache::get_time_class_obj()( + row.append(FetchTemporal::time( t2.hour, t2.minute, t2.second, t2.fraction / 1000)); // ns to µs } else { if (!SQL_SUCCEEDED(ret)) { @@ -3790,7 +3791,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p ret = SQLGetData_ptr(hStmt, i, SQL_C_TYPE_TIMESTAMP, ×tampValue, sizeof(timestampValue), NULL); if (SQL_SUCCEEDED(ret)) { - row.append(PyTypeCache::get_datetime_class_obj()( + row.append(FetchTemporal::datetime( timestampValue.year, timestampValue.month, timestampValue.day, timestampValue.hour, timestampValue.minute, timestampValue.second, timestampValue.fraction / 1000 // Convert back ns to µs @@ -4428,32 +4429,23 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum case SQL_TYPE_TIMESTAMP: case SQL_DATETIME: { const SQL_TIMESTAMP_STRUCT& ts = buffers.timestampBuffers[col - 1][i]; - PyObject* datetimeObj = PyTypeCache::get_datetime_class_obj()( - ts.year, ts.month, ts.day, ts.hour, ts.minute, - ts.second, ts.fraction / 1000) - .release() - .ptr(); - PyList_SET_ITEM(row, col - 1, datetimeObj); + py::object datetimeObj = FetchTemporal::datetime( + ts.year, ts.month, ts.day, ts.hour, ts.minute, ts.second, + ts.fraction / 1000); + PyList_SET_ITEM(row, col - 1, datetimeObj.release().ptr()); break; } case SQL_TYPE_DATE: { - PyObject* dateObj = - PyTypeCache::get_date_class_obj()(buffers.dateBuffers[col - 1][i].year, - buffers.dateBuffers[col - 1][i].month, - buffers.dateBuffers[col - 1][i].day) - .release() - .ptr(); - PyList_SET_ITEM(row, col - 1, dateObj); + const SQL_DATE_STRUCT& value = buffers.dateBuffers[col - 1][i]; + py::object dateObj = FetchTemporal::date(value.year, value.month, value.day); + PyList_SET_ITEM(row, col - 1, dateObj.release().ptr()); break; } case SQL_SS_TIME2: { const SQL_SS_TIME2_STRUCT& t2 = buffers.timeBuffers[col - 1][i]; - PyObject* timeObj = - PyTypeCache::get_time_class_obj()(t2.hour, t2.minute, t2.second, - t2.fraction / 1000) // ns to µs - .release() - .ptr(); - PyList_SET_ITEM(row, col - 1, timeObj); + py::object timeObj = + FetchTemporal::time(t2.hour, t2.minute, t2.second, t2.fraction / 1000); + PyList_SET_ITEM(row, col - 1, timeObj.release().ptr()); break; } case SQL_SS_TIMESTAMPOFFSET: { diff --git a/mssql_python/pybind/fetch_temporal.hpp b/mssql_python/pybind/fetch_temporal.hpp new file mode 100644 index 000000000..473069117 --- /dev/null +++ b/mssql_python/pybind/fetch_temporal.hpp @@ -0,0 +1,59 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +#include +#include + +#include "py_ref.hpp" +#include "py_type_cache.hpp" + +namespace FetchTemporal { + +// datetime.h keeps PyDateTimeAPI per translation unit, so these helpers must too. +static inline void ensure_datetime_api() { + if (PyDateTimeAPI == nullptr) { + PyDateTime_IMPORT; + if (PyDateTimeAPI == nullptr) throw py::error_already_set(); + } +} + +static inline py::object date(int year, int month, int day) { + ensure_datetime_api(); + // Cached substitutes must still receive the original constructor call. + if (PyTypeCache::get_date_class() != + reinterpret_cast(PyDateTimeAPI->DateType)) { + return PyTypeCache::get_date_class_obj()(year, month, day); + } + py::object result = steal(PyDate_FromDate(year, month, day)); + if (!result) throw py::error_already_set(); + return result; +} + +static inline py::object time(int hour, int minute, int second, int microsecond) { + ensure_datetime_api(); + if (PyTypeCache::get_time_class() != + reinterpret_cast(PyDateTimeAPI->TimeType)) { + return PyTypeCache::get_time_class_obj()(hour, minute, second, microsecond); + } + py::object result = steal(PyTime_FromTime(hour, minute, second, microsecond)); + if (!result) throw py::error_already_set(); + return result; +} + +static inline py::object datetime(int year, int month, int day, int hour, int minute, int second, + int microsecond) { + ensure_datetime_api(); + if (PyTypeCache::get_datetime_class() != + reinterpret_cast(PyDateTimeAPI->DateTimeType)) { + return PyTypeCache::get_datetime_class_obj()(year, month, day, hour, minute, second, + microsecond); + } + py::object result = + steal(PyDateTime_FromDateAndTime(year, month, day, hour, minute, second, microsecond)); + if (!result) throw py::error_already_set(); + return result; +} + +} // namespace FetchTemporal diff --git a/tests/test_038_fetch_temporal.py b/tests/test_038_fetch_temporal.py new file mode 100644 index 000000000..074a324d5 --- /dev/null +++ b/tests/test_038_fetch_temporal.py @@ -0,0 +1,262 @@ +"""Temporal fetch parity, including cached constructors in fresh interpreters.""" + +import datetime +from decimal import Decimal +import gc +import json +import os +from pathlib import Path +import subprocess +import sys +import uuid + +import pytest + +MODES = ("default", "custom", "date", "time", "datetime", "uuid") +APIS = ("fetchone", "fetchmany", "fetchall", "iteration") + + +@pytest.mark.parametrize("mode", MODES) +def test_fetch_temporal_constructors(mode): + if not os.environ.get("DB_CONNECTION_STRING"): + pytest.skip("DB_CONNECTION_STRING is required") + result = subprocess.run( + [sys.executable, str(Path(__file__).resolve()), mode], + capture_output=True, + text=True, + encoding="utf-8", + timeout=120, + env={**os.environ, "PYTHONIOENCODING": "utf-8"}, + ) + assert result.returncode == 0, result.stdout + result.stderr + report = json.loads(result.stdout) + assert report["mode"] == mode + assert report["checks"] == 24 + + +def _drain(cursor, method): + if method == "iteration": + return [tuple(row) for row in cursor] + result = [] + while True: + if method == "fetchone": + row = cursor.fetchone() + batch = [] if row is None else [row] + elif method == "fetchmany": + batch = cursor.fetchmany(2) + else: + batch = cursor.fetchall() + if not batch: + return result + result.extend(tuple(row) for row in batch) + + +def _probe(mode, root): + originals = { + "date": datetime.date, + "time": datetime.time, + "datetime": datetime.datetime, + "uuid": uuid.UUID, + } + replacements = dict(originals) + calls = [] + state = {"record": False, "raise": None} + + class ConstructorFailure(Exception): + pass + + failure = ConstructorFailure("cached constructor failure") + + def record(name, positional, keywords): + if state["record"]: + calls.append((name, positional, keywords)) + if state["raise"] == name: + raise failure + + if mode != "default": + + def substitute(name, original): + class RecordingTemporal(original): + def __new__(cls, *positional, **keywords): + record(name, positional, keywords) + return original.__new__(cls, *positional, **keywords) + + return RecordingTemporal + + for name in ("date", "time", "datetime"): + replacements[name] = substitute(name, originals[name]) + setattr(datetime, name, replacements[name]) + + class RecordingUUID(originals["uuid"]): + def __init__(self, *, bytes): + record("uuid", (), {"bytes": bytes}) + super().__init__(bytes=bytes) + + replacements["uuid"] = RecordingUUID + uuid.UUID = RecordingUUID + + # Substitutions must precede native module initialization, not just connection creation. + sys.path.insert(0, str(root)) + import mssql_python + + assert Path(mssql_python.ddbc_bindings.module.__file__).resolve().is_relative_to(root) + mssql_python.native_uuid = True + query = """ + SELECT CAST(v.d AS date) AS d, CAST(v.t AS time(7)) AS t, + CAST(v.ts AS datetime2(7)) AS ts, CAST(v.u AS uniqueidentifier) AS u, + CAST(v.n AS nvarchar(64)) AS n, CAST(v.v AS varchar(64)) AS v, + CAST(v.b AS varbinary(64)) AS b, + CASE WHEN v.id = 4 THEN NULL ELSE CAST(123.4567 AS decimal(12,4)) END AS dec, + CASE WHEN v.id = 4 THEN NULL ELSE + CAST('2024-02-29T12:34:56.1234567+05:30' AS datetimeoffset(7)) END AS dto + {extra} + FROM (VALUES + (1, '0001-01-01', '00:00:00.0000000', '0001-01-01T00:00:00.0000000', + '00112233-4455-6677-8899-aabbccddeeff', N'A' + NCHAR(0) + N'\U0001f642', 'abc', 0x00FF), + (2, '2000-02-29', '12:34:56.1234567', '2000-02-29T12:34:56.1234567', + 'ffffffff-ffff-ffff-ffff-ffffffffffff', N'', '', 0x), + (3, '9999-12-31', '23:59:59.9999999', '9999-12-31T23:59:59.9999999', + '00000000-0000-0000-0000-000000000000', N'caf\u00e9', 'xyz', 0x000100), + (4, NULL, NULL, NULL, NULL, NULL, NULL, NULL) + ) AS v(id, d, t, ts, u, n, v, b) + ORDER BY v.id + """ + date, time, timestamp, guid = (originals[k] for k in ("date", "time", "datetime", "uuid")) + dto = timestamp( + 2024, 2, 29, 12, 34, 56, 123456, datetime.timezone(datetime.timedelta(minutes=330)) + ) + expected = [ + ( + date(1, 1, 1), + time(), + timestamp(1, 1, 1), + guid("00112233-4455-6677-8899-aabbccddeeff"), + "A\0\U0001f642", + "abc", + b"\0\xff", + ), + ( + date(2000, 2, 29), + time(12, 34, 56, 123456), + timestamp(2000, 2, 29, 12, 34, 56, 123456), + guid("ffffffff-ffff-ffff-ffff-ffffffffffff"), + "", + "", + b"", + ), + ( + date(9999, 12, 31), + time(23, 59, 59, 999999), + timestamp(9999, 12, 31, 23, 59, 59, 999999), + guid("00000000-0000-0000-0000-000000000000"), + "caf\u00e9", + "xyz", + b"\0\1\0", + ), + ] + expected = [row + (Decimal("123.4567"), dto) for row in expected] + [(None,) * 9] + checks = 0 + recovery_checks = 0 + with mssql_python.connect(os.environ["DB_CONNECTION_STRING"]) as connection: + # The ASCII VARCHAR control also exercises SQL_CHAR without the Windows UTF-8 upgrade. + for encoding, ctype in ( + ("utf-16le", mssql_python.SQL_WCHAR), + ("latin-1", mssql_python.SQL_CHAR), + ): + connection.setdecoding(mssql_python.SQL_CHAR, encoding=encoding, ctype=ctype) + for forced_lob in (False, True): + extra = ", CAST(N'lob' AS nvarchar(max)) AS force_lob" if forced_lob else "" + wanted = [row + (("lob",) if forced_lob else ()) for row in expected] + for method in APIS: + with connection.cursor() as cursor: + cursor.execute(query.format(extra=extra)) + if mode not in ("default", "custom"): + state.update(record=True, **{"raise": mode}) + try: + _drain(cursor, method) + except ConstructorFailure as error: + assert error is failure + else: + raise AssertionError("Constructor failure was swallowed") + finally: + state.update(record=False, **{"raise": None}) + recovery_checks += 1 + cursor.execute(query.format(extra=extra)) + calls.clear() + state["record"] = True + try: + actual = _drain(cursor, method) + finally: + state["record"] = False + assert actual == wanted, (method, encoding, forced_lob, actual) + for row in actual[:3]: + for index, name in enumerate(("date", "time", "datetime", "uuid")): + assert type(row[index]) is replacements[name], (method, name) + for index in (1, 2): + assert row[index].tzinfo is None and row[index].fold == 0 + assert type(row[7]) is Decimal + assert type(row[8]) is replacements["datetime"] + assert row[8].utcoffset() == datetime.timedelta(minutes=330) + assert row[8].fold == 0 + assert all(type(row[i]) is str for i in (4, 5)) + assert type(row[6]) is bytes + if mode != "default": + for name, count in (("date", 3), ("time", 4), ("datetime", 7)): + recorded = [ + call for call in calls if call[0] == name and len(call[1]) != 8 + ] + assert len(recorded) == 3, (method, name, recorded) + assert all(len(p) == count and not k for _, p, k in recorded) + dto_calls = [call for call in calls if len(call[1]) == 8] + assert len(dto_calls) == 3 + assert all(n == "datetime" and not k for n, _, k in dto_calls) + uuid_calls = [call for call in calls if call[0] == "uuid"] + assert len(uuid_calls) == 3 + assert [k["bytes"] for _, _, k in uuid_calls] == [ + row[3].bytes for row in wanted[:3] + ] + assert all(not p and set(k) == {"bytes"} for _, p, k in uuid_calls) + checks += 1 + gc.collect() + + legacy_query = """ + SELECT CAST(v.dt AS datetime), CAST(v.small AS smalldatetime) {extra} + FROM (VALUES + (1, '1753-01-01T00:00:00.000', '1900-01-01T00:00:00'), + (2, '9999-12-31T23:59:59.997', '2079-06-06T23:59:00'), + (3, NULL, NULL) + ) AS v(id, dt, small) ORDER BY v.id + """ + legacy = [ + (timestamp(1753, 1, 1), timestamp(1900, 1, 1)), + (timestamp(9999, 12, 31, 23, 59, 59, 997000), timestamp(2079, 6, 6, 23, 59)), + (None, None), + ] + for forced_lob in (False, True): + extra = ", CAST(N'lob' AS nvarchar(max))" if forced_lob else "" + wanted = [row + (("lob",) if forced_lob else ()) for row in legacy] + for method in APIS: + with connection.cursor() as cursor: + cursor.execute(legacy_query.format(extra=extra)) + calls.clear() + state["record"] = True + try: + actual = _drain(cursor, method) + finally: + state["record"] = False + assert actual == wanted + for row in actual[:2]: + for value in row[:2]: + assert type(value) is replacements["datetime"] + assert value.tzinfo is None and value.fold == 0 + if mode != "default": + assert len(calls) == 4 + assert all(n == "datetime" and len(p) == 7 and not k for n, p, k in calls) + checks += 1 + assert recovery_checks == (16 if mode not in ("default", "custom") else 0) + print(json.dumps({"mode": mode, "checks": checks, "recovery_checks": recovery_checks})) + + +if __name__ == "__main__": + root = Path(sys.argv[2]).resolve() if len(sys.argv) > 2 else Path(__file__).resolve().parents[1] + _probe(sys.argv[1], root)