Skip to content

Commit 87c345b

Browse files
authored
Use SQL_NUMERIC_STRUCT by default for DECIMAL/NUMERIC values (#1470)
POTENTIALLY BREAKING: this changes the default behavior for fetching DECIMAL and NUMERIC values for every connection on upgrade. DECIMAL/NUMERIC values are now fetched via the binary SQL_NUMERIC_STRUCT (SQL_C_NUMERIC) path by default, instead of being read as text and parsed -- the old path guessed the locale decimal separator and produced wrong values in some locales (#753). The binary path is locale-independent and also helps TVP numerics (#996). Opt out / restore the legacy string-based path per connection with: connection.fetch_decimal_as_string = True The string path is also used automatically as a fallback when the column scale is outside [0, 127] (e.g. PostgreSQL's negative-scale reporting) or when descriptor setup fails. Also fixes a missing-argument bug in the DecimalFromText fallback. Fixes #753. Helps #996. Note for release notes: call this out as potentially breaking, document the fetch_decimal_as_string bypass, and note the historical driver-compatibility caution around SQL_NUMERIC_STRUCT.
1 parent e8b69a2 commit 87c345b

11 files changed

Lines changed: 226 additions & 44 deletions

src/connection.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,7 @@ PyObject* Connection_New(PyObject* pConnectString, bool fAutoCommit, long timeou
309309
cnxn->compat_diagrec_byte_length = false;
310310

311311
cnxn->attrs_before = attrs_before_o.Detach();
312+
cnxn->fetch_decimal_as_string = false;
312313

313314
// This is an inefficient default, but should work all the time. When we are offered
314315
// single-byte text we don't actually know what the encoding is. For example, with SQL
@@ -1082,6 +1083,37 @@ static int Connection_settimeout(PyObject* self, PyObject* value, void* closure)
10821083
return 0;
10831084
}
10841085

1086+
static PyObject* Connection_getfetchdecimalasstring(PyObject* self, void* closure)
1087+
{
1088+
UNUSED(closure);
1089+
1090+
Connection* cnxn = Connection_Validate(self);
1091+
if (!cnxn)
1092+
return 0;
1093+
1094+
PyObject* result = cnxn->fetch_decimal_as_string ? Py_True : Py_False;
1095+
Py_INCREF(result);
1096+
return result;
1097+
}
1098+
1099+
static int Connection_setfetchdecimalasstring(PyObject* self, PyObject* value, void* closure)
1100+
{
1101+
UNUSED(closure);
1102+
1103+
Connection* cnxn = Connection_Validate(self);
1104+
if (!cnxn)
1105+
return -1;
1106+
1107+
if (value == 0)
1108+
{
1109+
PyErr_SetString(PyExc_TypeError, "Cannot delete the fetch_decimal_as_string attribute.");
1110+
return -1;
1111+
}
1112+
1113+
cnxn->fetch_decimal_as_string = PyObject_IsTrue(value);
1114+
return 0;
1115+
}
1116+
10851117
static PyObject* Connection_getcompat_diagrec_byte_length(PyObject* self, void* closure)
10861118
{
10871119
return PyBool_FromLong(((Connection*)self)->compat_diagrec_byte_length);
@@ -1502,6 +1534,10 @@ static PyGetSetDef Connection_getseters[] = {
15021534
{ "timeout", Connection_gettimeout, Connection_settimeout,
15031535
"The timeout in seconds, zero means no timeout.", 0 },
15041536
{ "maxwrite", Connection_getmaxwrite, Connection_setmaxwrite, "The maximum bytes to write before using SQLPutData.", 0 },
1537+
{ "fetch_decimal_as_string", Connection_getfetchdecimalasstring, Connection_setfetchdecimalasstring,
1538+
"If True, DECIMAL and NUMERIC values are fetched as strings using the legacy\n"
1539+
"locale-aware path. If False (the default), values are fetched using a binary\n"
1540+
"representation that is not affected by the locale.", 0 },
15051541
{ "compat_diagrec_byte_length", Connection_getcompat_diagrec_byte_length, Connection_setcompat_diagrec_byte_length,
15061542
"If True, the driver reports byte length instead of character length in SQLGetDiagRecW().", 0 },
15071543
{ 0 }

src/connection.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ struct Connection
3939
// to insert NULLs into binary columns.
4040
bool supports_describeparam;
4141

42+
// Set to true if the driver doesn't handle SQL_NUMERIC_STRUCT properly.
43+
bool fetch_decimal_as_string;
44+
4245
// The column size of datetime columns, obtained from SQLGetInfo(), used to determine the datetime precision.
4346
int datetime_precision;
4447

src/cursor.cpp

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,53 @@ bool InitColumnInfo(Cursor* cursor, SQLUSMALLINT iCol, ColumnInfo* pinfo)
543543
pinfo->is_unsigned = false;
544544
}
545545

546+
// For a NUMERIC column, determine how we will fetch the values.
547+
pinfo->scale = DecimalDigits;
548+
pinfo->use_decimal_binary = false;
549+
switch (pinfo->sql_type)
550+
{
551+
case SQL_DECIMAL:
552+
case SQL_NUMERIC:
553+
case SQL_DB2_DECFLOAT:
554+
555+
// It is puzzling that ODBC abandons support for scale values between 128 and 255
556+
// by using a signed byte so that it can support negative scale values, even though
557+
// the SQL standard only allows values between zero and precision. Not many databases
558+
// support negative scale. PostgreSQL is one, and its ODBC driver currently (version
559+
// 17.00.0007) reports that scale is 2046 for SELECT CAST('1234500' AS NUMERIC(10,-2))
560+
// which doesn't make much sense (to me, anyway). So I'm going to follow the lead of
561+
// the SQL standard and fall back on fetching the value from the driver as a string
562+
// when scale does not fall in the range 0..127.
563+
if (!cursor->cnxn->fetch_decimal_as_string && pinfo->scale >= 0 && pinfo->scale <= 127)
564+
{
565+
// Set up the ARD once for this column so SQLGetData fills a SQL_NUMERIC_STRUCT.
566+
// These settings persist for all rows on this statement handle.
567+
SQLHDESC hDesc = NULL;
568+
SQLRETURN descRet;
569+
570+
Py_BEGIN_ALLOW_THREADS
571+
descRet = SQLGetStmtAttr(cursor->hstmt, SQL_ATTR_APP_ROW_DESC, &hDesc, 0, NULL);
572+
Py_END_ALLOW_THREADS
573+
574+
if (SQL_SUCCEEDED(descRet))
575+
{
576+
SQLULEN precision = pinfo->column_size;
577+
SQLSMALLINT scale = pinfo->scale;
578+
SQLSMALLINT i = (SQLSMALLINT)iCol;
579+
580+
Py_BEGIN_ALLOW_THREADS
581+
// SQL_DESC_TYPE must be set first.
582+
descRet =
583+
SQLSetDescField(hDesc, i, SQL_DESC_TYPE, (void*)SQL_C_NUMERIC, 0) == SQL_SUCCESS &&
584+
SQLSetDescField(hDesc, i, SQL_DESC_PRECISION, (void*)precision, 0) == SQL_SUCCESS &&
585+
SQLSetDescField(hDesc, i, SQL_DESC_SCALE, (void*)scale, 0) == SQL_SUCCESS
586+
? SQL_SUCCESS : SQL_ERROR;
587+
Py_END_ALLOW_THREADS
588+
589+
pinfo->use_decimal_binary = SQL_SUCCEEDED(descRet);
590+
}
591+
}
592+
}
546593
return true;
547594
}
548595

src/cursor.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ struct ColumnInfo
3131
// of the integer types are the same size whether signed and unsigned, so we can allocate memory ahead of time
3232
// without knowing this. We use this during the fetch when converting to a Python integer or long.
3333
bool is_unsigned;
34+
35+
// For SQL_DECIMAL/SQL_NUMERIC columns: scale from SQLDescribeCol, and whether
36+
// the binary (SQL_C_NUMERIC) fetch path can be used for this column.
37+
// use_decimal_binary is false if scale is outside [0, 127] (doesn't fit in
38+
// SQL_NUMERIC_STRUCT.scale or is negative) or if the driver failed ARD setup.
39+
SQLSMALLINT scale;
40+
bool use_decimal_binary;
3441
};
3542

3643
struct ParamInfo
@@ -117,10 +124,10 @@ struct Cursor
117124

118125
// Parameter set array (used with executemany)
119126
unsigned char *paramArray;
120-
127+
121128
// Whether to use fast executemany with parameter arrays and other optimisations
122129
char fastexecmany;
123-
130+
124131
// The list of information for setinputsizes().
125132
PyObject *inputsizes;
126133

src/decimal.cpp

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,6 @@ bool SetDecimalPoint(PyObject* pNew)
109109
return true;
110110
}
111111

112-
113112
PyObject* DecimalFromText(const TextEnc& enc, const byte* pb, Py_ssize_t cb)
114113
{
115114
// Creates a Decimal object from a text buffer.
@@ -131,11 +130,53 @@ PyObject* DecimalFromText(const TextEnc& enc, const byte* pb, Py_ssize_t cb)
131130

132131
if (pLocaleDecimalEscaped)
133132
{
134-
Object c2(PyObject_CallFunctionObjArgs(re_sub, pLocaleDecimalEscaped, pDecimalPoint, 0));
133+
Object c2(PyObject_CallFunctionObjArgs(re_sub, pLocaleDecimalEscaped, pDecimalPoint, cleaned.Get(), 0));
135134
if (!c2)
136135
return 0;
137136
cleaned.Attach(c2.Detach());
138137
}
139138

140139
return PyObject_CallFunctionObjArgs(decimal, cleaned.Get(), 0);
141140
}
141+
142+
PyObject* DecimalFromNumericStruct(const SQL_NUMERIC_STRUCT& num)
143+
{
144+
// Avoid the problems introduced by locale-specific delimiters in numeric strings
145+
// by having the driver provide the value using a well-defined binary structure.
146+
147+
// Convert the little-endian (1) unsigned (0) magnitude to a Python int.
148+
PyObject* magnitude = _PyLong_FromByteArray(num.val, SQL_MAX_NUMERIC_LEN, 1, 0);
149+
if (!magnitude)
150+
return NULL;
151+
152+
// Convert magnitude to a tuple of decimal digits via str().
153+
Object magStr(PyObject_Str(magnitude));
154+
Py_DECREF(magnitude);
155+
if (!magStr)
156+
return NULL;
157+
158+
Py_ssize_t nDigits = PyUnicode_GET_LENGTH(magStr.Get());
159+
Object digitTuple(PyTuple_New(nDigits));
160+
if (!digitTuple)
161+
return NULL;
162+
163+
for (Py_ssize_t i = 0; i < nDigits; i++)
164+
{
165+
Py_UCS4 ch = PyUnicode_READ_CHAR(magStr.Get(), i);
166+
PyObject* digit = PyLong_FromLong((long)(ch - '0'));
167+
if (!digit)
168+
return NULL;
169+
PyTuple_SET_ITEM(digitTuple.Get(), i, digit);
170+
}
171+
172+
// Decimal sign: 0 = positive, 1 = negative (opposite of ODBC convention).
173+
int decimalSign = (num.sign == 1) ? 0 : 1;
174+
long exponent = -(long)num.scale;
175+
176+
// Decimal((sign, (d, d, ...), exponent))
177+
Object args(Py_BuildValue("((iOl))", decimalSign, digitTuple.Get(), exponent));
178+
if (!args)
179+
return NULL;
180+
181+
return PyObject_CallObject(decimal, args.Get());
182+
}

src/decimal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@ PyObject* GetDecimalPoint();
55
bool SetDecimalPoint(PyObject* pNew);
66

77
PyObject* DecimalFromText(const TextEnc& enc, const byte* pb, Py_ssize_t cb);
8+
PyObject* DecimalFromNumericStruct(const SQL_NUMERIC_STRUCT& num);

src/getdata.cpp

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -326,8 +326,27 @@ static PyObject* GetDataUser(Cursor* cur, Py_ssize_t iCol, PyObject* func)
326326
return result;
327327
}
328328

329+
static PyObject* GetDataDecimalBinary(Cursor* cur, Py_ssize_t iCol)
330+
{
331+
SQL_NUMERIC_STRUCT numStruct;
332+
SQLLEN cbFetched = 0;
333+
SQLRETURN ret;
334+
SQLUSMALLINT i = (SQLUSMALLINT)(iCol + 1);
335+
336+
Py_BEGIN_ALLOW_THREADS
337+
ret = SQLGetData(cur->hstmt, i, SQL_ARD_TYPE, &numStruct, sizeof(numStruct), &cbFetched);
338+
Py_END_ALLOW_THREADS
339+
340+
if (!SQL_SUCCEEDED(ret))
341+
return NULL; // no Python exception — caller falls back to string path
329342

330-
static PyObject* GetDataDecimal(Cursor* cur, Py_ssize_t iCol)
343+
if (cbFetched == SQL_NULL_DATA)
344+
Py_RETURN_NONE;
345+
346+
return DecimalFromNumericStruct(numStruct);
347+
}
348+
349+
static PyObject* GetDataDecimalString(Cursor* cur, Py_ssize_t iCol)
331350
{
332351
// The SQL_NUMERIC_STRUCT support is hopeless (SQL Server ignores scale on input parameters
333352
// and output columns, Oracle does something else weird, and many drivers don't support it
@@ -731,7 +750,16 @@ PyObject* GetData(Cursor* cur, Py_ssize_t iCol)
731750
case SQL_DECIMAL:
732751
case SQL_NUMERIC:
733752
case SQL_DB2_DECFLOAT:
734-
return GetDataDecimal(cur, iCol);
753+
if (cur->colinfos[iCol].use_decimal_binary)
754+
{
755+
PyObject* obj = GetDataDecimalBinary(cur, iCol);
756+
if (obj != NULL)
757+
return obj;
758+
if (PyErr_Occurred())
759+
return NULL;
760+
// SQLGetData failed without a Python exception — fall through.
761+
}
762+
return GetDataDecimalString(cur, iCol);
735763

736764
case SQL_BIT:
737765
return GetDataBit(cur, iCol);

src/pyodbc.pyi

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,17 @@ class Connection:
361361
"""Returns True if the connection is closed, False otherwise."""
362362
...
363363

364+
@property
365+
def fetch_decimal_as_string(self) -> bool:
366+
"""If True, DECIMAL and NUMERIC values will be fetched as strings using the
367+
legacy local-aware path. If False (the default), values are fetched using a
368+
binary representation that is not affected by the locale."""
369+
...
370+
371+
@fetch_decimal_as_string.setter
372+
def fetch_decimal_as_string(self, value: bool) -> None:
373+
...
374+
364375
@property
365376
def hdbc(self) -> ctypes.c_void_p | None:
366377
"""ODBC handle for the connection."""

tests/mysql_test.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,15 @@ def test_decimal(cursor: pyodbc.Cursor):
118118
('-10.0010', '19,4')
119119
]
120120

121-
for value, prec in tests:
122-
value = Decimal(value)
123-
cursor.execute("drop table if exists t1")
124-
cursor.execute(f"create table t1(c1 numeric({prec}))")
125-
cursor.execute("insert into t1 values (?)", value)
126-
v = cursor.execute("select c1 from t1").fetchone()[0]
127-
assert v == value
121+
for mode in (True, False):
122+
cursor.connection.fetch_decimal_as_string = mode
123+
for value, prec in tests:
124+
value = Decimal(value)
125+
cursor.execute("drop table if exists t1")
126+
cursor.execute(f"create table t1(c1 numeric({prec}))")
127+
cursor.execute("insert into t1 values (?)", value)
128+
v = cursor.execute("select c1 from t1").fetchone()[0]
129+
assert v == value
128130

129131

130132
def test_multiple_bindings(cursor: pyodbc.Cursor):

tests/postgresql_test.py

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -168,11 +168,13 @@ def test_decimal(cursor: pyodbc.Cursor):
168168
params = [Decimal(n) for n in "-1000.10 -1234.56 -1 0 1 1000.10 1234.56 100010 123456789.21".split()]
169169
params.append(None)
170170

171-
for param in params:
172-
cursor.execute("truncate table t1")
173-
cursor.execute("insert into t1 values (?)", param)
174-
result = cursor.execute("select col from t1").fetchval()
175-
assert result == param
171+
for mode in (True, False):
172+
cursor.connection.fetch_decimal_as_string = mode
173+
for param in params:
174+
cursor.execute("truncate table t1")
175+
cursor.execute("insert into t1 values (?)", param)
176+
result = cursor.execute("select col from t1").fetchval()
177+
assert result == param
176178

177179

178180
def test_numeric(cursor: pyodbc.Cursor):
@@ -182,11 +184,13 @@ def test_numeric(cursor: pyodbc.Cursor):
182184
params = [Decimal(n) for n in "-1234.56 -1 0 1 1234.56 123456789.21".split()]
183185
params.append(None)
184186

185-
for param in params:
186-
cursor.execute("truncate table t1")
187-
cursor.execute("insert into t1 values (?)", param)
188-
result = cursor.execute("select col from t1").fetchval()
189-
assert result == param
187+
for mode in (True, False):
188+
cursor.connection.fetch_decimal_as_string = mode
189+
for param in params:
190+
cursor.execute("truncate table t1")
191+
cursor.execute("insert into t1 values (?)", param)
192+
result = cursor.execute("select col from t1").fetchval()
193+
assert result == param
190194

191195

192196
def test_maxwrite(cursor: pyodbc.Cursor):

0 commit comments

Comments
 (0)