[Django] #33185: sqlmigrate crashes given a RenameModel operation with a self-referencing foreign key on MySQL
Django
foreign key on MySQL
---------------------------------------+------------------------
Reporter: Jacob Walls | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------+------------------------
A migration that renames a table having a self-referencing foreign key
will migrate just fine but crashes when using `sqlmigrate` to inspect SQL
statements beforehand. I tested on MySQL 5.7.31.
1. fresh project `selfapp`
2. models.py -->
{{{
class Employee(models.Model):
manager = models.ForeignKey('self', on_delete=models.SET_NULL,
null=True)
}}}
3. `python manage.py makemigrations`
4. `python manage.py migrate`
5. models.py -->
{{{
class RenamedEmployee(models.Model):
manager = models.ForeignKey('self', on_delete=models.SET_NULL,
null=True)
}}}
6. `python manage.py makemigrations` answer prompt `Was the model
selfapp.Employee renamed to RenamedEmployee? [y/N] ` with Y
-- resulting migration: --
{{{
# Generated by Django 4.1.dev20211008135104 on 2021-10-10 16:11
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
('selfapp', '0001_initial'),
]
operations = [
migrations.RenameModel(
old_name='Employee',
new_name='RenamedEmployee',
),
]
}}}
7. `python manage.py sqlmigrate selfapp 0002`
{{{
Traceback (most recent call last):
File "/Users/jwalls/django/django/db/backends/utils.py", line 82, in
_execute
return self.cursor.execute(sql)
File "/Users/jwalls/django/django/db/backends/mysql/base.py", line 73,
in execute
return self.cursor.execute(query, args)
File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9
/site-packages/MySQLdb/cursors.py", line 206, in execute
res = self._query(query)
File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9
/site-packages/MySQLdb/cursors.py", line 319, in _query
db.query(q)
File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9
/site-packages/MySQLdb/connections.py", line 259, in query
_mysql.connection.query(self, query)
MySQLdb._exceptions.ProgrammingError: (1146, "Table
'mydatabase.selfapp_renamedemployee' doesn't exist")
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/Users/jwalls/self/manage.py", line 22, in <module>
main()
File "/Users/jwalls/self/manage.py", line 18, in main
execute_from_command_line(sys.argv)
File "/Users/jwalls/django/django/core/management/__init__.py", line
419, in execute_from_command_line
utility.execute()
File "/Users/jwalls/django/django/core/management/__init__.py", line
413, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/Users/jwalls/django/django/core/management/base.py", line 363, in
run_from_argv
self.execute(*args, **cmd_options)
File
"/Users/jwalls/django/django/core/management/commands/sqlmigrate.py", line
29, in execute
return super().execute(*args, **options)
File "/Users/jwalls/django/django/core/management/base.py", line 407, in
execute
output = self.handle(*args, **options)
File
"/Users/jwalls/django/django/core/management/commands/sqlmigrate.py", line
65, in handle
sql_statements = loader.collect_sql(plan)
File "/Users/jwalls/django/django/db/migrations/loader.py", line 352, in
collect_sql
state = migration.apply(state, schema_editor, collect_sql=True)
File "/Users/jwalls/django/django/db/migrations/migration.py", line 125,
in apply
operation.database_forwards(self.app_label, schema_editor, old_state,
project_state)
File "/Users/jwalls/django/django/db/migrations/operations/models.py",
line 343, in database_forwards
schema_editor.alter_field(
File "/Users/jwalls/django/django/db/backends/base/schema.py", line 618,
in alter_field
self._alter_field(model, old_field, new_field, old_type, new_type,
File "/Users/jwalls/django/django/db/backends/base/schema.py", line 631,
in _alter_field
fk_names = self._constraint_names(model, [old_field.column],
foreign_key=True)
File "/Users/jwalls/django/django/db/backends/base/schema.py", line
1344, in _constraint_names
constraints = self.connection.introspection.get_constraints(cursor,
model._meta.db_table)
File "/Users/jwalls/django/django/db/backends/mysql/introspection.py",
line 287, in get_constraints
cursor.execute("SHOW INDEX FROM %s" %
self.connection.ops.quote_name(table_name))
File "/Users/jwalls/django/django/db/backends/utils.py", line 98, in
execute
return super().execute(sql, params)
File "/Users/jwalls/django/django/db/backends/utils.py", line 66, in
execute
return self._execute_with_wrappers(sql, params, many=False,
executor=self._execute)
File "/Users/jwalls/django/django/db/backends/utils.py", line 75, in
_execute_with_wrappers
return executor(sql, params, many, context)
File "/Users/jwalls/django/django/db/backends/utils.py", line 84, in
_execute
return self.cursor.execute(sql, params)
File "/Users/jwalls/django/django/db/utils.py", line 90, in __exit__
raise dj_exc_value.with_traceback(traceback) from exc_value
File "/Users/jwalls/django/django/db/backends/utils.py", line 82, in
_execute
return self.cursor.execute(sql)
File "/Users/jwalls/django/django/db/backends/mysql/base.py", line 73,
in execute
return self.cursor.execute(query, args)
File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9
/site-packages/MySQLdb/cursors.py", line 206, in execute
res = self._query(query)
File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9
/site-packages/MySQLdb/cursors.py", line 319, in _query
db.query(q)
File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9
/site-packages/MySQLdb/connections.py", line 259, in query
_mysql.connection.query(self, query)
django.db.utils.ProgrammingError: (1146, "Table
'mydatabase.selfapp_renamedemployee' doesn't exist")
}}}
8. `python manage.py migrate` works fine:
{{{
Operations to perform:
Apply all migrations: admin, auth, contenttypes, selfapp, sessions
Running migrations:
Applying selfapp.0002_rename_employee_renamedemployee... OK
}}}
9. And now `python manage.py sqlmigrate selfapp 0002` works, but I wanted
to verify the safety of SQL (in a larger migration) before migrating:
{{{
--
-- Rename model Employee to RenamedEmployee
--
RENAME TABLE `selfapp_employee` TO `selfapp_renamedemployee`;
ALTER TABLE `selfapp_renamedemployee` DROP FOREIGN KEY
`selfapp_renamedemplo_manager_id_51e9d90c_fk_selfapp_r`;
ALTER TABLE `selfapp_renamedemployee` ADD CONSTRAINT
`selfapp_renamedemplo_manager_id_51e9d90c_fk_selfapp_r` FOREIGN KEY
(`manager_id`) REFERENCES `selfapp_renamedemployee` (`id`);
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33185>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
foreign key on MySQL
Reporter: Jacob Walls | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: dev
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Unreviewed => Accepted
Comment:
Thanks for the report. Reproduced at
01bf679e59850bb7b3e6395f1683bd1463ed9969 and MySQL 8.0.26.
--
Ticket URL: <https://code.djangoproject.com/ticket/33185#comment:1>
Django
foreign key on MySQL
Reporter: Jacob Walls | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
I don't see an easy way to address this bug unfortunately.
The logic in `RenameModel.database_forwards`
[https://github.com/django/django/blob/514c16e85f7ac2512235f3b6413646627420e969/django/db/migrations/operations/models.py#L324-L347
that deals with related fields alterations] expects the table to be
effectively renamed by the time it calls `SchemaEditor.alter_field`. This
code was added by 4ce7a6bc84c68406e39f48550434faeef3277eba to fix #22750
and #22248.
We could add a hack of `schema_editor.collect_sql` somewhere but it seems
like the proper way of dealing with this issue would be to avoid dropping
constraints and recreating them which is wasteful and the reason why we
are doing table introspection against the old table name.
Maybe we should have a low level `SchemaEditor.rename_model` operation
that takes care of performing the constraint renames and implicit many-to-
many table renames by itself (like `rename_field` does). I could see
having `RenameConstraint` and `RenameIndex` operations with associated
`SchemaEditor.rename_(constraint|index)` method could also be useful in
the future.
----
In the end there's no way we can get `sqlmigrate` to be foolproof while
having the schema editor rely on introspection. The above solutions would
work for a migration with a single `[RenameModel]` operation but would
fail for a sequence of two due to the state drift between the database and
the model state caused by SQL capture of DDL statement. I think we should
either commit to not using introspection in schema editor and making
`sqlmigrate` a first class citizen that doesn't require a database
connection or 'wontfix' such tickets and possibly consider deprecating
this command in the future.
--
Ticket URL: <https://code.djangoproject.com/ticket/33185#comment:2>
Django
foreign key on MySQL
Reporter: Jacob Walls | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by David Wobrock):
Replying to [comment:2 Simon Charette]:
> there's no way we can get `sqlmigrate` to be foolproof while having the
schema editor rely on introspection
I completely agree with this statement, and it would be interesting to try
and address it.
> either commit to not using introspection in schema editor and making
`sqlmigrate` a first class citizen that doesn't require a database
connection
But I'm wondering if that is even possible. I have the impression that
many objects and names created in DB by Django can be vendor specific. And
if we don't establish a DB connection (to know the vendor and the DB
version), we cannot deterministically know the names used and allowed in
the database.
It would probably be very interesting to have such a common and
predictable ground across supported database, but backward compatibility
might bite us when trying to commonise it - and I'm not even sure it's
possible.
--
Ticket URL: <https://code.djangoproject.com/ticket/33185#comment:3>
Django
foreign key on MySQL
Reporter: Jacob Walls | Owner: wookkl
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by wookkl):
* owner: nobody => wookkl
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/33185#comment:4>
Django
foreign key on MySQL
Reporter: Jacob Walls | Owner: (none)
Component: Migrations | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by wookkl):
* owner: wookkl => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/33185#comment:5>
Django
foreign key on MySQL
Reporter: Jacob Walls | Owner: (none)
Component: Migrations | Version: dev
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
Changes (by HarshAlreja):
* has_patch: 0 => 1
Comment:
Submitted my PR to this ticket
->https://github.com/django/django/pull/20523
--
Ticket URL: <https://code.djangoproject.com/ticket/33185#comment:6>
Django
foreign key on MySQL
Reporter: Jacob Walls | Owner: (none)
Component: Migrations | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Changes (by Jacob Walls):
* needs_better_patch: 0 => 1
Comment:
PR is not ready for review.
--
Ticket URL: <https://code.djangoproject.com/ticket/33185#comment:7>