lionheart / django-pyodbc

An ODBC-powered MS SQL Server DB backend for Django 1.4+
Apache License 2.0
203 stars 103 forks source link

DatabaseWrapper's get_new_connection does not take OPTIONS into account #86

Open jorisroovers opened 9 years ago

jorisroovers commented 9 years ago

When using test fixtures in combination with FreeTDS, I keep running into the following error:

DatabaseError: ('42000', '[42000] [FreeTDS][SQL Server]Unicode data in a Unicode-only collation or ntext data cannot be sent to clients using DB-Library (such as ISQL) or ODBC version 3.7 or earlier. (4004) (SQLExecDirectW)'

For the most part, the key to solving this issue is specifying TDS_VERSION=8.0 as part of the extra_params in the OPTIONS key of Django's DATABASES setting.

DATABASES = {
    'default': {
        'ENGINE': 'django_pyodbc',
        'NAME': "MyDB",
        'HOST': "myserver",
        'USER': "user",
        'PASSWORD': "password",
        'PORT': "1433",
        'OPTIONS': {
            'driver': 'FreeTDS',
            'host_is_server': True,
            'extra_params': "TDS_VERSION=8.0"
        }
    }
}

However, this problem keeps occurring when running unit tests that make use of test fixtures, even though it doesn't occur when regular users are hitting the code in question.

After spending multiple hours on this, I figured out that the django test runners call the get_new_connection() method of the DatabaseWrapper class in django_pyodbc's base.py module. https://github.com/lionheart/django-pyodbc/blob/master/django_pyodbc/base.py#L182

This method is not called when using regular code, instead the DB connection is created during the first call to _cursor(): https://github.com/lionheart/django-pyodbc/blob/master/django_pyodbc/base.py#L267

I believe the issue here is that get_new_connection() doesn't specify the connectionstring to pyodbc as is done in _cursor(): https://github.com/lionheart/django-pyodbc/blob/master/django_pyodbc/base.py#L278

A workaround is to explicitly specify the connectionstring as follows:

DATABASES = {
    'default': {
        'ENGINE': 'django_pyodbc',
        'NAME': "MyDB",
        'HOST': "myserver",
        'USER': "user",
        'PASSWORD': "password",
        'PORT': "1433",
        'OPTIONS': {
            'connectionstring': "DRIVER={FreeTDS};SERVER=myserver;PORT=1433;UID=user;PWD=password;DATABASE=MyDB;TDS_VERSION=8.0",
            'driver': 'FreeTDS',
            'host_is_server': True,
            'extra_params': "TDS_VERSION=8.0"
        }
    }
}

However, this should be fixed in get_new_connection() itself. I believe the fix is as straightforward as calling _get_connection_string() and passing it to Database.connect() as its first argument. I'd be happy to submit a pull request if someone can confirm this and the repo owner is accepting fixes.

vegitron commented 9 years ago

I have a similar issue - looking at my trace file, the DB connection from get_new_connection has a connection string of [length = 170 (SQL_NTS)], while the DB connection from _cursor is [length = 134]

I'm putting together a pull request for this now, but I'm not sure how to unittest it.