diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index 52fcbfd2..8c77f578 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -673,10 +673,10 @@ def _map_sql_type( # pylint: disable=too-many-arguments,too-many-positional-arg if isinstance(param, datetime.time): return ( - ddbc_sql_const.SQL_TIME.value, - ddbc_sql_const.SQL_C_TYPE_TIME.value, - 8, - 0, + ddbc_sql_const.SQL_TYPE_TIME.value, + ddbc_sql_const.SQL_C_CHAR.value, + 16, + 6, False, ) @@ -941,6 +941,16 @@ def _create_parameter_types_list( # pylint: disable=too-many-arguments,too-many parameter, parameters_list, i, min_val=min_val, max_val=max_val ) + # If TIME values are being bound via text C-types, normalize them to a + # textual representation expected by SQL_C_CHAR/SQL_C_WCHAR binding. + if isinstance(parameter, datetime.time) and c_type in ( + ddbc_sql_const.SQL_C_CHAR.value, + ddbc_sql_const.SQL_C_WCHAR.value, + ): + time_text = parameter.isoformat(timespec="microseconds") + parameters_list[i] = time_text + column_size = max(column_size, len(time_text)) + paraminfo.paramCType = c_type paraminfo.paramSQLType = sql_type paraminfo.inputOutputType = ddbc_sql_const.SQL_PARAM_INPUT.value @@ -2250,6 +2260,12 @@ def executemany( # pylint: disable=too-many-locals,too-many-branches,too-many-s for i, val in enumerate(processed_row): if val is None: continue + if isinstance(val, datetime.time) and parameters_type[i].paramCType in ( + ddbc_sql_const.SQL_C_CHAR.value, + ddbc_sql_const.SQL_C_WCHAR.value, + ): + processed_row[i] = val.isoformat(timespec="microseconds") + continue if ( isinstance(val, decimal.Decimal) and parameters_type[i].paramSQLType == ddbc_sql_const.SQL_VARCHAR.value diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 0933d4fa..d5d3d84f 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -9,6 +9,7 @@ #include "connection/connection_pool.h" #include "logger_bridge.hpp" +#include #include #include // For std::memcpy #include @@ -28,6 +29,7 @@ #define SQL_MAX_NUMERIC_LEN 16 #define SQL_SS_XML (-152) #define SQL_SS_UDT (-151) +#define SQL_TIME_TEXT_MAX_LEN 32 #define STRINGIFY_FOR_CASE(x) \ case x: \ @@ -53,6 +55,79 @@ inline std::string GetEffectiveCharDecoding(const std::string& userEncoding) { #endif } +namespace PythonObjectCache { +py::object get_time_class(); +} + +inline py::object ParseSqlTimeTextToPythonObject(const char* timeText, SQLLEN timeTextLen) { + if (!timeText || (timeTextLen <= 0 && timeTextLen != SQL_NO_TOTAL)) { + return py::none(); + } + + size_t len; + if (timeTextLen == SQL_NO_TOTAL) { + // When the driver reports SQL_NO_TOTAL, the buffer may not be null-terminated. + // Bound the scan to the maximum expected TIME/TIME2 text length. + const void* nul = std::memchr(timeText, '\0', SQL_TIME_TEXT_MAX_LEN - 1); + len = nul ? static_cast(static_cast(nul) - timeText) + : static_cast(SQL_TIME_TEXT_MAX_LEN - 1); + } else { + len = static_cast(timeTextLen); + if (len > SQL_TIME_TEXT_MAX_LEN - 1) { + len = SQL_TIME_TEXT_MAX_LEN - 1; + } + } + + std::string value(timeText, len); + + size_t start = value.find_first_not_of(" \t\r\n"); + if (start == std::string::npos) { + return py::none(); + } + size_t end = value.find_last_not_of(" \t\r\n"); + value = value.substr(start, end - start + 1); + + size_t firstColon = value.find(':'); + size_t secondColon = + (firstColon == std::string::npos) ? std::string::npos : value.find(':', firstColon + 1); + if (firstColon == std::string::npos || secondColon == std::string::npos) { + ThrowStdException("Failed to parse TIME/TIME2 value: missing ':' separators"); + } + + int hour = std::stoi(value.substr(0, firstColon)); + int minute = std::stoi(value.substr(firstColon + 1, secondColon - firstColon - 1)); + + size_t dotPos = value.find('.', secondColon + 1); + int second = 0; + int microsecond = 0; + + if (dotPos == std::string::npos) { + second = std::stoi(value.substr(secondColon + 1)); + } else { + second = std::stoi(value.substr(secondColon + 1, dotPos - secondColon - 1)); + std::string frac = value.substr(dotPos + 1); + + size_t digitCount = 0; + while (digitCount < frac.size() && + std::isdigit(static_cast(frac[digitCount]))) { + ++digitCount; + } + frac = frac.substr(0, digitCount); + + if (frac.size() > 6) { + frac = frac.substr(0, 6); + } + while (frac.size() < 6) { + frac.push_back('0'); + } + if (!frac.empty()) { + microsecond = std::stoi(frac); + } + } + + return PythonObjectCache::get_time_class()(hour, minute, second, microsecond); +} + //------------------------------------------------------------------------------------------------- //------------------------------------------------------------------------------------------------- // Logging Infrastructure: @@ -3244,17 +3319,20 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p } break; } - case SQL_TIME: - case SQL_TYPE_TIME: case SQL_SS_TIME2: { - SQL_TIME_STRUCT timeValue; - ret = - SQLGetData_ptr(hStmt, i, SQL_C_TYPE_TIME, &timeValue, sizeof(timeValue), NULL); + char timeTextBuffer[SQL_TIME_TEXT_MAX_LEN] = {0}; + SQLLEN timeDataLen = 0; + ret = SQLGetData_ptr(hStmt, i, SQL_C_CHAR, timeTextBuffer, sizeof(timeTextBuffer), + &timeDataLen); if (SQL_SUCCEEDED(ret)) { - row.append(PythonObjectCache::get_time_class()(timeValue.hour, timeValue.minute, - timeValue.second)); + if (timeDataLen == SQL_NULL_DATA) { + // Normal NULL value: append None without logging an error. + row.append(py::none()); + } else { + row.append(ParseSqlTimeTextToPythonObject(timeTextBuffer, timeDataLen)); + } } else { - LOG("SQLGetData: Error retrieving SQL_TYPE_TIME for column " + LOG("SQLGetData: Error retrieving SQL_SS_TIME2 for column " "%d - SQLRETURN=%d", i, ret); row.append(py::none()); @@ -3585,13 +3663,10 @@ SQLRETURN SQLBindColums(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& column SQLBindCol_ptr(hStmt, col, SQL_C_TYPE_DATE, buffers.dateBuffers[col - 1].data(), sizeof(SQL_DATE_STRUCT), buffers.indicators[col - 1].data()); break; - case SQL_TIME: - case SQL_TYPE_TIME: case SQL_SS_TIME2: - buffers.timeBuffers[col - 1].resize(fetchSize); - ret = - SQLBindCol_ptr(hStmt, col, SQL_C_TYPE_TIME, buffers.timeBuffers[col - 1].data(), - sizeof(SQL_TIME_STRUCT), buffers.indicators[col - 1].data()); + buffers.charBuffers[col - 1].resize(fetchSize * SQL_TIME_TEXT_MAX_LEN); + ret = SQLBindCol_ptr(hStmt, col, SQL_C_CHAR, buffers.charBuffers[col - 1].data(), + SQL_TIME_TEXT_MAX_LEN, buffers.indicators[col - 1].data()); break; case SQL_GUID: buffers.guidBuffers[col - 1].resize(fetchSize); @@ -3895,16 +3970,12 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum PyList_SET_ITEM(row, col - 1, dateObj); break; } - case SQL_TIME: - case SQL_TYPE_TIME: case SQL_SS_TIME2: { - PyObject* timeObj = - PythonObjectCache::get_time_class()(buffers.timeBuffers[col - 1][i].hour, - buffers.timeBuffers[col - 1][i].minute, - buffers.timeBuffers[col - 1][i].second) - .release() - .ptr(); - PyList_SET_ITEM(row, col - 1, timeObj); + const char* rawData = reinterpret_cast( + &buffers.charBuffers[col - 1][i * SQL_TIME_TEXT_MAX_LEN]); + SQLLEN timeDataLen = buffers.indicators[col - 1][i]; + py::object timeObj = ParseSqlTimeTextToPythonObject(rawData, timeDataLen); + PyList_SET_ITEM(row, col - 1, timeObj.release().ptr()); break; } case SQL_SS_TIMESTAMPOFFSET: { @@ -4034,10 +4105,8 @@ size_t calculateRowSize(py::list& columnNames, SQLUSMALLINT numCols) { case SQL_TYPE_DATE: rowSize += sizeof(SQL_DATE_STRUCT); break; - case SQL_TIME: - case SQL_TYPE_TIME: case SQL_SS_TIME2: - rowSize += sizeof(SQL_TIME_STRUCT); + rowSize += SQL_TIME_TEXT_MAX_LEN; break; case SQL_GUID: rowSize += sizeof(SQLGUID); @@ -4048,7 +4117,8 @@ size_t calculateRowSize(py::list& columnNames, SQLUSMALLINT numCols) { break; case SQL_SS_UDT: rowSize += (static_cast(columnSize) == SQL_NO_TOTAL || columnSize == 0) - ? SQL_MAX_LOB_SIZE : columnSize; + ? SQL_MAX_LOB_SIZE + : columnSize; break; case SQL_BINARY: case SQL_VARBINARY: @@ -4112,8 +4182,7 @@ SQLRETURN FetchMany_wrap(SqlHandlePtr StatementHandle, py::list& rows, int fetch if ((dataType == SQL_WVARCHAR || dataType == SQL_WLONGVARCHAR || dataType == SQL_VARCHAR || dataType == SQL_LONGVARCHAR || dataType == SQL_VARBINARY || - dataType == SQL_LONGVARBINARY || dataType == SQL_SS_XML || - dataType == SQL_SS_UDT) && + dataType == SQL_LONGVARBINARY || dataType == SQL_SS_XML || dataType == SQL_SS_UDT) && (columnSize == 0 || columnSize == SQL_NO_TOTAL || columnSize > SQL_MAX_LOB_SIZE)) { lobColumns.push_back(i + 1); // 1-based } @@ -4252,8 +4321,7 @@ SQLRETURN FetchAll_wrap(SqlHandlePtr StatementHandle, py::list& rows, if ((dataType == SQL_WVARCHAR || dataType == SQL_WLONGVARCHAR || dataType == SQL_VARCHAR || dataType == SQL_LONGVARCHAR || dataType == SQL_VARBINARY || - dataType == SQL_LONGVARBINARY || dataType == SQL_SS_XML || - dataType == SQL_SS_UDT) && + dataType == SQL_LONGVARBINARY || dataType == SQL_SS_XML || dataType == SQL_SS_UDT) && (columnSize == 0 || columnSize == SQL_NO_TOTAL || columnSize > SQL_MAX_LOB_SIZE)) { lobColumns.push_back(i + 1); // 1-based } @@ -4432,6 +4500,12 @@ PYBIND11_MODULE(ddbc_bindings, m) { // Expose architecture-specific constants m.attr("ARCHITECTURE") = ARCHITECTURE; + // Test helper: expose time-text parser for unit testing edge cases + m.def("_test_parse_time_text", &ParseSqlTimeTextToPythonObject, + "Parse a SQL TIME/TIME2 text buffer into a Python datetime.time object (test helper)", + py::arg("timeText"), py::arg("timeTextLen")); + m.attr("SQL_NO_TOTAL") = static_cast(SQL_NO_TOTAL); + // Expose the C++ functions to Python m.def("ThrowStdException", &ThrowStdException); m.def("GetDriverPathCpp", &GetDriverPathCpp, "Get the path to the ODBC driver"); diff --git a/tests/test_004_cursor.py b/tests/test_004_cursor.py index 80995b6e..70e27482 100644 --- a/tests/test_004_cursor.py +++ b/tests/test_004_cursor.py @@ -305,7 +305,7 @@ def test_insert_nvarchar_column(cursor, db_connection): except Exception as e: pytest.fail(f"Nvarchar column insertion/fetch failed: {e}") finally: - cursor.execute("DROP TABLE #pytest_single_column") + drop_table_if_exists(cursor, "#pytest_single_column") db_connection.commit() @@ -326,10 +326,223 @@ def test_insert_time_column(cursor, db_connection): except Exception as e: pytest.fail(f"Time column insertion/fetch failed: {e}") finally: - cursor.execute("DROP TABLE #pytest_single_column") + drop_table_if_exists(cursor, "#pytest_single_column") + db_connection.commit() + + +def test_insert_time_column_preserves_microseconds(cursor, db_connection): + """Test TIME fractional seconds round-trip for datetime.time values.""" + try: + drop_table_if_exists(cursor, "#pytest_time_microseconds") + cursor.execute("CREATE TABLE #pytest_time_microseconds (time_column TIME(6))") + db_connection.commit() + + original_value = time(14, 30, 15, 234567) + cursor.execute( + "INSERT INTO #pytest_time_microseconds (time_column) VALUES (?)", + [original_value], + ) + db_connection.commit() + + cursor.execute("SELECT time_column FROM #pytest_time_microseconds") + row = cursor.fetchone() + assert row is not None, "Expected one row" + assert row[0] == original_value, "TIME microseconds were not preserved" + assert row[0].microsecond == 234567, "Expected microseconds to round-trip" + except Exception as e: + pytest.fail(f"TIME microseconds round-trip failed: {e}") + finally: + drop_table_if_exists(cursor, "#pytest_time_microseconds") + db_connection.commit() + + +def test_time_column_with_null(cursor, db_connection): + """Test TIME(2) column with NULL values — exercises NULL branch in SQLGetData path.""" + try: + drop_table_if_exists(cursor, "#pytest_time_null") + cursor.execute("CREATE TABLE #pytest_time_null (id INT, time_column TIME(2))") + db_connection.commit() + cursor.execute("INSERT INTO #pytest_time_null VALUES (1, '10:30:00.00')") + cursor.execute("INSERT INTO #pytest_time_null VALUES (2, NULL)") + cursor.execute("INSERT INTO #pytest_time_null VALUES (3, '23:59:59.99')") + db_connection.commit() + + cursor.execute("SELECT time_column FROM #pytest_time_null ORDER BY id") + rows = cursor.fetchall() + assert len(rows) == 3 + assert rows[0][0] == time(10, 30, 0) + assert rows[1][0] is None, "NULL TIME should be returned as None" + assert rows[2][0] == time(23, 59, 59, 990000) + except Exception as e: + pytest.fail(f"TIME with NULL test failed: {e}") + finally: + drop_table_if_exists(cursor, "#pytest_time_null") + db_connection.commit() + + +def test_time_column_null_fetchone(cursor, db_connection): + """Test fetchone on a NULL TIME value — exercises NULL branch in per-row SQLGetData path.""" + try: + drop_table_if_exists(cursor, "#pytest_time_null_one") + cursor.execute("CREATE TABLE #pytest_time_null_one (time_column TIME(2))") + db_connection.commit() + cursor.execute("INSERT INTO #pytest_time_null_one VALUES (NULL)") + db_connection.commit() + + cursor.execute("SELECT time_column FROM #pytest_time_null_one") + row = cursor.fetchone() + assert row is not None, "Expected one row" + assert row[0] is None, "NULL TIME should be returned as None" + except Exception as e: + pytest.fail(f"TIME NULL fetchone test failed: {e}") + finally: + drop_table_if_exists(cursor, "#pytest_time_null_one") + db_connection.commit() + + +def test_time_column_no_fractional_seconds(cursor, db_connection): + """Test TIME(0) column without fractional seconds — exercises dotPos==npos branch.""" + try: + drop_table_if_exists(cursor, "#pytest_time_nofrac") + cursor.execute("CREATE TABLE #pytest_time_nofrac (time_column TIME(0))") + db_connection.commit() + cursor.execute( + "INSERT INTO #pytest_time_nofrac (time_column) VALUES (?)", [time(8, 15, 30)] + ) + db_connection.commit() + + cursor.execute("SELECT time_column FROM #pytest_time_nofrac") + row = cursor.fetchone() + assert row is not None + assert row[0] == time(8, 15, 30) + except Exception as e: + pytest.fail(f"TIME(0) no fractional seconds test failed: {e}") + finally: + drop_table_if_exists(cursor, "#pytest_time_nofrac") + db_connection.commit() + + +def test_time_column_batch_fetch(cursor, db_connection): + """Test fetching multiple TIME(6) rows via fetchall — exercises batch/column-bound path.""" + try: + drop_table_if_exists(cursor, "#pytest_time_batch") + cursor.execute("CREATE TABLE #pytest_time_batch (id INT, time_column TIME(6))") + db_connection.commit() + + expected = [ + time(0, 0, 0), + time(6, 30, 0, 123456), + time(12, 0, 0), + time(18, 45, 59, 999999), + time(23, 59, 59, 0), + ] + for i, t in enumerate(expected): + cursor.execute("INSERT INTO #pytest_time_batch (id, time_column) VALUES (?, ?)", [i, t]) + db_connection.commit() + + cursor.execute("SELECT time_column FROM #pytest_time_batch ORDER BY id") + rows = cursor.fetchall() + assert len(rows) == len(expected) + for i, row in enumerate(rows): + assert row[0] == expected[i], f"Row {i}: expected {expected[i]}, got {row[0]}" + except Exception as e: + pytest.fail(f"TIME batch fetch test failed: {e}") + finally: + drop_table_if_exists(cursor, "#pytest_time_batch") + db_connection.commit() + + +def test_time_executemany(cursor, db_connection): + """Test executemany with TIME column — exercises cursor.py time→isoformat conversion.""" + try: + drop_table_if_exists(cursor, "#pytest_time_execmany") + cursor.execute("CREATE TABLE #pytest_time_execmany (id INT, time_column TIME(6))") + db_connection.commit() + + values = [ + (1, time(9, 0, 0)), + (2, time(14, 30, 15, 234567)), + (3, time(23, 59, 59, 999999)), + ] + cursor.executemany( + "INSERT INTO #pytest_time_execmany (id, time_column) VALUES (?, ?)", values + ) + db_connection.commit() + + cursor.execute("SELECT id, time_column FROM #pytest_time_execmany ORDER BY id") + rows = cursor.fetchall() + assert len(rows) == 3 + for (exp_id, exp_time), row in zip(values, rows): + assert row[0] == exp_id + assert row[1] == exp_time, f"id {exp_id}: expected {exp_time}, got {row[1]}" + except Exception as e: + pytest.fail(f"TIME executemany test failed: {e}") + finally: + drop_table_if_exists(cursor, "#pytest_time_execmany") db_connection.commit() +# --------------------------------------------------------------------------- +# Unit tests for ParseSqlTimeTextToPythonObject (exposed via _test_parse_time_text) +# These exercise defensive C++ branches that are unreachable through normal +# ODBC queries: null/zero-length input, SQL_NO_TOTAL, oversized length, +# whitespace-only buffers, and malformed text without colon separators. +# --------------------------------------------------------------------------- + + +class TestParseSqlTimeText: + """Direct tests for the C++ ParseSqlTimeTextToPythonObject helper.""" + + @staticmethod + def _parse(text, length): + from mssql_python.ddbc_bindings import _test_parse_time_text + + return _test_parse_time_text(text, length) + + @staticmethod + def _sql_no_total(): + from mssql_python.ddbc_bindings import SQL_NO_TOTAL + + return SQL_NO_TOTAL + + def test_zero_length_returns_none(self): + """Lines 64-65: timeTextLen <= 0 → py::none()""" + assert self._parse("12:00:00", 0) is None + + def test_negative_length_returns_none(self): + """Lines 64-65: timeTextLen < 0 (but not SQL_NO_TOTAL) → py::none()""" + assert self._parse("12:00:00", -999) is None + + def test_sql_no_total_uses_bounded_scan(self): + """Lines 71-73: SQL_NO_TOTAL → memchr-bounded length scan.""" + result = self._parse("14:30:00.123456", self._sql_no_total()) + assert result == time(14, 30, 0, 123456) + + def test_oversized_length_is_clamped(self): + """Lines 77-78: timeTextLen > SQL_TIME_TEXT_MAX_LEN-1 → clamp.""" + result = self._parse("09:15:30.000000", 9999) + assert result == time(9, 15, 30) + + def test_whitespace_only_returns_none(self): + """Lines 85-86: value is all whitespace → py::none()""" + assert self._parse(" \t\n ", 7) is None + + def test_missing_colon_raises(self): + """Lines 94-95: no ':' separators → ThrowStdException.""" + with pytest.raises(Exception, match="missing ':' separators"): + self._parse("no-colons-here", 14) + + def test_normal_time_with_fraction(self): + """Sanity check: normal parse path.""" + result = self._parse("08:05:03.100000", 15) + assert result == time(8, 5, 3, 100000) + + def test_normal_time_without_fraction(self): + """Sanity check: parse path without fractional part.""" + result = self._parse("23:59:59", 8) + assert result == time(23, 59, 59) + + def test_insert_datetime_column(cursor, db_connection): """Test inserting data into the datetime_column""" try: @@ -349,7 +562,7 @@ def test_insert_datetime_column(cursor, db_connection): except Exception as e: pytest.fail(f"Datetime column insertion/fetch failed: {e}") finally: - cursor.execute("DROP TABLE #pytest_single_column") + drop_table_if_exists(cursor, "#pytest_single_column") db_connection.commit()