mkleehammer / pyodbc

Python ODBC bridge
https://github.com/mkleehammer/pyodbc/wiki
MIT No Attribution
2.95k stars 563 forks source link

Regression from 4.0.35 to 4.0.39 - fetch fails for decimal fields when system locale decimal separator is not '.' #1236

Closed bitranox closed 1 year ago

bitranox commented 1 year ago

Issue

regression from pyodbc: 4.0.35 to 4.0.39

on 4.0.39 fetching a row from a table fails, if the field type is decimal on 4.0.35 it works just fine

Pseudocode:

    cursor.execute("select testfield_ from testtable where Nr = ?", ['42'])
    cursor.fetchone()
    Traceback (most recent call last):
    TypeError: sub() missing 1 required positional argument: 'string'
gordthompson commented 1 year ago

I am unable to reproduce your issue with 64-bit Python on Linux hitting SQL Server:

import pyodbc

print(pyodbc.version)  # 4.0.39

connection_string = (
    "DRIVER=ODBC Driver 18 for SQL Server;"
    "SERVER=192.168.0.199;"
    "UID=scott;PWD=tiger^5HHH;"
    "DATABASE=test;"
    "TrustServerCertificate=yes;"
)
cnxn = pyodbc.connect(connection_string, autocommit=True)
crsr = cnxn.cursor()

crsr.execute("DROP TABLE IF EXISTS testtable")
crsr.execute("CREATE TABLE testtable (Nr varchar(2), testfield_ varchar(10))")
crsr.execute("INSERT INTO testtable (Nr, testfield_) VALUES ('42', 'foo')")

crsr.execute("SELECT testfield_ FROM testtable WHERE Nr = ?", ["42"])
result = crsr.fetchone()
print(result)  # ('foo',)
bitranox commented 1 year ago

dear @gordthompson, the reason seems NOT to be the underscore - it is the only field which has type "decimal" I use a very old CODBC driver for Navision Financials, which I can not change (there is no newer version).

I can say , the same code works with 4.0.45 and with 4.0.39 I get TypeError: sub() missing 1 required positional argument: 'string' as soons as I fetchone or fetchall.

If I exclude the particular field, its working fine

the type is "decimal", the result with 4.0.35 is : 'kg__Gesamt_': Decimal('5300000')

the original value in the database is : '53,00' the original column name in the database is : kg_(Gesamt) so it seems there is happening some translation in the CODBC Driver, and the comma got lost somehow(?). it also might be, that the navision database internally stores decimal values as integers, to avoid rounding mistakes. so all that might be some hints to find the different behaviour all other (text)fields are working correctly,

v-chojas commented 1 year ago

Can you post an ODBC trace of the working and non-working case?

bitranox commented 1 year ago

sure I will try to make a trace. I guess the error is somewhere around here:

 Object cleaned(PyObject_CallMethod(pRegExpRemove, "sub", "sO", "", text.Get()));
    if (!cleaned)
        return 0;

    if (pLocaleDecimalEscaped)
    {
        Object c2(PyObject_CallFunctionObjArgs(re_sub, pLocaleDecimalEscaped, pDecimalPoint, 0));
        if (!c2)
            return 0;
        cleaned.Attach(c2.Detach());
    }
bitranox commented 1 year ago

sorry, the odbc trace does not work, no matter what I set - there is no logfile. it is a system-dsn and I set all computers and C:\temp\odbc32.log - no luck. There is simply no logfile created. any other way to help to debug it ?

gordthompson commented 1 year ago

You followed the instructions here?

https://github.com/mkleehammer/pyodbc/wiki/Troubleshooting-%E2%80%93-Generating-an-ODBC-trace-log#windows

bitranox commented 1 year ago

yes. 32 Bit ODBC Admininstration - to the point. No Luck. I try now to add my own output converter for decimal - lets see .... fingers crossed

gordthompson commented 1 year ago

possibly related: https://github.com/mkleehammer/pyodbc/issues/753

bitranox commented 1 year ago

I used a simple outpout converter:

    def handle_decimal(dto_value):
        return str(dto_value)

    connection.add_output_converter(pyodbc.SQL_DECIMAL, handle_decimal)

and got (for Version 4.0.35): {'kg_Gesamt': "b'53.00000'"} maybe Your regexp can not handle the bytes?

bitranox commented 1 year ago

ok, with 4.0.39 I also get {'kg__Gesamt_': "b'53.00000'"} with the converter mentioned above. so there is some hickup in the decimal conversion ? Or is it my poor COCBC Driver ? Which way You would recommend, should I put my own converter ? And how would that work for writing into the database ?

gordthompson commented 1 year ago

I am able to reproduce the issue.

Windows 8.1, English (US)
Python 3.10.9, 64 bit
msodbcsql 18.02.0001

repro code:

import pyodbc

from util import get_connection_string

connection_string = get_connection_string()
print(connection_string)
cnxn = pyodbc.connect(
    connection_string,
    autocommit=True,
)
crsr = cnxn.cursor()

pyodbc.setDecimalSeparator(",")

result = crsr.execute("SELECT CAST('3.1416' AS decimal(10, 4)) AS foo").fetchval()
print(repr(result))

With pyodbc 4.0.35, I get

Decimal('31416')

as noted in #753 .

With pyodbc 4.0.39, I get

Traceback (most recent call last):
  File "C:\Users\Gord\PycharmProjects\pyodbc_demo\main.py", line 35, in <module>
    result = crsr.execute("SELECT CAST('3.1416' AS numeric(10, 4)) AS foo").fetchval()
TypeError: sub() missing 1 required positional argument: 'string'
v-chojas commented 1 year ago

Seems like another regression due to https://github.com/mkleehammer/pyodbc/commit/6b107a2bcaf7379e5ba182007b6ecae1bc2fc931

gordthompson commented 1 year ago

@bitranox - Given my example above, see if

pyodbc.setDecimalSeparator(".")

avoids the issue for you.

bitranox commented 1 year ago

@gordthompson , thank You very much, that works

anyway, a better error message would be good.

You might Introduce : by default the rightmost "." or "," should be considered as decimal separator. if the rightmost "." or "," exists more then once in the string, it is the thousands separator this automatic works 99% of the time as expected - for special cases You can still set the decimal separator manually

if "." is detected or set as decimal separator, all "," needs to be deleted before converted to Decimal if "," is detected or set as decimal separator, all "." needs to be deleted before converted to Decimal

tests should be written, covering all those decimal formats, testing all the paths through the code and meaningful error messages.

pyodbc.setDecimalSeparator("auto")      # (default) the rightmost "." or "," is considered as decimal point
pyodbc.setDecimalSeparator(".")            # "." is manually set as decimal separator,  "," are removed and ignored
pyodbc.setDecimalSeparator(",")            # "," is manually set as decimal separator,  "." are removed and ignored

# and introduce meaningful errors
bitranox commented 1 year ago

@gordthompson , thanks again, I updated my proposal for future handling. i hope it can be useful. Thank You very much and greetings from Vienna, Austria Robert Nowotny, aka bitranox