ActianCorp / sqlalchemy-ingres

Python SQLAlchemy dialect for Actian databases; Actian Data Platform (nee Avalanche), Actian X, Ingres, and Vector SQL - II-5734
https://pypi.org/project/sqlalchemy-ingres/
Apache License 2.0
4 stars 4 forks source link

Hard coded character encoding/transliteration taking place, ignoring II_CHARSETxx (etc.) #36

Open clach04 opened 8 months ago

clach04 commented 8 months ago

For example see:

https://github.com/ActianCorp/sqlalchemy-ingres/blob/89119aff4126b5d3d377cb95c2e609cdc5fe1f0b/lib/sqlalchemy_ingres/base.py#L637

https://github.com/ActianCorp/sqlalchemy-ingres/blob/89119aff4126b5d3d377cb95c2e609cdc5fe1f0b/lib/sqlalchemy_ingres/base.py#L644

clach04 commented 7 months ago

@hab6 additional notes, that maybe overkill but hopefully avoid confusion.

Initial idea / recommendations:

  1. remove current code
  2. then test with BOTH Python 2.7 and Python 3.x with pyodbc/pypyodbc (ignoring the other connectors)
  3. If needed add appropriate encode()/decode() (possibly with if_py3 style code) - again focusing on the ODBC connector case
hab6 commented 7 months ago

@clach04 Thanks for the suggestions.

To start off, I ran the SQLAlchemy test suite with the Ingres dialect methods normalize_name and denormalize_name both enabled (using Ingres dialect version 0.0.7.dev1) versus disabled (by changing those methods to simply return unaltered what was passed in).

Environment

(Both client and DBMS exist on the same machine)

Microsoft Windows [Version 10.0.19045.3930] 
Python 3.10.7
DBMS: Ingres 11.2.0 15807

mock              5.1.0
pyodbc            5.1.0
pypyodbc          1.3.6
pytest            8.1.1
setuptools        63.2.0
SQLAlchemy        2.0.28.dev0
sqlalchemy-ingres 0.0.7.dev1
tox               4.14.1

Command used to execute the SQLAlchemy test suite

pytest --db ingres_odbc --maxfail=15000 --ignore=test\dialect --ignore=test\orm\test_versioning.py --tb=no

Differences between the methods enabled vs disabled

Test Failed Passed Skipped Warnings Errors Run Time
Enabled 1914 12381 2037 7 2238 22m 39s
Disabled 1915 12438 2037 7 2161 24m 47s
Difference +1 +57 same same -77 2m 8s

Summary: By disabling the dialect methods normalize_name and denormalize_name, the SQLAlchemy test suite ended with 57 more passing tests and 77 fewer errors.

I will run a few more tests with different environments and post the results.

hab6 commented 7 months ago

More test results, this time using Ubuntu systems for the client and server. The pytest command to execute the test suite was the same one used in the prior test on the Windows system.

Unexpectedly, these results are way different (much worse) than running the test on Windows. For example, the tests on Ubuntu with VW encountered around 5K more errors than the results on the Windows system. This probably will merit opening a separate issue to investigate the huge disparity of results.

Client: Ubuntu 22.04.2 LTS
DBMS: VectorWise 6.3.0 14608 on Ubuntu 20.04.6 LTS

Python 3.10.12

mock              5.1.0
pluggy            1.4.0
pyodbc            5.1.0
pypyodbc          1.3.6
pytest            8.1.1
setuptools        69.2.0
SQLAlchemy        2.0.28.dev0
sqlalchemy-ingres 0.0.7.dev1
tox               4.14.1

Results from running the SQLAlchemy test suite with the Ingres dialect methods normalize_name and denormalize_name enabled versus disabled.

Test Failed Passed Skipped Warnings Errors Run Time
Enabled 1026 9284 1737 8 7002 28m 32s
Disabled 996 9253 1738 9 7068 29m 8s
Difference -30 -31 +1 +1 +66 +36s
clach04 commented 7 months ago

Change looks good, sounds like next decision is whether to retain or delete these functions now.