Merged PR 5317: Fix for Numeric & float type + Common Python/C++ logger

bewithgaurav · Theekshna Kotian · commit 4b89f3a550d9 · 2025-02-19T12:44:08.000Z
Related work items: #33810
diff --git a/main.py b/main.py
@@ -1,21 +1,42 @@
 from mssql_python import connect
+from mssql_python import setup_logging
 import os
+import decimal
+
+setup_logging('stdout')
 
 conn_str = os.getenv("DB_CONNECTION_STRING")
 conn = connect(conn_str)
 
+# conn.autocommit = True
+
 cursor = conn.cursor()
 cursor.execute("SELECT database_id, name from sys.databases;")
 row = cursor.fetchmany(1)
 print(row)
 
-cursor.execute("SELECT database_id, name from sys.databases;")
-row = cursor.fetchone()
-print(row)
+cursor.execute("DROP TABLE IF EXISTS main_single_column")
+
+# cursor.execute("CREATE TABLE main_single_column (float_column FLOAT)")
+# cursor.executemany("INSERT INTO main_single_column (float_column) VALUES (?)", [[12.34], [1.234], [0.125], [0.0125], [0.00125], [23243243232.432432432], [0.247985732852735032750973209750]])
+# cursor.execute("SELECT * FROM main_single_column")
+# row = cursor.fetchall()
+# print(row)
+# print(len(row))
+
+import time
+cursor.execute("CREATE TABLE main_single_column (decimal_column NUMERIC(10, 4))")
+# time.sleep(45)
+
+cursor.execute("INSERT INTO main_single_column (decimal_column) VALUES (?)", [decimal.Decimal(123.45).quantize(decimal.Decimal('0.00'))])
+cursor.execute("SELECT * FROM main_single_column")
+row = cursor.fetchone()[0]
 
-cursor.execute("SELECT database_id, name from sys.databases;")
-row = cursor.fetchall()
 print(row)
+print(row.val)
+print(row.precision)
+print(row.scale)
+print(row.sign)
 
 cursor.close()
 conn.close()
diff --git a/mssql_python/__init__.py b/mssql_python/__init__.py
@@ -27,3 +27,5 @@
 
 # Cursor Objects
 from .cursor import Cursor
+
+from .logging_config import setup_logging, get_logger
diff --git a/mssql_python/connection.py b/mssql_python/connection.py
@@ -1,18 +1,12 @@
 import ctypes
 from mssql_python.cursor import Cursor
-from mssql_python.logging_config import setup_logging, ENABLE_LOGGING
+from mssql_python.logging_config import get_logger, ENABLE_LOGGING
 from mssql_python.constants import ConstantsODBC as odbc_sql_const
 from mssql_python.helpers import add_driver_to_connection_str, check_error
-import logging
 import os
-
-# Change the current working directory to the directory of the script to import ddbc_bindings
-os.chdir(os.path.dirname(__file__))
-
 from mssql_python import ddbc_bindings
 
-# Setting up logging
-setup_logging()
+logger = get_logger()
 
 class Connection:
     """
@@ -116,15 +110,15 @@ def _connect_to_db(self) -> None:
             InterfaceError: If there is an error related to the database interface.
         """
         if ENABLE_LOGGING:
-            logging.info("Connecting to the database")
+            logger.info("Connecting to the database")
         ret = ddbc_bindings.DDBCSQLDriverConnect(
             self.hdbc.value, # Connection handle
             0, # Window handle
             self.connection_str # Connection string
         )
         check_error(odbc_sql_const.SQL_HANDLE_DBC.value, self.hdbc.value, ret)
         if ENABLE_LOGGING:
-            logging.info("Connection established successfully.")
+            logger.info("Connection established successfully.")
 
     @property
     def autocommit(self) -> bool:
@@ -159,7 +153,7 @@ def setautocommit(self, value: bool) -> None:
         check_error(odbc_sql_const.SQL_HANDLE_DBC.value, self.hdbc.value, ret)
         self._autocommit = value
         if ENABLE_LOGGING:
-            logging.info("Autocommit mode set to %s.", value)
+            logger.info("Autocommit mode set to %s.", value)
 
     def cursor(self) -> Cursor:
         """
@@ -198,7 +192,7 @@ def commit(self) -> None:
         )
         check_error(odbc_sql_const.SQL_HANDLE_DBC.value, self.hdbc.value, ret)
         if ENABLE_LOGGING:
-            logging.info("Transaction committed successfully.")
+            logger.info("Transaction committed successfully.")
 
     def rollback(self) -> None:
         """
@@ -219,7 +213,7 @@ def rollback(self) -> None:
         )
         check_error(odbc_sql_const.SQL_HANDLE_DBC.value, self.hdbc.value, ret)
         if ENABLE_LOGGING:
-            logging.info("Transaction rolled back successfully.")
+            logger.info("Transaction rolled back successfully.")
 
     def close(self) -> None:
         """
@@ -243,4 +237,4 @@ def close(self) -> None:
         check_error(odbc_sql_const.SQL_HANDLE_DBC.value, self.hdbc.value, ret)
         
         if ENABLE_LOGGING:
-            logging.info("Connection closed successfully.")
+            logger.info("Connection closed successfully.")
diff --git a/mssql_python/constants.py b/mssql_python/constants.py
@@ -105,3 +105,4 @@ class ConstantsODBC(Enum):
     SQL_PARAM_INPUT_OUTPUT = 3
     SQL_C_WCHAR = -8
     SQL_NULLABLE = 1
+    SQL_MAX_NUMERIC_LEN = 16
diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py
@@ -1,19 +1,17 @@
 import ctypes
-import logging
 import decimal, uuid
 from typing import List, Union
-from mssql_python.logging_config import setup_logging, ENABLE_LOGGING
 from mssql_python.constants import ConstantsODBC as odbc_sql_const
 from mssql_python.helpers import check_error
+from mssql_python.logging_config import get_logger, ENABLE_LOGGING
 import datetime
 import decimal
 import uuid
 import os
 from mssql_python.exceptions import raise_exception
 from mssql_python import ddbc_bindings
 
-# Setting up logging
-setup_logging()
+logger = get_logger()
 
 class Cursor:
     """
@@ -60,7 +58,7 @@ def __init__(self, connection) -> None:
                                         # Is a list instead of a bool coz bools in Python are immutable.
                                         # Hence, we can't pass around bools by reference & modify them.
                                         # Therefore, it must be a list with exactly one bool element.
-        
+
     def _is_unicode_string(self, param):
         """
         Check if a string contains non-ASCII characters.
@@ -207,28 +205,67 @@ def _get_numeric_data(self, param):
         Get the data for a numeric parameter.
         
         Args:
-            param: The numeric parameter.
+            param: The numeric parameter. 
         
         Returns:
-            A tuple containing the numeric data.
-        """
+            A NumericData struct containing the numeric data.
+        """
+        decimal_as_tuple = param.as_tuple()
+        num_digits = len(decimal_as_tuple.digits)
+        exponent = decimal_as_tuple.exponent
+
+        # Calculate the SQL precision & scale
+        #   precision = no. of significant digits
+        #   scale     = no. digits after decimal point
+        if exponent >= 0:
+            # digits=314, exp=2 ---> '31400' --> precision=5, scale=0
+            precision = num_digits + exponent
+            scale = 0
+        elif (-1 * exponent) <= num_digits:
+            # digits=3140, exp=-3 ---> '3.140' --> precision=4, scale=3
+            precision = num_digits
+            scale = exponent * -1
+        else:
+            # digits=3140, exp=-5 ---> '0.03140' --> precision=5, scale=5
+            # TODO: double check the precision calculation here with SQL documentation
+            precision = exponent * -1
+            scale = exponent * -1
+
+        # TODO: Revisit this check, do we want this restriction?
+        if precision > 15:
+            raise ValueError("Precision of the numeric value is too high - " + str(param) +
+                             ". Should be less than or equal to 15")
         NumericData = ddbc_bindings.NumericData
         numeric_data = NumericData()
-        numeric_data.precision = len(param.as_tuple().digits)
-        numeric_data.scale = param.as_tuple().exponent * -1
-        numeric_data.sign = param.as_tuple().sign
-        numeric_data.val = str(param)
-
+        numeric_data.scale = scale
+        numeric_data.precision = precision
+        numeric_data.sign = 1 if decimal_as_tuple.sign == 0 else 0
+        # strip decimal point from param & convert the significant digits to integer
+        # Ex: 12.34 ---> 1234
+        val = str(param)
+        if '.' in val:
+            val = val.replace('.', '')
+            val = val.replace('-', '')
+            val = int(val)
+        numeric_data.val = val
         return numeric_data
 
     def _map_sql_type(self, param, parameters_list, i):
-        """Map a Python data type to the corresponding SQL type,C type,Columnsize and Decimal digits."""
+        """
+        Map a Python data type to the corresponding SQL type,C type,Columnsize and Decimal digits.
+        Takes:
+            - param: The parameter to map.
+            - parameters_list: The list of parameters to bind.
+            - i: The index of the parameter in the list.
+        Returns:
+            - A tuple containing the SQL type, C type, column size, and decimal digits.
+        """
         if param is None:
-            return odbc_sql_const.SQL_NULL_DATA.value, odbc_sql_const.SQL_C_DEFAULT.value, 1, 0  
-        
+            return odbc_sql_const.SQL_NULL_DATA.value, odbc_sql_const.SQL_C_DEFAULT.value, 1, 0
+
         elif isinstance(param, bool):
             return odbc_sql_const.SQL_BIT.value, odbc_sql_const.SQL_C_BIT.value, 1, 0
-        
+
         elif isinstance(param, int):
             if 0 <= param <= 255:
                 return odbc_sql_const.SQL_TINYINT.value, odbc_sql_const.SQL_C_TINYINT.value, 3, 0
@@ -238,22 +275,20 @@ def _map_sql_type(self, param, parameters_list, i):
                 return odbc_sql_const.SQL_INTEGER.value, odbc_sql_const.SQL_C_LONG.value, 10, 0
             else:
                 return odbc_sql_const.SQL_BIGINT.value, odbc_sql_const.SQL_C_SBIGINT.value, 19, 0
-        
+
         elif isinstance(param, float):
-            if -3.4028235E+38 <= param <= 3.4028235E+38:
-                return odbc_sql_const.SQL_REAL.value, odbc_sql_const.SQL_C_FLOAT.value, 7, 0
-            else:
-                return odbc_sql_const.SQL_FLOAT.value, odbc_sql_const.SQL_C_DOUBLE.value, 15, 0
-        
+            return odbc_sql_const.SQL_DOUBLE.value, odbc_sql_const.SQL_C_DOUBLE.value, 15, 0
+
         elif isinstance(param, decimal.Decimal):
+            # TODO: Support for other numeric types (smallmoney, money etc.)
             # 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
-            return odbc_sql_const.SQL_DECIMAL.value, odbc_sql_const.SQL_C_NUMERIC.value, len(param.as_tuple().digits), param.as_tuple().exponent * -1
-        
+            return odbc_sql_const.SQL_NUMERIC.value, odbc_sql_const.SQL_C_NUMERIC.value, parameters_list[i].precision, parameters_list[i].scale
+
         elif isinstance(param, str):
             # Check for Well-Known Text (WKT) format for geography/geometry
             if param.startswith("POINT") or param.startswith("LINESTRING") or param.startswith("POLYGON"):
@@ -269,7 +304,7 @@ def _map_sql_type(self, param, parameters_list, i):
             elif self._parse_time(param):
                 parameters_list[i] = self._parse_time(param)
                 return odbc_sql_const.SQL_TIME.value, odbc_sql_const.SQL_C_TYPE_TIME.value, 8, 0
-            # Unsupported types
+            # TODO: Support for other types (Timestampoffset etc.)
             # elif self._parse_timestamptz(param):
             #     return odbc_sql_const.SQL_TIMESTAMPOFFSET.value, odbc_sql_const.SQL_C_TYPE_TIMESTAMP.value, 34, 7
             # elif self._parse_smalldatetime(param):
@@ -466,7 +501,7 @@ def execute(self, operation: str, *parameters, use_prepare: bool = True, reset_c
 
         ParamInfo = ddbc_bindings.ParamInfo
         parameters_type = []
-        
+
         # Flatten parameters if a single tuple or list is passed
         if len(parameters) == 1 and isinstance(parameters[0], (tuple, list)):
             parameters = parameters[0]
@@ -487,19 +522,19 @@ def execute(self, operation: str, *parameters, use_prepare: bool = True, reset_c
         '''
         Execute SQL Statement - (SQLExecute)
         '''
+        # TODO - Need to evaluate encrypted logs for query parameters
         if ENABLE_LOGGING:
-            # TODO - Need to evaluate encrypted logs for query parameters
-            logging.debug("Executing query: %s", operation)
+            logger.debug("Executing query: %s", operation)
             for i, param in enumerate(parameters):
-                logging.debug(
+                logger.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].paramSQLType,
+                    parameters_type[i].paramCType,
+                    parameters_type[i].columnSize,
+                    parameters_type[i].decimalDigits,
                     parameters_type[i].inputOutputType
                 )
 
@@ -538,7 +573,7 @@ def executemany(self, operation: str, seq_of_parameters: list) -> None:
                 # Converting the parameters to a list
                 parameters = list(parameters)
                 if ENABLE_LOGGING:
-                    logging.info("Executing query with parameters: %s", parameters)
+                    logger.info("Executing query with parameters: %s", parameters)
                 # Prepare the statement only during first execution. From second time
                 # onwards, skip preparing and directly execute. This helps avoid
                 # unnecessary 'prepare' network calls.
@@ -558,7 +593,7 @@ def executemany(self, operation: str, seq_of_parameters: list) -> None:
             self.rowcount = total_rowcount
         except Exception as e:
             if ENABLE_LOGGING:
-                logging.info("Executing query with parameters: %s", parameters)
+                logger.info("Executing query with parameters: %s", parameters)
             # Prepare the statement only during first execution. From second time
             # onwards, skip preparing and directly execute. This helps avoid
             # unnecessary 'prepare' network calls.
diff --git a/mssql_python/db_connection.py b/mssql_python/db_connection.py
@@ -1,11 +1,6 @@
-import logging
-from mssql_python.logging_config import setup_logging
 from mssql_python.exceptions import DatabaseError, InterfaceError
 from mssql_python.connection import Connection
 
-# Setting up logging
-setup_logging()
-
 def connect(connection_str: str) -> Connection:
     """
     Constructor for creating a connection to the database.
diff --git a/mssql_python/exceptions.py b/mssql_python/exceptions.py
@@ -1,7 +1,6 @@
-from mssql_python.logging_config import setup_logging, ENABLE_LOGGING
-import logging
+from mssql_python.logging_config import get_logger, ENABLE_LOGGING
 
-setup_logging()
+logger = get_logger()
 
 class Exception(Exception):
     """
@@ -231,7 +230,7 @@ def truncate_error_message(error_message: str) -> str:
         return string_first + string_third
     except Exception as e:
         if ENABLE_LOGGING:
-            logging.error(f"Error while truncating error message: {e}")
+            logger.error(f"Error while truncating error message: {e}")
         return error_message
 
 def raise_exception(sqlstate: str, ddbc_error: str) -> None:
@@ -250,6 +249,6 @@ def raise_exception(sqlstate: str, ddbc_error: str) -> None:
     exception_class = sqlstate_to_exception(sqlstate, ddbc_error)
     if exception_class:
         if ENABLE_LOGGING:
-            logging.error(exception_class)
+            logger.error(exception_class)
         raise exception_class
-    raise DatabaseError(driver_error="An error occurred with SQLSTATE code", ddbc_error=f"Unknown DDBC error: {sqlstate}")
+    raise DatabaseError(driver_error="An error occurred with SQLSTATE code", ddbc_error=f"Unknown DDBC error: {sqlstate}")
diff --git a/mssql_python/helpers.py b/mssql_python/helpers.py
@@ -1,10 +1,9 @@
 from mssql_python.constants import ConstantsODBC
 from mssql_python import ddbc_bindings
 from mssql_python.exceptions import raise_exception
-from mssql_python.logging_config import setup_logging, ENABLE_LOGGING
-import logging
+from mssql_python.logging_config import get_logger, ENABLE_LOGGING
 
-setup_logging()
+logger = get_logger()
 
 def add_driver_to_connection_str(connection_str):
     """
@@ -62,7 +61,7 @@ def check_error(handle_type, handle, ret):
     if ret < 0:
         error_info = ddbc_bindings.DDBCSQLCheckError(handle_type, handle, ret)
         if ENABLE_LOGGING:
-            logging.error(f"Error: {error_info.ddbcErrorMsg}")
+            logger.error(f"Error: {error_info.ddbcErrorMsg}")
         raise_exception(error_info.sqlState, error_info.ddbcErrorMsg)
 
 def add_driver_name_to_app_parameter(connection_string):
diff --git a/mssql_python/logging_config.py b/mssql_python/logging_config.py
diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp
diff --git a/mssql_python/testing_ddbc_bindings.py b/mssql_python/testing_ddbc_bindings.py
diff --git a/tests/test_004_cursor.py b/tests/test_004_cursor.py
