Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#29895 closed Cleanup/optimization (fixed)

Adjust atomic DDL documentation to avoid ambiguity with MySQL 8+ notion of atomic DDL.

Reported by: Nathan Klug Owned by: Rodrigo
Component: Documentation Version: 2.1
Severity: Normal Keywords:
Cc: Tom Forbes Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Rodrigo)

I noticed that Django's documentation says "The atomic attribute doesn’t have an effect on databases that don’t support DDL transactions (e.g. MySQL, Oracle)." here: https://docs.djangoproject.com/en/2.1/howto/writing-migrations/#non-atomic-migrations

I noticed that MySQL 8 adds support for atomic DDL statements though (https://dev.mysql.com/doc/refman/8.0/en/atomic-ddl.html). Is this something that Django supports when using MySQL 8 and the documentation is imprecise (if so, consider this a bug report to clarify documentation)? or is this not yet supported (if so, consider this a feature request)? Thanks!

https://github.com/django/django/pull/10718

Change History (9)

comment:1 by Simon Charette, 6 years ago

Component: UncategorizedMigrations
Summary: Atomic DDL statements in MySQL 8?Enable atomic DDL statements on MySQL 8+
Triage Stage: UnreviewedAccepted
Type: UncategorizedNew feature

The documentation is currently correct, the feature is disabled for all versions of MySQL. Did you give a try at running the suite locally against MySQL 8?

comment:2 by Tim Graham, 6 years ago

I tried adding:

@cached_property
def can_rollback_ddl(self):
    return not self.connection.mysql_is_mariadb and self.connection.mysql_version >= (8, 0)

to django/db/backends/mysql/features.py and was met with some failures such as "SAVEPOINT s140283873580864_x2 does not exist') in cache.tests.CreateCacheTableForDBCacheTests.test_createcachetable_observes_database_router that I couldn't figure out at a quick glance.

comment:3 by Tom Forbes, 6 years ago

Cc: Tom Forbes added

I had a look at this failure and I think I know why it occurs: https://dev.mysql.com/doc/refman/8.0/en/implicit-commit.html

When using atomic DDL in mysql certain statements immediately and implicitly commit the transaction. Django doesn't know about this, so it ends up failing because the savepoints no longer exist.

In any case MySQL cannot roll back DDL like Postgres can, and Atomic DDL is actually not the same as transactional DDL. I'm not entirely sure but it seems (according to this https://mysqlserverteam.com/atomic-ddl-in-mysql-8-0/) that it's just implicitly enabled/working without any additional code changes.

Version 0, edited 6 years ago by Tom Forbes (next)

comment:4 by Simon Charette, 6 years ago

Component: MigrationsDocumentation
Summary: Enable atomic DDL statements on MySQL 8+Adjust atomic DDL documentation to avoid ambiguity with MySQL 8+ notion of atomic DDL.
Type: New featureCleanup/optimization

Thanks for the investigation Tom.

When using atomic DDL in mysql certain statements immediately and implicitly commit the transaction...

Yeah that's the exact opposite of what Django means by atomic DDL as what PostgreSQL and SQLite support.

I've repurposed the ticket to be a documentation adjustment instead.

comment:5 by Rodrigo, 6 years ago

Owner: changed from nobody to Rodrigo
Status: newassigned

comment:6 by Rodrigo, 6 years ago

Description: modified (diff)
Has patch: set
Last edited 6 years ago by Tim Graham (previous) (diff)

comment:7 by Rodrigo, 6 years ago

Description: modified (diff)

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

Resolution: fixed
Status: assignedclosed

In ad191d9:

Fixed #29895 -- Doc'd why MySQL's atomic DDL statements don't work for atomic migrations.

comment:9 by Tim Graham <timograham@…>, 6 years ago

In 4c50673:

[2.1.x] Fixed #29895 -- Doc'd why MySQL's atomic DDL statements don't work for atomic migrations.

Backport of ad191d9e011f37d79a7f2df3da881b06539aaaea from master.

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