Skip to content

Commit

Permalink
Refs #29182 -- Stopped relying on legacy alter table semantic on SQLi…
Browse files Browse the repository at this point in the history
…te 3.26+.

SQLite 3.26 changed the behavior of table and column renaming operations to
repoint foreign key references even if foreign key checks are disabled.

This makes the workarounds in place to simulate this behavior unnecessary on
SQLite 3.26+. Refs #30033.
  • Loading branch information
charettes authored and carltongibson committed Dec 17, 2018
1 parent 7289874 commit 894cb13
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 9 deletions.
2 changes: 1 addition & 1 deletion django/db/backends/sqlite3/features.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class DatabaseFeatures(BaseDatabaseFeatures):
supports_transactions = True
atomic_transactions = False
can_rollback_ddl = True
supports_atomic_references_rename = False
supports_atomic_references_rename = Database.version_info >= (3, 26, 0)
supports_paramstyle_pyformat = False
supports_sequence_reset = False
can_clone_databases = True
Expand Down
10 changes: 5 additions & 5 deletions django/db/backends/sqlite3/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,11 @@ def __enter__(self):
'SQLite3 does not support disabling them in the middle of '
'a multi-statement transaction.'
)
self.connection.cursor().execute('PRAGMA legacy_alter_table = ON')
return super().__enter__()

def __exit__(self, exc_type, exc_value, traceback):
super().__exit__(exc_type, exc_value, traceback)
self.connection.check_constraints()
self.connection.cursor().execute('PRAGMA legacy_alter_table = OFF')
self.connection.enable_constraint_checking()

def quote_value(self, value):
Expand Down Expand Up @@ -84,11 +82,12 @@ def _is_referenced_by_fk_constraint(self, table_name, column_name=None, ignore_s
return False

def alter_db_table(self, model, old_db_table, new_db_table, disable_constraints=True):
if disable_constraints and self._is_referenced_by_fk_constraint(old_db_table):
if (not self.connection.features.supports_atomic_references_rename and
disable_constraints and self._is_referenced_by_fk_constraint(old_db_table)):
if self.connection.in_atomic_block:
raise NotSupportedError((
'Renaming the %r table while in a transaction is not '
'supported on SQLite because it would break referential '
'supported on SQLite < 3.26 because it would break referential '
'integrity. Try adding `atomic = False` to the Migration class.'
) % old_db_table)
self.connection.enable_constraint_checking()
Expand All @@ -102,11 +101,12 @@ def alter_field(self, model, old_field, new_field, strict=False):
table_name = model._meta.db_table
_, old_column_name = old_field.get_attname_column()
if (new_field.name != old_field_name and
not self.connection.features.supports_atomic_references_rename and
self._is_referenced_by_fk_constraint(table_name, old_column_name, ignore_self=True)):
if self.connection.in_atomic_block:
raise NotSupportedError((
'Renaming the %r.%r column while in a transaction is not '
'supported on SQLite because it would break referential '
'supported on SQLite < 3.26 because it would break referential '
'integrity. Try adding `atomic = False` to the Migration class.'
) % (model._meta.db_table, old_field_name))
with atomic(self.connection.alias):
Expand Down
6 changes: 3 additions & 3 deletions tests/backends/sqlite/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ def test_field_rename_inside_atomic_block(self):
new_field.set_attributes_from_name('renamed')
msg = (
"Renaming the 'backends_author'.'name' column while in a "
"transaction is not supported on SQLite because it would break "
"referential integrity. Try adding `atomic = False` to the "
"transaction is not supported on SQLite < 3.26 because it would "
"break referential integrity. Try adding `atomic = False` to the "
"Migration class."
)
with self.assertRaisesMessage(NotSupportedError, msg):
Expand All @@ -136,7 +136,7 @@ def test_table_rename_inside_atomic_block(self):
"""
msg = (
"Renaming the 'backends_author' table while in a transaction is "
"not supported on SQLite because it would break referential "
"not supported on SQLite < 3.26 because it would break referential "
"integrity. Try adding `atomic = False` to the Migration class."
)
with self.assertRaisesMessage(NotSupportedError, msg):
Expand Down

1 comment on commit 894cb13

@zyv
Copy link
Contributor

@zyv zyv commented on 894cb13 Jan 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just hit it on 1.11.18, any plans for a backport :( ? Or, if not, would a backport be accepted?

Please sign in to comment.