Merged PR 5315: Added Tests, Major Fixes, Logging and Support Datatypes

#### AI description  (iteration 1)
#### PR Classification
Bug fix and new feature implementation.

#### PR Summary
This pull request includes major fixes, logging enhancements, and support for new data types, along with added tests.
- `mssql_python/exceptions.py`: Refactored exception handling, added logging, and improved error message truncation.
- `mssql_python/cursor.py`: Enhanced logging for query execution and parameter handling, fixed data type handling for `decimal.Decimal` and `datetime`.
- Added new test files `tests/test_005_exceptions.py` and `tests/test_types.py` to cover exception handling and data type support.
- `mssql_python/pybind/ddbc_bindings.cpp`: Improved handling of datetime and error messages, added detailed logging.
- `mssql_python/helpers.py`: Updated error checking to raise custom exceptions with detailed logging.
<!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot -->

Related work items: #33809, #33810

Author: bewithgaurav

mssql_python/__init__.py

@@ -23,8 +23,7 @@
 
 # Connection Objects
 from .connection import Connection
+from .db_connection import connect
 
 # Cursor Objects
 from .cursor import Cursor
-
-from .db_connection import connect

mssql_python/cursor.py

@@ -222,16 +222,6 @@ def _get_numeric_data(self, param):
         numeric_data.sign = param.as_tuple().sign
         numeric_data.val = str(param)
 
-        print("NUMERIC DATA!!!", numeric_data.precision, numeric_data.scale, numeric_data.sign, numeric_data.val)
-        # precision = param.as_tuple().digits
-        # scale = param.as_tuple().exponent * -1
-        # sign = param.as_tuple().sign
-        # numeric_data = {
-        #     'precision': len(precision),
-        #     'scale': scale,
-        #     'sign': sign,
-        #     'value': param
-        # }
         return numeric_data
 
     def _map_sql_type(self, param, parameters_list, i):
@@ -259,13 +249,12 @@ def _map_sql_type(self, param, parameters_list, i):
                 return odbc_sql_const.SQL_FLOAT.value, odbc_sql_const.SQL_C_DOUBLE.value, 15, 0
 
         elif isinstance(param, decimal.Decimal):
-            if param.as_tuple().exponent == -4:  # Scale is 4
-                if -214748.3648 <= param <= 214748.3647:
-                    return odbc_sql_const.SQL_SMALLMONEY.value, odbc_sql_const.SQL_C_NUMERIC.value, 10, 4
-                elif -922337203685477.5808 <= param <= 922337203685477.5807:
-                    return odbc_sql_const.SQL_MONEY.value, odbc_sql_const.SQL_C_NUMERIC.value, 19, 4
+            # if param.as_tuple().exponent == -4:  # Scale is 4
+            #     if -214748.3648 <= param <= 214748.3647:
+            #         return odbc_sql_const.SQL_SMALLMONEY.value, odbc_sql_const.SQL_C_NUMERIC.value, 10, 4
+            #     elif -922337203685477.5808 <= param <= 922337203685477.5807:
+            #         return odbc_sql_const.SQL_MONEY.value, odbc_sql_const.SQL_C_NUMERIC.value, 19, 4
             parameters_list[i] = self._get_numeric_data(param)  # Replace the parameter with the dictionary
-            print("DECIMAL!!!", odbc_sql_const.SQL_DECIMAL.value, odbc_sql_const.SQL_C_NUMERIC.value)
             return odbc_sql_const.SQL_DECIMAL.value, odbc_sql_const.SQL_C_NUMERIC.value, len(param.as_tuple().digits), param.as_tuple().exponent * -1
 
         elif isinstance(param, str):
@@ -319,12 +308,13 @@ def _map_sql_type(self, param, parameters_list, i):
         elif isinstance(param, uuid.UUID):  # Handle uniqueidentifier
             return odbc_sql_const.SQL_GUID.value, odbc_sql_const.SQL_C_GUID.value, 36, 0
 
-        elif isinstance(param, datetime.date):
-            return odbc_sql_const.SQL_DATE.value, odbc_sql_const.SQL_C_TYPE_DATE.value, 10, 0
-        
         elif isinstance(param, datetime.datetime):
+            # Always keep datetime.datetime check before datetime.date check since datetime.datetime is a subclass of datetime (isinstance(datetime.datetime, datetime.date) returns True)
             return odbc_sql_const.SQL_TIMESTAMP.value, odbc_sql_const.SQL_C_TYPE_TIMESTAMP.value, 23, 3
 
+        elif isinstance(param, datetime.date):
+            return odbc_sql_const.SQL_DATE.value, odbc_sql_const.SQL_C_TYPE_DATE.value, 10, 0
+        
         elif isinstance(param, datetime.time):
             return odbc_sql_const.SQL_TIME.value, odbc_sql_const.SQL_C_TYPE_TIME.value, 8, 0
 
@@ -336,11 +326,7 @@ def _initialize_cursor(self) -> None:
         """
         Initialize the ODBC statement handle.
         """
-        # Allocate the statement handle
-        # try:
         self._allocate_statement_handle()
-        # except Exception as e:
-        #     logging.error("An error occurred during initialization: %s", e)
 
     def _allocate_statement_handle(self):
             """
@@ -504,6 +490,22 @@ def execute(self, operation: str, *parameters, use_prepare: bool = True, reset_c
         '''
         Execute SQL Statement - (SQLExecute)
         '''
+        if ENABLE_LOGGING:
+            # TODO - Need to evaluate encrypted logs for query parameters
+            logging.debug("Executing query: %s", operation)
+            for i, param in enumerate(parameters):
+                logging.debug(
+                    "Parameter number: %s, Parameter: %s, Param Python Type: %s, ParamInfo: %s, %s, %s, %s, %s",
+                    i+1,
+                    param,
+                    str(type(param)),
+                    parameters_type[i].paramSQLType, 
+                    parameters_type[i].paramCType, 
+                    parameters_type[i].columnSize, 
+                    parameters_type[i].decimalDigits, 
+                    parameters_type[i].inputOutputType
+                )
+
         ret = ddbc_bindings.DDBCSQLExecute(self.hstmt.value, operation, parameters, parameters_type,
                                             self.is_stmt_prepared, use_prepare)
         check_error(odbc_sql_const.SQL_HANDLE_STMT.value, self.hstmt.value, ret)

mssql_python/exceptions.py

@@ -9,7 +9,7 @@ class Exception(Exception):
     """
     def __init__(self, driver_error, ddbc_error) -> None:
         self.driver_error = driver_error
-        self.ddbc_error = ddbc_error
+        self.ddbc_error = truncate_error_message(ddbc_error)
         self.message = f"Driver Error: {self.driver_error}; DDBC Error: {self.ddbc_error}"
         super().__init__(self.message)
 
@@ -218,13 +218,8 @@ def sqlstate_to_exception(sqlstate: str, ddbc_error: str) -> Exception:
 
 def truncate_error_message(error_message: str) -> str:
     '''
-    Sample Error Message:
-    mssql_python.exceptions.ProgrammingError: Driver Error: Syntax error or access violation; DDBC Error: [Microsoft][ODBC Driver 18 for SQL Server][SQL Server]Incorrect syntax near the keyword 'from'.
-    Remove [ODBC Driver 18 for SQL Server] from the error message.
-    - The Driver Error message is the message that is returned by the ODBC driver.
-    - It will always start with [ODBC Driver <version> for SQL Server].
-    - The DDBC Error message is the message that is returned by the database server.
-    - this section will always be at the start of the message.
+    - The Driver Error message is the message that is returned by the Internal driver.
+    - This section will always be at the start of the message.
     '''
     try:
         if not error_message.startswith('[Microsoft]'):
@@ -252,10 +247,9 @@ def raise_exception(sqlstate: str, ddbc_error: str) -> None:
     Raises:
         DatabaseError: If the SQLSTATE code is not found in the mapping.
     """
-    exception_class = sqlstate_to_exception(
-        sqlstate,
-        truncate_error_message(ddbc_error)
-    )
+    exception_class = sqlstate_to_exception(sqlstate, ddbc_error)
     if exception_class:
+        if ENABLE_LOGGING:
+            logging.error(exception_class)
         raise exception_class
     raise DatabaseError(driver_error="An error occurred with SQLSTATE code", ddbc_error=f"Unknown DDBC error: {sqlstate}")

mssql_python/helpers.py

@@ -59,14 +59,12 @@ def check_error(handle_type, handle, ret):
     Raises:
         RuntimeError: If an error is found.
     """
-    if ENABLE_LOGGING:
-        logging.debug(f"Checking error for handle type: {handle_type}, handle: {handle}, ret: {ret}")
-    if ret == ConstantsODBC.SQL_ERROR.value:
+    if ret < 0:
         error_info = ddbc_bindings.DDBCSQLCheckError(handle_type, handle, ret)
         if ENABLE_LOGGING:
             logging.error(f"Error: {error_info.ddbcErrorMsg}")
         raise_exception(error_info.sqlState, error_info.ddbcErrorMsg)
-    
+
 def add_driver_name_to_app_parameter(connection_string):
     """
     Modifies the input connection string by appending the APP name.

mssql_python/logging_config.py

@@ -5,7 +5,7 @@
 # Variable to enable or disable logging
 ENABLE_LOGGING = False
 
-def setup_logging(log_level=logging.INFO):
+def setup_logging(log_level=logging.DEBUG):
     """
     Set up logging configuration.
 

mssql_python/pybind/ddbc_bindings.cpp

@@ -12,6 +12,7 @@
 #include <pybind11/chrono.h>
 #include <pybind11/complex.h>
 #include <pybind11/functional.h>
+#include <pybind11/pytypes.h>  // Add this line for datetime support
 #include <pybind11/stl.h>
 #include <windows.h>  // windows.h needs to be included before sql.h
 #include <sql.h>
@@ -957,7 +958,13 @@ SQLRETURN SQLGetData_wrap(intptr_t StatementHandle, SQLUSMALLINT colCount, py::l
                 ret =
                     SQLGetData_ptr(hStmt, i, SQL_C_TYPE_DATE, &dateValue, sizeof(dateValue), NULL);
                 if (SQL_SUCCEEDED(ret)) {
-                    row.append(py::make_tuple(dateValue.year, dateValue.month, dateValue.day));
+                    row.append(
+                        py::module_::import("datetime").attr("date")(
+                            dateValue.year,
+                            dateValue.month,
+                            dateValue.day
+                        )
+                    );
                 } else {
                     row.append(py::none());
                 }
@@ -970,7 +977,13 @@ SQLRETURN SQLGetData_wrap(intptr_t StatementHandle, SQLUSMALLINT colCount, py::l
                 ret =
                     SQLGetData_ptr(hStmt, i, SQL_C_TYPE_TIME, &timeValue, sizeof(timeValue), NULL);
                 if (SQL_SUCCEEDED(ret)) {
-                    row.append(py::make_tuple(timeValue.hour, timeValue.minute, timeValue.second));
+                    row.append(
+                        py::module_::import("datetime").attr("time")(
+                            timeValue.hour,
+                            timeValue.minute,
+                            timeValue.second
+                        )
+                    );
                 } else {
                     row.append(py::none());
                 }
@@ -983,9 +996,16 @@ SQLRETURN SQLGetData_wrap(intptr_t StatementHandle, SQLUSMALLINT colCount, py::l
                 ret = SQLGetData_ptr(hStmt, i, SQL_C_TYPE_TIMESTAMP, &timestampValue,
                                      sizeof(timestampValue), NULL);
                 if (SQL_SUCCEEDED(ret)) {
-                    row.append(py::make_tuple(timestampValue.year, timestampValue.month,
-                                              timestampValue.day, timestampValue.hour,
-                                              timestampValue.minute, timestampValue.second));
+                    row.append(
+                        py::module_::import("datetime").attr("datetime")(
+                            timestampValue.year,
+                            timestampValue.month,
+                            timestampValue.day,
+                            timestampValue.hour,
+                            timestampValue.minute,
+                            timestampValue.second
+                        )
+                    );
                 } else {
                     row.append(py::none());
                 }
@@ -1305,27 +1325,40 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum
                     case SQL_TIMESTAMP:
                     case SQL_TYPE_TIMESTAMP:
                     case SQL_DATETIME:
-                        row.append(py::make_tuple(buffers.timestampBuffers[col - 1][i].year,
-                                                  buffers.timestampBuffers[col - 1][i].month,
-                                                  buffers.timestampBuffers[col - 1][i].day,
-                                                  buffers.timestampBuffers[col - 1][i].hour,
-                                                  buffers.timestampBuffers[col - 1][i].minute,
-                                                  buffers.timestampBuffers[col - 1][i].second));
+                        row.append(
+                            py::module_::import("datetime").attr("datetime")(
+                                buffers.timestampBuffers[col - 1][i].year,
+                                buffers.timestampBuffers[col - 1][i].month,
+                                buffers.timestampBuffers[col - 1][i].day,
+                                buffers.timestampBuffers[col - 1][i].hour,
+                                buffers.timestampBuffers[col - 1][i].minute,
+                                buffers.timestampBuffers[col - 1][i].second
+                            )
+                            
+                        );
                         break;
                     case SQL_BIGINT:
                         row.append(buffers.bigIntBuffers[col - 1][i]);
                         break;
                     case SQL_TYPE_DATE:
-                        row.append(py::make_tuple(buffers.dateBuffers[col - 1][i].year,
-                                                  buffers.dateBuffers[col - 1][i].month,
-                                                  buffers.dateBuffers[col - 1][i].day));
+                        row.append(
+                            py::module_::import("datetime").attr("date")(
+                                buffers.dateBuffers[col - 1][i].year,
+                                buffers.dateBuffers[col - 1][i].month,
+                                buffers.dateBuffers[col - 1][i].day
+                            )   
+                        );
                         break;
                     case SQL_TIME:
                     case SQL_TYPE_TIME:
                     case SQL_SS_TIME2:
-                        row.append(py::make_tuple(buffers.timeBuffers[col - 1][i].hour,
-                                                  buffers.timeBuffers[col - 1][i].minute,
-                                                  buffers.timeBuffers[col - 1][i].second));
+                        row.append(
+                            py::module_::import("datetime").attr("time")(
+                                buffers.timeBuffers[col - 1][i].hour,
+                                buffers.timeBuffers[col - 1][i].minute,
+                                buffers.timeBuffers[col - 1][i].second
+                            )
+                        );
                         break;
                     case SQL_GUID:
                         row.append(py::bytes(

mssql_python/type.py

@@ -66,13 +66,13 @@ def TimeFromTicks(ticks: int) -> datetime.time:
     """
     Generates a time object from ticks.
     """
-    return datetime.time(*time.localtime(ticks)[3:6])
+    return datetime.time(*time.gmtime(ticks)[3:6])
 
 def TimestampFromTicks(ticks: int) -> datetime.datetime:
     """
     Generates a timestamp object from ticks.
     """
-    return datetime.datetime.fromtimestamp(ticks)
+    return datetime.datetime.fromtimestamp(ticks, datetime.UTC)
 
 def Binary(string: str) -> bytes:
     """

tests/conftest.py

@@ -38,4 +38,3 @@ def db_connection(conn_str):
 def cursor(db_connection):
     cursor = db_connection.cursor()
     yield cursor
-

tests/test_globals.py tests/test_001_globals.pytests/test_globals.py renamed to tests/test_001_globals.py

@@ -21,4 +21,4 @@ def test_threadsafety():
 
 def test_paramstyle():
     # Check if paramstyle has the expected value
-    assert paramstyle == "qmark", "paramstyle should be 'qmark'"
+    assert paramstyle == "qmark", "paramstyle should be 'qmark'"

tests/test_002_types.py

@@ -0,0 +1,57 @@
+import pytest
+import datetime
+from mssql_python.type import STRING, BINARY, NUMBER, DATETIME, ROWID, Date, Time, Timestamp, DateFromTicks, TimeFromTicks, TimestampFromTicks, Binary
+
+def test_string_type():
+    assert STRING().type == "STRING", "STRING type mismatch"
+
+def test_binary_type():
+    assert BINARY().type == "BINARY", "BINARY type mismatch"
+
+def test_number_type():
+    assert NUMBER().type == "NUMBER", "NUMBER type mismatch"
+
+def test_datetime_type():
+    assert DATETIME().type == "DATETIME", "DATETIME type mismatch"
+
+def test_rowid_type():
+    assert ROWID().type == "ROWID", "ROWID type mismatch"
+
+def test_date_constructor():
+    date = Date(2023, 10, 5)
+    assert isinstance(date, datetime.date), "Date constructor did not return a date object"
+    assert date.year == 2023 and date.month == 10 and date.day == 5, "Date constructor returned incorrect date"
+
+def test_time_constructor():
+    time = Time(12, 30, 45)
+    assert isinstance(time, datetime.time), "Time constructor did not return a time object"
+    assert time.hour == 12 and time.minute == 30 and time.second == 45, "Time constructor returned incorrect time"
+
+def test_timestamp_constructor():
+    timestamp = Timestamp(2023, 10, 5, 12, 30, 45)
+    assert isinstance(timestamp, datetime.datetime), "Timestamp constructor did not return a datetime object"
+    assert timestamp.year == 2023 and timestamp.month == 10 and timestamp.day == 5, "Timestamp constructor returned incorrect date"
+    assert timestamp.hour == 12 and timestamp.minute == 30 and timestamp.second == 45, "Timestamp constructor returned incorrect time"
+
+def test_date_from_ticks():
+    ticks = 1696500000  # Corresponds to 2023-10-05
+    date = DateFromTicks(ticks)
+    assert isinstance(date, datetime.date), "DateFromTicks did not return a date object"
+    assert date == datetime.date(2023, 10, 5), "DateFromTicks returned incorrect date"
+
+def test_time_from_ticks():
+    ticks = 1696500000  # Corresponds to 10:00:00
+    time = TimeFromTicks(ticks)
+    assert isinstance(time, datetime.time), "TimeFromTicks did not return a time object"
+    assert time == datetime.time(10, 0, 0), "TimeFromTicks returned incorrect time"
+
+def test_timestamp_from_ticks():
+    ticks = 1696500000  # Corresponds to 2023-10-05 10:00:00
+    timestamp = TimestampFromTicks(ticks)
+    assert isinstance(timestamp, datetime.datetime), "TimestampFromTicks did not return a datetime object"
+    assert timestamp == datetime.datetime(2023, 10, 5, 10, 0, 0, tzinfo=datetime.timezone.utc), "TimestampFromTicks returned incorrect timestamp"
+
+def test_binary_constructor():
+    binary = Binary("test")
+    assert isinstance(binary, bytes), "Binary constructor did not return a bytes object"
+    assert binary == b"test", "Binary constructor returned incorrect bytes"