Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions mssql_python/pybind/ddbc_bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2892,7 +2892,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];
Expand Down Expand Up @@ -3615,7 +3615,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
Expand Down
59 changes: 59 additions & 0 deletions tests/test_decimal_separator.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import os
from decimal import Decimal

import pytest
import mssql_python


@pytest.mark.skipif(not os.getenv("DB_CONNECTION_STRING"), reason="Requires DB_CONNECTION_STRING")
def test_decimal_separator_bug(db_connection):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job on adding this focused regression test!
Suggestion: Instead of creating a new file test_decimal_separator.py, could we add these tests to test_004_cursor.py?

There are already decimal separator tests there (around lines 8370-8730), so it would keep all the related tests together. Plus, these are really testing cursor fetch behavior with the decimal separator setting, which fits perfectly in that file.

"""
Test that fetchall() dealing with DECIMALS works correctly even when
setDecimalSeparator is set to something other than '.'
"""
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)")
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(".")
if cursor:
try:
cursor.execute("DROP TABLE #TestDecimal")
conn.commit()
except Exception:
pass
cursor.close()