Opened 8 years ago

Closed 8 years ago

#26541 closed Cleanup/optimization (fixed)

Add a DatabaseFeatures.supports_transactions() method for MySQL

Reported by: Marcin Nowak Owned by: Mariusz Felisiak
Component: Database layer (models, ORM) Version: 1.8
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When something goes wrong during checking database features, the state of db stays affected:

  File "/home/marcin/myproject/eggs/Django-1.8.6-py2.7.egg/django/test/testcases.py", line 182, in __call__
    self._pre_setup()
  File "/home/marcin/myproject/eggs/Django-1.8.6-py2.7.egg/django/test/testcases.py", line 787, in _pre_setup
    self._fixture_setup()
  File "/home/marcin/myproject/eggs/Django-1.8.6-py2.7.egg/django/test/testcases.py", line 986, in _fixture_setup
    if not connections_support_transactions():
  File "/home/marcin/myproject/eggs/Django-1.8.6-py2.7.egg/django/test/testcases.py", line 911, in connections_support_transactions
    for conn in connections.all())
  File "/home/marcin/myproject/eggs/Django-1.8.6-py2.7.egg/django/test/testcases.py", line 911, in <genexpr>
    for conn in connections.all())
  File "/home/marcin/myproject/eggs/Django-1.8.6-py2.7.egg/django/utils/functional.py", line 59, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
  File "/home/marcin/myproject/eggs/Django-1.8.6-py2.7.egg/django/db/backends/base/features.py", line 217, in supports_transactions
    cursor.execute('CREATE TABLE ROLLBACK_TEST (X INT)')
  File "/home/marcin/myproject/eggs/Django-1.8.6-py2.7.egg/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/home/marcin/myproject/eggs/Django-1.8.6-py2.7.egg/django/db/utils.py", line 97, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/home/marcin/myproject/eggs/Django-1.8.6-py2.7.egg/django/db/backends/utils.py", line 62, in execute
    return self.cursor.execute(sql)
  File "/home/marcin/myproject/eggs/Django-1.8.6-py2.7.egg/django/db/backends/mysql/base.py", line 124, in execute
    return self.cursor.execute(query, args)
  File "/home/marcin/myproject/eggs/MySQL_python-1.2.3-py2.7-linux-x86_64.egg/MySQLdb/cursors.py", line 174, in execute
    self.errorhandler(self, exc, value)
  File "/home/marcin/myproject/eggs/MySQL_python-1.2.3-py2.7-linux-x86_64.egg/MySQLdb/connections.py", line 36, in defaulterrorhandler
    raise errorclass, errorvalue
OperationalError: (1050, "Table 'ROLLBACK_TEST' already exists")
  1. Django should not create any table during startup - there may be readonly or sensitive databases connected
  2. Let the Django do some cleanups, if it must "dig" databases.

Change History (7)

comment:1 by Tim Graham, 8 years ago

Summary: Table 'ROLLBACK_TEST' already existsAdd a DatabaseFeatures.supports_transactions() method for MySQL
Type: BugCleanup/optimization

For Django's built in database backends, this issues seems to be limited to MySQL as the other built in database backend set DatabaseFeatures.supports_transactions = True and override the default implementation which does the feature test involving the temporary table indicated in the traceback. Is it feasible to add a DatabaseFeatures.supports_transactions() method for the MySQL backend that detects transaction support by examining the storage engine?

comment:2 by Marcin Nowak, 8 years ago

Tim, what about transaction handling where MyISAM engine is set per table?

Checking transaction handling by table creation does not cover this use case, same as general property of the connection.
So in that case I'd like to see this cleanup as a property/parameter instead of sniffing feature by table creation (because it is safer and configurable).

But if you'd like to be perfect you should always inspect transaction (an atomic session) and raise exception when user is trying to modify "non-transactional" table. In that case every model should "support_transaction" or not, and this seems like a design change.

In the other side I'm opposed to MySQL`s engines concept and I'm always trying to avoid this database where possible. So from my point of view it is enough to assume that MySQL supports transactions (or not), and allow to change this property by the user using some OPTION in engine configuration.

comment:3 by Tim Graham, 8 years ago

Triage Stage: UnreviewedAccepted

I don't have much MySQL expertise so I'm afraid I can't advise much here but I don't think Django needs to support the use case of per table transaction handling. If someone has such a need, perhaps that could be accomplished by routing to different database aliases.

As a workaround until a solution lands in Django, you could create your own MySQL backend that sets support_transaction as you like. Unless a discussion on the DevelopersMailingList reaches a different consensus, I don't think adding something in OPTIONS makes sense given that this doesn't seem to be much of an issue as far as tell. I haven't seen anyone else hit this problem since the current technique was introduced in Django 1.1 (344f16e2205a4959ba65d975716a38db77d2061e).

comment:4 by Marcin Nowak, 8 years ago

As a workaround until a solution lands in Django, you could create your own MySQL backend that sets support_transaction as you like.

It sound good. Thanks.

comment:5 by Mariusz Felisiak, 8 years ago

Owner: changed from nobody to Mariusz Felisiak
Status: newassigned
Last edited 8 years ago by Tim Graham (previous) (diff)

comment:6 by Mariusz Felisiak, 8 years ago

Triage Stage: AcceptedReady for checkin

comment:7 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In 6e4e0f4:

Fixed #26541 -- Allowed MySQL transaction detection to work without table creation.

Note: See TracTickets for help on using tickets.
Back to Top