From f647b8c0c9e15df61ae21aa76cb89560d7bc0719 Mon Sep 17 00:00:00 2001 From: Chetan-Kansal Date: Mon, 22 Dec 2025 02:01:34 +0530 Subject: [PATCH 1/2] FIX: clarify decimal parsing in bulk fetch and add regression test --- mssql_python/pybind/ddbc_bindings.cpp | 4 +- tests/test_decimal_separator.py | 70 +++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 tests/test_decimal_separator.py diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 63696f91..d9cfa4f3 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -2899,7 +2899,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p SQLHSTMT hStmt = StatementHandle->get(); // Cache decimal separator to avoid repeated system calls - std::string decimalSeparator = GetDecimalSeparator(); + for (SQLSMALLINT i = 1; i <= colCount; ++i) { SQLWCHAR columnName[256]; @@ -3622,7 +3622,7 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum columnInfos[col].processedColumnSize + 1; // +1 for null terminator } - std::string decimalSeparator = GetDecimalSeparator(); // Cache decimal separator + // Performance: Build function pointer dispatch table (once per batch) // This eliminates the switch statement from the hot loop - 10,000 rows × 10 diff --git a/tests/test_decimal_separator.py b/tests/test_decimal_separator.py new file mode 100644 index 00000000..54578b6f --- /dev/null +++ b/tests/test_decimal_separator.py @@ -0,0 +1,70 @@ +import pytest +import mssql_python +import os +from decimal import Decimal + +# Helper to get connection +def get_connection(): + server = os.getenv("TEST_SERVER", "localhost") + database = os.getenv("TEST_DATABASE", "master") + username = os.getenv("TEST_USERNAME", "sa") + password = os.getenv("TEST_PASSWORD", "yourStrong(!)Password") + driver = os.getenv("TEST_DRIVER", "ODBC Driver 17 for SQL Server") + + conn_str = f"DRIVER={{{driver}}};SERVER={server};DATABASE={database};UID={username};PWD={password};TrustServerCertificate=yes" + return mssql_python.connect(conn_str) + +@pytest.mark.skipif(not os.getenv("TEST_SERVER"), reason="TEST_SERVER not set") +def test_decimal_separator_bug(): + """ + Test that fetchall() dealing with DECIMALS works correctly even when + setDecimalSeparator is set to something other than '.' + """ + conn = get_connection() + cursor = conn.cursor() + + try: + # Create a temp table + cursor.execute("CREATE TABLE #TestDecimal (Val DECIMAL(10, 2))") + cursor.execute("INSERT INTO #TestDecimal VALUES (1234.56)") + cursor.execute("INSERT INTO #TestDecimal VALUES (78.90)") + conn.commit() + + # Set custom separator + mssql_python.setDecimalSeparator(",") + + # Test fetchall + cursor.execute("SELECT Val FROM #TestDecimal ORDER BY Val") + rows = cursor.fetchall() + + # Verify fetchall results + assert len(rows) == 2, f"Expected 2 rows, got {len(rows)}" + assert isinstance(rows[0][0], Decimal), f"Expected Decimal, got {type(rows[0][0])}" + assert rows[0][0] == Decimal("78.90"), f"Expected 78.90, got {rows[0][0]}" + assert rows[1][0] == Decimal("1234.56"), f"Expected 1234.56, got {rows[1][0]}" + + # Verify fetchmany + cursor.execute("SELECT Val FROM #TestDecimal ORDER BY Val") + batch = cursor.fetchmany(2) + assert len(batch) == 2 + assert batch[1][0] == Decimal("1234.56") + + # Verify fetchone behavior is consistent + cursor.execute("SELECT CAST(99.99 AS DECIMAL(10,2))") + val = cursor.fetchone()[0] + assert isinstance(val, Decimal) + assert val == Decimal("99.99") + + finally: + # Reset separator to default just in case + mssql_python.setDecimalSeparator(".") + cursor.execute("DROP TABLE #TestDecimal") + conn.close() + +if __name__ == "__main__": + # Allow running as a script too + try: + test_decimal_separator_bug() + print("Test PASSED") + except Exception as e: + print(f"Test FAILED: {e}") From 1aef4ec698527163958c901a8b77fd9753ded409 Mon Sep 17 00:00:00 2001 From: Chetan-Kansal Date: Fri, 26 Dec 2025 23:31:03 +0530 Subject: [PATCH 2/2] Fix CI: Refactor test_decimal_separator.py for linting, coverage, and security --- tests/test_decimal_separator.py | 51 +++++++++++++-------------------- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/tests/test_decimal_separator.py b/tests/test_decimal_separator.py index 54578b6f..1839d9b7 100644 --- a/tests/test_decimal_separator.py +++ b/tests/test_decimal_separator.py @@ -1,29 +1,21 @@ -import pytest -import mssql_python import os from decimal import Decimal -# Helper to get connection -def get_connection(): - server = os.getenv("TEST_SERVER", "localhost") - database = os.getenv("TEST_DATABASE", "master") - username = os.getenv("TEST_USERNAME", "sa") - password = os.getenv("TEST_PASSWORD", "yourStrong(!)Password") - driver = os.getenv("TEST_DRIVER", "ODBC Driver 17 for SQL Server") - - conn_str = f"DRIVER={{{driver}}};SERVER={server};DATABASE={database};UID={username};PWD={password};TrustServerCertificate=yes" - return mssql_python.connect(conn_str) +import pytest +import mssql_python + -@pytest.mark.skipif(not os.getenv("TEST_SERVER"), reason="TEST_SERVER not set") -def test_decimal_separator_bug(): +@pytest.mark.skipif(not os.getenv("DB_CONNECTION_STRING"), reason="Requires DB_CONNECTION_STRING") +def test_decimal_separator_bug(db_connection): """ - Test that fetchall() dealing with DECIMALS works correctly even when + Test that fetchall() dealing with DECIMALS works correctly even when setDecimalSeparator is set to something other than '.' """ - conn = get_connection() - cursor = conn.cursor() - + conn = db_connection + cursor = None + try: + cursor = conn.cursor() # Create a temp table cursor.execute("CREATE TABLE #TestDecimal (Val DECIMAL(10, 2))") cursor.execute("INSERT INTO #TestDecimal VALUES (1234.56)") @@ -32,17 +24,17 @@ def test_decimal_separator_bug(): # Set custom separator mssql_python.setDecimalSeparator(",") - + # Test fetchall cursor.execute("SELECT Val FROM #TestDecimal ORDER BY Val") rows = cursor.fetchall() - + # Verify fetchall results assert len(rows) == 2, f"Expected 2 rows, got {len(rows)}" assert isinstance(rows[0][0], Decimal), f"Expected Decimal, got {type(rows[0][0])}" assert rows[0][0] == Decimal("78.90"), f"Expected 78.90, got {rows[0][0]}" assert rows[1][0] == Decimal("1234.56"), f"Expected 1234.56, got {rows[1][0]}" - + # Verify fetchmany cursor.execute("SELECT Val FROM #TestDecimal ORDER BY Val") batch = cursor.fetchmany(2) @@ -58,13 +50,10 @@ def test_decimal_separator_bug(): finally: # Reset separator to default just in case mssql_python.setDecimalSeparator(".") - cursor.execute("DROP TABLE #TestDecimal") - conn.close() - -if __name__ == "__main__": - # Allow running as a script too - try: - test_decimal_separator_bug() - print("Test PASSED") - except Exception as e: - print(f"Test FAILED: {e}") + if cursor: + try: + cursor.execute("DROP TABLE #TestDecimal") + conn.commit() + except Exception: + pass + cursor.close()