Django

Code

Ticket #2358 (closed: invalid)

Opened 2 years ago

Last modified 3 months ago

[patch] MS-SQL server fixes

Reported by: Dan Hristodorescu danh@borderfree.com Assigned to: nobody
Component: Database wrapper Version: SVN
Keywords: Cc: moof@metamoof.net
Triage Stage: Accepted Has patch: 1
Needs documentation: 0 Needs tests: 0
Patch needs improvement: 1

Description

I attached a patch to try to make django work with MS-SQL server. The patch is for these issues:

  • The backend didn't create the proper wrapper cursor to replace parameter placeholders to ?
  • MS-SQL doesn't support microseconds, so I added it like it was already in for MySQL. Since there are already two databases that need this I would suggest to refactor the code and add a flag in the backend drivers for this.
  • In django/db/models/base.py line 175: cursor.execute("SELECT 1 FROM %s WHERE %s=%%s LIMIT 1"

In SQL server should be SELECT TOP 1 instead of LIMIT although I don't understand why one would put a limit on an exact match on the primary key. It could only return one row anyway. I removed it but someone should check if it's ok.

Attachments

mssql.diff (3.1 kB) - added by Dan Hristodorescu on 07/16/06 00:11:48.
mssql_updated.diff (4.3 kB) - added by Dan Hristodorescu on 07/16/06 12:18:21.
updated
mssql_update1.diff (4.4 kB) - added by Dan Hristodorescu on 07/20/06 12:55:43.
forgot brackets and added microseconds check
mssql_update2.diff (12.0 kB) - added by sdelatorre+django@gmail.com on 09/07/06 18:21:25.
[patch]The patch mentioned in the last message.
mssql_update3.diff (12.4 kB) - added by sdelatorre+django@gmail.com on 09/28/06 20:01:07.
Added a patch for queries that use a datetime.datetime object. Because MSSQL can't parse the microseconds from a datetime object, the microseconds need to be set to 0 before being used in a query. Also re-ran the diff against the SVN trunk for all the changes made in this ticket.
mssql_update4.diff (13.4 kB) - added by sdelatorre+django@gmail.com on 10/11/06 10:00:05.
I made a slight modification to the django.db.backends.util file to work with custom mssql queries. In general, the CursorDebugWrapper? attempts to record the full SQL statement that was executed by performing a string substitution on the SQL statement with its query parameters. This doesn't work with mssql queries because they use a '?' for string substitution instead of the typical Python '%s'. The new changes introduced in this patch change all ‘?’ in mssql queries to ‘%s’ before the substitution is executed. The entire patch have also been updated against the latest SVN trunk.
mssql_update5.diff (14.0 kB) - added by sdelatorre+django@gmail.com on 10/16/06 14:26:13.
Added another fix to the util.py file. If '%' wildcards are used in a custom query, the CursorDebugWrapper?.execute function fails. This was patched by wrapping the string substituion in a try/except block to prevent the exception from halting execution.
mssql.patch (125.7 kB) - added by anonymous on 12/26/06 09:59:00.
mssql_update6.diff (13.7 kB) - added by sdelatorre+django@gmail.com on 02/04/07 10:26:46.
Added another CoInitialize?()/CoUninitialize() wrapper around the executeHelper function to fix related errors. Updated the diff against revision 4459.
mssql_update6a.diff (14.1 kB) - added by wycharon@gmail.com on 02/05/07 09:37:05.
modified backends/ado_mssq/base : 1. changes 'import adodbapi as Database' to 'import adodbapi.adodbapi as Database'. 2. in variantToPython,add explicitly DEFAULT_CHARSET translate for unicode
mssql_update6b.diff (14.2 kB) - added by wycharon@gmail.com on 02/05/07 09:47:58.
i am sorry. in mssql_update6a.diff. I forgot importing django.conf.settings
mssql_update7.diff (14.6 kB) - added by wycharon@gmail.com on 02/06/07 03:37:43.
modified function get_last_insert_id(). use IDENT_CURRENT(..) instead of @@IDENTITY. if someone deploy a sync insert trigger on the table, there is another insert sql after orignal insert,so @@IDENTITY is error prone
mssql_update8.diff (0.6 kB) - added by tclancy@gmail.com on 02/07/07 13:09:39.
Fixed typo in ado_mssql\base.py
mssql_update8a.diff (14.5 kB) - added by wycharon@gmail.com on 02/08/07 23:11:36.
full pacth for typo. i am sorry. it's my mistake for the typo. i will re upload the mssql_update7.diff
mssql_update9_NOT_WORKING.patch (18.4 kB) - added by moof@metamoof.net on 03/20/07 12:04:50.
Not working patch that resolves some issues towards getting the test suite running.
patch_9_test_output.txt (10.2 kB) - added by moof@metamoof.net on 03/20/07 12:07:04.
test output at --verbose=2 from running patch 9

Change History

07/16/06 00:11:48 changed by Dan Hristodorescu

  • attachment mssql.diff added.

07/16/06 12:18:21 changed by Dan Hristodorescu

  • attachment mssql_updated.diff added.

updated

07/16/06 12:23:32 changed by Dan Hristodorescu

Now it's working with MS-SQL but I had to remove the microseconds in session handling in django/contrib/sessions/middleware.py

I don't think it's a big deal in session expiration just being millisecond accurate. The datetime handling should be improved to handle these database differences.

07/20/06 12:55:43 changed by Dan Hristodorescu

  • attachment mssql_update1.diff added.

forgot brackets and added microseconds check

08/31/06 11:39:08 changed by sdelatorre+django@gmail.com

This patch worked out great for me. I completed the rest of the intropection.py functionality in patch #2563.

09/07/06 18:20:10 changed by sdelatorre+django@gmail.com

After going live with this patch, I ran into a frustrating error. I found that I would randomly get connection failures from the adodbapi lib. After further investigation, the error tunred out to be "com_error: 'CoInitialize has not been called.'". To remedy the situation, the pythoncom.CoInitialize?() and pythoncom.CoUninitialize?() functions need to be called before/after the database connection is established. I've attached a patch for this, and merged my changes from Ticket #2563 into this one since they target the same functionality.

09/07/06 18:21:25 changed by sdelatorre+django@gmail.com

  • attachment mssql_update2.diff added.

[patch]The patch mentioned in the last message.

09/28/06 20:01:07 changed by sdelatorre+django@gmail.com

  • attachment mssql_update3.diff added.

Added a patch for queries that use a datetime.datetime object. Because MSSQL can't parse the microseconds from a datetime object, the microseconds need to be set to 0 before being used in a query. Also re-ran the diff against the SVN trunk for all the changes made in this ticket.

10/07/06 12:27:32 changed by anonymous

Great job! But it does lack of pagination support, doesn't it? (get_limit_offset_sql() returns "".) I don't think there is any 'easy' sql statements that can manipulate the task well on SQL Server. Maybe using a server-side cursor would be a quick & dirty solution?

10/08/06 22:41:33 changed by sdelatorre+django@gmail.comj

But it does lack of pagination support, doesn't it? (get_limit_offset_sql() returns "".) I don't think there is any 'easy' sql statements that can manipulate the task well on SQL Server. Maybe using a server-side cursor would be a quick & dirty solution?

Yes, it still lacks pagination support. A server-side cursor does seem to be the best option right now, though I don't have any time to test it out at the moment.

10/11/06 10:00:05 changed by sdelatorre+django@gmail.com

  • attachment mssql_update4.diff added.

I made a slight modification to the django.db.backends.util file to work with custom mssql queries. In general, the CursorDebugWrapper? attempts to record the full SQL statement that was executed by performing a string substitution on the SQL statement with its query parameters. This doesn't work with mssql queries because they use a '?' for string substitution instead of the typical Python '%s'. The new changes introduced in this patch change all ‘?’ in mssql queries to ‘%s’ before the substitution is executed. The entire patch have also been updated against the latest SVN trunk.

10/16/06 14:26:13 changed by sdelatorre+django@gmail.com

  • attachment mssql_update5.diff added.

Added another fix to the util.py file. If '%' wildcards are used in a custom query, the CursorDebugWrapper?.execute function fails. This was patched by wrapping the string substituion in a try/except block to prevent the exception from halting execution.

12/26/06 09:59:00 changed by anonymous

  • attachment mssql.patch added.

01/22/07 03:09:16 changed by anonymous

02/04/07 10:26:46 changed by sdelatorre+django@gmail.com

  • attachment mssql_update6.diff added.

Added another CoInitialize?()/CoUninitialize() wrapper around the executeHelper function to fix related errors. Updated the diff against revision 4459.

02/05/07 00:09:34 changed by wycharon@gmail.com

there are two problems in mssql_update6: 1. in django.db.backends.ado_mssql,we should import adodbapi.adodbapi other than adodbapi, line10:

import adodbapi as Database

should be:

import adodbapi.adodbapi as Database

otherwise,any change to attrubites of Database has no effect 2. function variantToPython should handle charset translate.

in charset other than ascii,we should translate unicode to DEFAULT_CHARSET explicitly. As follows:

... from django.conf import settings ... if type(res) == unicode:

return isinstance(res,unicode) and res.encode(settings.DEFAULT_CHARSET) or res

02/05/07 09:17:32 changed by wycharon@gmail.com

i am sorry. the last two lines should be: if type(res) == unicode:return es.encode(settings.DEFAULT_CHARSET)

02/05/07 09:37:05 changed by wycharon@gmail.com

  • attachment mssql_update6a.diff added.

modified backends/ado_mssq/base : 1. changes 'import adodbapi as Database' to 'import adodbapi.adodbapi as Database'. 2. in variantToPython,add explicitly DEFAULT_CHARSET translate for unicode

02/05/07 09:47:58 changed by wycharon@gmail.com

  • attachment mssql_update6b.diff added.

i am sorry. in mssql_update6a.diff. I forgot importing django.conf.settings

02/06/07 03:37:43 changed by wycharon@gmail.com

  • attachment mssql_update7.diff added.

modified function get_last_insert_id(). use IDENT_CURRENT(..) instead of @@IDENTITY. if someone deploy a sync insert trigger on the table, there is another insert sql after orignal insert,so @@IDENTITY is error prone

02/07/07 13:09:39 changed by tclancy@gmail.com

  • attachment mssql_update8.diff added.

Fixed typo in ado_mssql\base.py

02/08/07 23:11:36 changed by wycharon@gmail.com

  • attachment mssql_update8a.diff added.

full pacth for typo. i am sorry. it's my mistake for the typo. i will re upload the mssql_update7.diff

02/22/07 01:13:48 changed by Simon G. <dev@simon.net.nz>

  • stage changed from Unreviewed to Accepted.

Reading the comments here, it looks like this is ready for checkin, but can someone with MS-SQL see if this passes all tests?

03/10/07 21:12:09 changed by moof@metamoof.net

  • needs_better_patch set to 1.

Not Quite There Yet. Still giving a bunch of errors here, which I'm slowly starting to fix. Inserting values for AutoFields? needs massaging, and a couple of other things.

I have a big beef with the flush commands, right now, as MSSQL doesn't cascade by default, so you have to explicitly ask for it on table creation. I'm also running into a weird bug involving missing references on database creation.

I'll post up a patch when I manage to fix that.

Though this might have something to do with the thread bullying that's been done in the patch I downloaded, as it looks like a classic race condition to me.

03/20/07 12:03:58 changed by moof@metamoof.net

  • cc set to moof@metamoof.net.

OK, so far I haven't managed to run the test suite, but I offer my improvements on the patch that get it a few steps further into running it. Specifically, I've got AutoField? insertion working OK, and got the flush command sort of working. I've also updated it to run with the latest svn HEAD - #4757

This Patch does not fully work

I have come into a problem whereby the standard test suite tries to create a database and fails. I attach test_output.txt run at --verbose=3 so people can see what I mean, I can't create test databases. Specifically, for some reason the application is attempting to create comments_karmascore before comments_comment, which sounds like a race condition to me, possibly at the driver level or database level, as the output shows that the tables creation command is issued. Inspecting the database after the run shows that none of the comment tables are created.

I don't know enough about Django internals to know quite how threading is handled in this matter, or transactions. I'll try asking on the mailing list, but I attach this patch in case anyone else wishes to give it a try.

For the record, I'm using SQL Server 2005 Express Edition.

03/20/07 12:04:50 changed by moof@metamoof.net

  • attachment mssql_update9_NOT_WORKING.patch added.

Not working patch that resolves some issues towards getting the test suite running.

03/20/07 12:07:04 changed by moof@metamoof.net

  • attachment patch_9_test_output.txt added.

test output at --verbose=2 from running patch 9

04/03/07 11:27:16 changed by anonymous

When will all this be ready and usable?

04/03/07 12:18:27 changed by ubernostrum

Please do not ask "when will this be finished" or "when will this merge" questions in the ticket tracker; such discussion is more appropriate for the django-developers list, and having it there keeps noise down in the ticket system.

04/20/07 05:04:25 changed by cwt@bashell.com

I found that datetime and smalldatetime are not converted correctly. I try to change both in creation.py I got this error messages while create django admin user (syncdb is OK)...

com_error: (-2147352567, 'Exception occurred.', (0, 'Microsoft OLE DB Provider

for SQL Server', 'Syntax error converting character string to smalldatetime data

type.', None, 0, -2147217913), None)

I use django from SVN and adodbpai 2.0.1 from http://sourceforge.net/projects/adodbapi It seem like there are some bugs in adodbapi. But I'm not so sure.

Hope this can help.

02/27/08 19:18:43 changed by jacob

  • status changed from new to closed.
  • resolution set to invalid.

This is too "big" a feature for a ticket. The best thing to do with this is to create an external project with the mssql backend (see http://www.djangoproject.com/documentation/settings/#database-engine). Ask on django-dev if you need help doing this.


Add/Change #2358 ([patch] MS-SQL server fixes)




Change Properties
Action