Opened 12 months ago

Closed 12 months ago

Last modified 12 months ago

#22309 closed Bug (needsinfo)

DatabaseFeatures.supports_transactions should use temporary tables

Reported by: lucastan@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I am using South 0.8.4 (refer to source https://bitbucket.org/andrewgodwin/south/src/c26229113db6bbdebf36e6b31d76b45a48e29340/south/db/generic.py?at=0.8.4#cl-124)

The file south/db/generic.py has this function has_ddl_transactions which access django.db.connection.features.supports_transactions

However, upon accessing, it gives an error. The error could be reproduced by the following:

./manage.py shell

>>> from django.db import connection
>>> connection.features.supports_transactions

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/home/xx/venv/local/lib/python2.7/site-packages/django/utils/functional.py", line 49, in __get__
    res = instance.__dict__[self.func.__name__] = self.func(instance)
  File "/home/xx/venv/local/lib/python2.7/site-packages/django/db/backends/__init__.py", line 664, in supports_transactions
    self.connection.leave_transaction_management()
  File "/home/xx/venv/local/lib/python2.7/site-packages/django/db/backends/__init__.py", line 315, in leave_transaction_management
    "Transaction managed block ended with pending COMMIT/ROLLBACK")
TransactionManagementError: Transaction managed block ended with pending COMMIT/ROLLBACK



Using south 0.8.4 and django 1.6. Seems like a Django error? But in the first place, is it safe to access features attribute?

Change History (7)

comment:1 Changed 12 months ago by anonymous

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I want to point that for SQLite backend, there is no error. But for MySQL, there is an error.

comment:2 Changed 12 months ago by timo

I cannot reproduce this using the steps provided and connecting to a MySQL database. Perhaps you could provide a minimal project to reproduce?

comment:3 Changed 12 months ago by timo

  • Resolution set to needsinfo
  • Status changed from new to closed

comment:4 Changed 12 months ago by anonymous

Ok, I have resolved this issue. The whole problem is that the supports_Transactions method actually creates a table and drop it to test transaction. Due to the privileges, it couldn't drop a table , resulting in leave_transaction_management throwing an error due to an incomplete transaction.

So my question is: is the supports_Transactions method safe for use in a multi-process django app, since multiple threads could create and drop the same table concurrently? I suggest to create a unique name for the table based on UUID or some sort.

comment:5 Changed 12 months ago by akaariai

We should use temporary tables. They are connection local, and in any case using temporary table is a lot better than using normal tables. Temporary tables are supported from MySQL 3.23 onwards, so See http://dev.mysql.com/doc/refman/4.1/en/create-table.html. The only question is if temporary tables have different transaction rules than normal tables. I don't see anything indicating that this is the case, but then again when dealing with MySQL it wouldn't be a surprise if this is the case.

So, needed here:

1) check that temp tables work like normal tables when it comes to transaction management. My understanding is that the default engine in use for MySQL affects if transactions are usable, so the check would be to switch to using MyISAM tables as default and then verify that temp table gives the same result as normal tables.
2) create temp table doesn't commit the current transaction - create table does. This might change the behavior of the method.
3) check that this doesn't affect other backends (sqlite seems to use the supports_transactions method, too)
4) write a patch

Also, the supports_transactions method seems scary. If ran inside transaction the end result is silent rollback + switch to autocommit mode. So, step 5) is to verify when exactly this method is ran, and also make the method assert that no transaction is open when it is ran. If it is possible that this method will get ran inside transaction and when not testing then we should likely backpatch the fix, too, as this is a clear data loss issue in that case.

comment:6 Changed 12 months ago by akaariai

  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

comment:7 Changed 12 months ago by timo

  • Summary changed from error accessing django.db.connection.features? to DatabaseFeatures.supports_transactions should use temporary tables
Note: See TracTickets for help on using tickets.
Back to Top