setDecimalSeparator() causes DECIMAL values to become None

[bewithgaurav]

Describe the bug

When using setDecimalSeparator() to change the decimal separator from the default . (period) to any other character (e.g., , for European locale), DECIMAL values retrieved via bulk fetch operations (fetchall(), fetchmany() with large result sets) become None, resulting in data loss.

Critical Finding: The bug only affects the bulk/columnar fetch path in the C++ layer. Single-row fetches via fetchone() work correctly because they use a different code path that doesn't apply the separator during parsing.

The decimal separator setting is intended to affect display formatting only (in Row.__str__()), but the bulk fetch implementation incorrectly applies it during parsing of numeric values from the database.

Exception/Error:
No exception is raised. DECIMAL values silently become None in bulk fetch operations.

Expected: Decimal('1234.56')
Got:      None

To reproduce

Bug appears ONLY with bulk fetch operations (fetchall(), fetchmany()). Single-row fetchone() works correctly.

Complete reproduction code:

from mssql_python import connect, setDecimalSeparator, getDecimalSeparator
import os
from decimal import Decimal

conn_str = os.getenv("DB_CONNECTION_STRING")
conn = connect(conn_str)
cursor = conn.cursor()

# Create temp table with multiple rows
cursor.execute("DROP TABLE IF EXISTS #test_decimal")
cursor.execute("""
    CREATE TABLE #test_decimal (
        id INT,
        price DECIMAL(10, 2)
    )
""")

# Insert test data
for i in range(10):
    cursor.execute(f"INSERT INTO #test_decimal VALUES ({i}, {1234.56 + i})")

# Test 1: Bulk fetch with default separator works
print("Test 1: Bulk fetch with default separator")
setDecimalSeparator('.')
cursor.execute("SELECT id, price FROM #test_decimal ORDER BY id")
rows = cursor.fetchall()
print(f"Fetched {len(rows)} rows")
print(f"First row: {rows[0][1]}")  # Output: Decimal('1234.56') ✅
print(f"Type: {type(rows[0][1])}")

# Test 2: Bulk fetch with comma separator - BUG!
print("\nTest 2: Bulk fetch with comma separator")
setDecimalSeparator(',')
cursor.execute("SELECT id, price FROM #test_decimal ORDER BY id")
rows = cursor.fetchall()
print(f"Fetched {len(rows)} rows")
print(f"First row: {rows[0][1]}")  # Output: None ❌ BUG!
print(f"Type: {type(rows[0][1])}")
print(f"Expected: Decimal('1234.56')")
print(f"Got: {rows[0][1]}")

# Test 3: fetchone() works correctly (different code path)
print("\nTest 3: fetchone() with comma separator (works!)")
setDecimalSeparator(',')
cursor.execute("SELECT CAST(9876.54 AS DECIMAL(10, 2)) AS price")
row = cursor.fetchone()
print(f"Result: {row[0]}")  # Output: Decimal('9876.54') ✅
print(f"Type: {type(row[0])}")

# Cleanup
cursor.execute("DROP TABLE #test_decimal")
cursor.close()
conn.close()

Output:

Test 1: Bulk fetch with default separator
Fetched 10 rows
First row: 1234.56
Type: <class 'decimal.Decimal'>

Test 2: Bulk fetch with comma separator
Fetched 10 rows
First row: None
Type: <class 'NoneType'>
Expected: Decimal('1234.56')
Got: None

Test 3: fetchone() with comma separator (works!)
Result: 9876.54
Type: <class 'decimal.Decimal'>

Expected behavior

1. Data Retrieval: DECIMAL values should always be parsed correctly from the database, regardless of the decimal separator setting. The database returns strings like "1234.56" (with period), and parsing should always use . as the decimal separator.

2. Display Only: The setDecimalSeparator() setting should only affect the display format when converting Row objects to strings via __str__():

   setDecimalSeparator(',')
   row = cursor.fetchone()
   print(row[0])      # Should print: Decimal('1234.56') - actual value
   print(str(row))    # Should print: (1234,56) - formatted display

Further technical details

Python version: 3.11.4 (tested), likely affects all versions
SQL Server version: SQL Server 2022, Azure SQL Database (all versions affected)
Operating system: macOS 14.7.1, Windows 11 (cross-platform issue)
mssql-python version: 0.13.0

Root Cause Analysis:

The bug is in mssql_python/pybind/ddbc_bindings.cpp at line 3243-3253 in the bulk/columnar fetch path. This code path is used by fetchall() and fetchmany() for optimized batch fetching.

Buggy Code (lines 3235-3260):

case SQL_DECIMAL:
case SQL_NUMERIC: {
    try {
        std::string numStr(reinterpret_cast<const char*>(
            &buffers.charBuffers[col - 1][i * MAX_DIGITS_IN_NUMERIC]),
            buffers.indicators[col - 1][i]);
        
        // BUG: Applies custom separator BEFORE parsing
        std::string separator = GetDecimalSeparator();
        
        if (separator != ".") {
            // Replaces '.' with custom separator (e.g., ',')
            size_t pos = numStr.find('.');
            if (pos != std::string::npos) {
                numStr.replace(pos, 1, separator);  // "1234.56" → "1234,56"
            }
        }
        
        // BUG: Passes invalid string to Python's Decimal()
        // Python's Decimal("1234,56") throws exception → caught → returns None
        row.append(py::module_::import("decimal").attr("Decimal")(numStr));
    } catch (const py::error_already_set& e) {
        LOG("Error converting to decimal: {}", e.what());
        row.append(py::none());  // Silently returns None on parse error
    }
    break;
}

Why fetchone() works: The non-bulk fetch path (lines 2616-2670) doesn't apply the separator during parsing - it passes the string directly to Decimal() without modification.

Files Affected:

• mssql_python/pybind/ddbc_bindings.cpp lines 3243-3253 (bulk fetch - BUGGY)
• mssql_python/pybind/ddbc_bindings.cpp lines 2616-2670 (single fetch - CORRECT)
• mssql_python/row.py lines 152-169 (display formatting - CORRECT)

Additional context

This is a critical bug as it causes silent data loss. Users setting the decimal separator for European locales (,) will receive None values instead of actual DECIMAL data when using fetchall() or fetchmany(), potentially causing calculation errors or application failures.

Workaround:
Only use the default . separator (do not call setDecimalSeparator()), or use fetchone() exclusively (not practical for large result sets).

---

Proposed Solution

Fix the bulk fetch path to match the non-bulk fetch path behavior:

Changes Required in ddbc_bindings.cpp

Location: Lines 3235-3260 (bulk fetch path)

Current buggy code:

case SQL_DECIMAL:
case SQL_NUMERIC: {
    try {
        std::string numStr(reinterpret_cast<const char*>(
            &buffers.charBuffers[col - 1][i * MAX_DIGITS_IN_NUMERIC]),
            buffers.indicators[col - 1][i]);
        
        // BUG: Don't modify string before parsing!
        std::string separator = GetDecimalSeparator();
        if (separator != ".") {
            size_t pos = numStr.find('.');
            if (pos != std::string::npos) {
                numStr.replace(pos, 1, separator);
            }
        }
        
        row.append(py::module_::import("decimal").attr("Decimal")(numStr));
    } catch (const py::error_already_set& e) {
        LOG("Error converting to decimal: {}", e.what());
        row.append(py::none());
    }
    break;
}

Fixed code (remove separator replacement):

case SQL_DECIMAL:
case SQL_NUMERIC: {
    try {
        std::string numStr(reinterpret_cast<const char*>(
            &buffers.charBuffers[col - 1][i * MAX_DIGITS_IN_NUMERIC]),
            buffers.indicators[col - 1][i]);
        
        // FIX: Always parse with '.' separator (database format)
        // Custom separator only affects display in Row.__str__()
        row.append(py::module_::import("decimal").attr("Decimal")(numStr));
    } catch (const py::error_already_set& e) {
        LOG("Error converting to decimal: {}", e.what());
        row.append(py::none());
    }
    break;
}

Implementation Steps

1. Remove lines 3243-3253 in ddbc_bindings.cpp (the separator replacement logic)
2. Keep the display formatting in row.py (lines 152-169) - already correct
3. Add test cases for bulk fetch with custom decimal separators
4. Verify that both fetchone() and fetchall() work consistently

Expected Behavior After Fix

setDecimalSeparator(',')
cursor.execute("SELECT CAST(1234.56 AS DECIMAL(10,2))")

# Data retrieval: Always returns proper Decimal object
rows = cursor.fetchall()
print(rows[0][0])          # Decimal('1234.56') ✅
print(type(rows[0][0]))    # <class 'decimal.Decimal'> ✅

# Display formatting: Uses custom separator
print(str(rows[0]))        # (1234,56) ✅ - display only

Testing Checklist

• fetchone() with default separator (already works)
• fetchone() with custom separator (already works)
• fetchall() with default separator (already works)
• fetchall() with custom separator (currently broken, will fix)
• fetchmany() with custom separator (currently broken, will fix)
• Row.__str__() display formatting with custom separator (already works)
• Cross-platform testing (Windows, Linux, macOS)