Opened 9 years ago
Closed 4 years ago
#25253 closed Cleanup/optimization (fixed)
MySQL migrations drop & recreate constraints unnecessarily when changing attributes that don't affect the schema
Reported by: | Thomas Recouvreux | Owned by: | Çağıl Uluşahin |
---|---|---|---|
Component: | Migrations | Version: | 2.1 |
Severity: | Normal | Keywords: | migrations m2m mysql |
Cc: | Phil Krylov, Adam Johnson | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
When running a MySQL migration to add blank=True
on a ManyToManyField
, Django drops the FK constraints on the join table and creates them back.
Here is the changeset for models.py:
from django.contrib.auth.models import User from django.db import models class Banana(models.Model): - users = models.ManyToManyField(User) + users = models.ManyToManyField(User, blank=True)
And here the corresponding migration:
.. operations = [ migrations.AlterField( model_name='banana', name='users', field=models.ManyToManyField(to=settings.AUTH_USER_MODEL, blank=True), ), ] ..
The resulting sql for MySQL backend is:
ALTER TABLE `myproject_banana_users` DROP FOREIGN KEY `myproject_banana_users_user_id_93c9ecfd1bc0dc2_fk_auth_user_id`; ALTER TABLE `myproject_banana_users` ADD CONSTRAINT `myproject_banana_users_user_id_60a2450416dd445f_fk_auth_user_id` FOREIGN KEY (`user_id`) REFERENCES `auth_user` (`id`); ALTER TABLE `myproject_banana_users` DROP FOREIGN KEY `myproject_bana_banana_id_4be506ef34515098_fk_myproject_banana_id`; ALTER TABLE `myproject_banana_users` ADD CONSTRAINT `myproject_bana_banana_id_43426c15f7d142f5_fk_myproject_banana_id` FOREIGN KEY (`banana_id`) REFERENCES `myproject_banana` (`id`);
I checked on PostgreSQL the resulting sql is empty, which looks good to me since there is no change to issue to the database.
Change History (23)
comment:1 by , 9 years ago
comment:2 by , 9 years ago
Hello,
You can find here the test project I built for this ticket : https://github.com/trecouvr/django_25253.
Just run ./manage.py sqlmigrate myproject 0002
to see the mentionned SQL query.
comment:3 by , 9 years ago
Did you try with Django 1.8.3 instead of 1.8 as listed in the requirements of that project? There have been a lot of bug fixes since 1.8 (not to mention security fixes!).
comment:5 by , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|
I can reproduce using your sample project.
comment:6 by , 9 years ago
Type: | Bug → Cleanup/optimization |
---|
comment:7 by , 9 years ago
Summary: | Migrations do unecessary work when adding `blank=True` to M2M field on MySQL → Migrations do unnecessary work when adding `blank=True` to M2M field on MySQL |
---|
comment:8 by , 9 years ago
Summary: | Migrations do unnecessary work when adding `blank=True` to M2M field on MySQL → MySQL migrations drop & recreate constraints unnecessarily when changing attributes that don't affect the schema |
---|
#25614 reported the same unnecessary dropping/adding of constraints when changing ForeignKey.on_delete
. I assume that's a duplicate of this ticket, but please reopen it if fixing this issue doesn't also fix it.
comment:9 by , 9 years ago
For what it's worth, I'm also seeing this on PostgreSQL 9.1, Django 1.9.5 and psycopg2 2.6.1. Specifically, adding an on_delete
argument to a ForeignKey
(but I was also able to reproduce it with just adding blank=True
) and then running sqlmigrate
on the generated migration results in this output:
BEGIN; -- -- Alter field last_occurrence on event -- ALTER TABLE "event_event" DROP CONSTRAINT "last_occurrence_id_refs_id_9b97c921"; ALTER TABLE "event_event" ADD CONSTRAINT "event_event_last_occurrence_id_a6d9e0d9_fk_event_instance_id" FOREIGN KEY ("last_occurrence_id") REFERENCES "event_instance" ("id") DEFERRABLE INITIALLY DEFERRED; COMMIT;
It might be closer related to #25614 (which is marked as a duplicate of this bug), since it only drops/adds the constraint, but leaves the foreign key itself alone.
comment:10 by , 7 years ago
I'm still hitting this with MySQL 5.7 and Django 1.11.3:
-- -- Alter field job on textlogstep -- ALTER TABLE `text_log_step` DROP FOREIGN KEY `text_log_step_job_id_698f3bfb_fk_job_id`; ALTER TABLE `text_log_step` ADD CONSTRAINT `text_log_step_job_id_698f3bfb_fk_job_id` FOREIGN KEY (`job_id`) REFERENCES `job` (`id`);
comment:11 by , 7 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:14 by , 7 years ago
Still seems to be an issue in Django 2.0.5.
The source of the problem appears to be BaseDatabaseSchemaEditor._alter_field in django.db.backends.base.schema. It unconditionally drops any existing foreign key constraint(s) and later re-creates it—whether or not anything is changing. (It seems to be more selective about dropping other types of constraints only if the field is changing in a way that impacts them.) The origin of this code is 2012, I think during initial implementation of Django migrations.
Are there non-obvious field changes where the field's FK constraint needs to be dropped and re-created? Or some other reason this code runs unconditionally? Optimization around other field changes?
A fix might be to defer dropping the FK constraint until just before the field's actions/null_actions/post_actions are collected and ready to be applied (around line 650), and then drop it only if there are actually actions to execute.
comment:15 by , 6 years ago
We had this problem with Postgres and Django 1.11 when deleting a not-null constraint in a ForeignKey field (adding null=True to a field). Instead of just dropping the not-null constraint which is a safe database operation, it first dropped the ForeignKey constraint, then dropped the not-null constraint, and last re-created the ForeignKey constraint (3 statements).
Re-creating ForeignKey constraints locks tables and can generate downtime. Since we cannot afford the downtime we used a RunSQL migration operation with state_operations
as a workaround:
migrations.RunSQL( """-- -- Alter field trip on payment -- ALTER TABLE "app_payment" ALTER COLUMN "trip_id" DROP NOT NULL; """, state_operations=[ migrations.AlterField( model_name='payment', name='trip', field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, related_name='payments', to='app.Trip'), ), ])
This way we only apply the SQL we want and Django won't complain about the migration state not matching the current model. :)
comment:16 by , 5 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
The issue in this ticket is still happening for me on Django 2.1.9 with Postgres. Adding blank=True
to a ManyToManyField
drops the FK constraints on the join table and creates them back, exactly as described.
This ticket hasn't been touched in a couple of years so I'm going to deassign it. I hope this is the right thing to do here, I'm new to the Django community :)
comment:17 by , 5 years ago
Version: | 1.8 → 2.1 |
---|
comment:18 by , 5 years ago
Cc: | added |
---|
comment:19 by , 5 years ago
Cc: | added |
---|
comment:20 by , 4 years ago
Needs tests: | set |
---|---|
Owner: | set to |
Status: | new → assigned |
comment:21 by , 4 years ago
Has patch: | set |
---|---|
Needs tests: | unset |
comment:22 by , 4 years ago
Patch needs improvement: | set |
---|
comment:23 by , 4 years ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
I couldn't reproduce this on master or the stable/1.8.x branch. I turned on Django's logging and didn't see this queries among the SQL that is executed. Could you provide any additional information?