[Django] #25253: Migrations do unecessary work when adding `blank=True` to M2M field on MySQL
Django
MySQL
----------------------------+----------------------------------
Reporter: trecouvr | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Keywords: migrations m2m mysql
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+----------------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25253>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
MySQL
Reporter: trecouvr | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: migrations m2m | Triage Stage:
mysql | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:1>
Django
MySQL
Reporter: trecouvr | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: migrations m2m | Triage Stage:
mysql | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by trecouvr):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:2>
Django
MySQL
Reporter: trecouvr | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: migrations m2m | Triage Stage:
mysql | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
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!).
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:3>
Django
MySQL
Reporter: trecouvr | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: migrations m2m | Triage Stage:
mysql | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by trecouvr):
Yes, I tried with Django 1.8, 1.8.2 & 1.8.3
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:4>
Django
MySQL
Reporter: trecouvr | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: migrations m2m mysql | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
Comment:
I can reproduce using your sample project.
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:5>
Django
MySQL
Reporter: trecouvr | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: migrations m2m mysql | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* type: Bug => Cleanup/optimization
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:6>
Django
MySQL
--------------------------------------+------------------------------------
Reporter: trecouvr | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: migrations m2m mysql | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:7>
Django
attributes that don't affect the schema
--------------------------------------+------------------------------------
Reporter: trecouvr | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: migrations m2m mysql | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
#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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:8>
Django
attributes that don't affect the schema
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: migrations m2m mysql | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by piquadrat):
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;
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:9>
Django
attributes that don't affect the schema
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: migrations m2m mysql | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Cameron Dawson):
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`);}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:10>
Django
attributes that don't affect the schema
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: assigned
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: migrations m2m mysql | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Shun Yu):
* owner: nobody => Shun Yu
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:11>
Django
attributes that don't affect the schema
--------------------------------------+------------------------------------
Reporter: Thomas Recouvreux | Owner: Shun Yu
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: migrations m2m mysql | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Shun Yu):
Just for clarity, this seems to be a MySQL specific symptom. I tried using
PostgreSQL 9.5.8 and Django 2.0.dev and could not replicate the add/drop
of indices.
{{{
from django.db import models
from django.contrib.auth.models import User
class Banana(models.Model):
- users = models.ForeignKey(User, on_delete=models.CASCADE)
+ users = models.ForeignKey(User, on_delete=models.PROTECT)
}}}
{{{
class Migration(migrations.Migration):
dependencies = [
('banana', '0001_initial'),
]
operations = [
migrations.AlterField(
model_name='banana',
name='users',
field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT,
to=settings.AUTH_USER_MODEL),
),
]
}}}
{{{
$ python manage.py sqlmigrate --database mysql banana 0001
BEGIN;
--
-- Create model Banana
--
CREATE TABLE `banana_banana` (`id` integer AUTO_INCREMENT NOT NULL PRIMARY
KEY, `users_id` integer NOT NULL);
ALTER TABLE `banana_banana` ADD CONSTRAINT
`banana_banana_users_id_b9f5715e_fk_auth_user_id` FOREIGN KEY (`users_id`)
REFERENCES `auth_user` (`id`);
COMMIT;
$ python manage.py sqlmigrate --database mysql banana 0002
BEGIN;
--
-- Alter field users on banana
--
ALTER TABLE `banana_banana` DROP FOREIGN KEY
`banana_banana_users_id_b9f5715e_fk_auth_user_id`;
ALTER TABLE `banana_banana` ADD CONSTRAINT
`banana_banana_users_id_b9f5715e_fk_auth_user_id` FOREIGN KEY (`users_id`)
REFERENCES `auth_user` (`id`);
COMMIT;
}}}
{{{
$ python manage.py sqlmigrate --database postgres banana 0001
BEGIN;
--
-- Create model Banana
--
CREATE TABLE "banana_banana" ("id" serial NOT NULL PRIMARY KEY, "users_id"
integer NOT NULL);
ALTER TABLE "banana_banana" ADD CONSTRAINT
"banana_banana_users_id_b9f5715e_fk_auth_user_id" FOREIGN KEY ("users_id")
REFERENCES "auth_user" ("id") DEFERRABLE INITIALLY DEFERRED;
CREATE INDEX "banana_banana_users_id_b9f5715e" ON "banana_banana"
("users_id");
COMMIT;
$ python manage.py sqlmigrate --database postgres banana 0002
BEGIN;
--
-- Alter field users on banana
--
COMMIT;
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:12>
Django
attributes that don't affect the schema
--------------------------------------+------------------------------------
Reporter: Thomas Recouvreux | Owner: Shun Yu
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: migrations m2m mysql | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Shun Yu):
I have a pull request: https://github.com/django/django/pull/9269
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:13>
Django
attributes that don't affect the schema
--------------------------------------+------------------------------------
Reporter: Thomas Recouvreux | Owner: Shun Yu
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: migrations m2m mysql | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mike Edmunds):
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''
[https://github.com/django/django/blob/2.0.5/django/db/backends/base/schema.py#L530-L542
drops any existing foreign key constraint(s)] and
[https://github.com/django/django/blob/2.0.5/django/db/backends/base/schema.py#L530-L542
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
[https://github.com/django/django/commit/a92bae0f0622fb45afb94bf5448b49bc32ebb643
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:14>
Django
attributes that don't affect the schema
--------------------------------------+------------------------------------
Reporter: Thomas Recouvreux | Owner: Shun Yu
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: migrations m2m mysql | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by israel-tk):
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
[https://docs.djangoproject.com/en/2.1/ref/migration-operations/#runsql
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. :)
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:15>
Django
attributes that don't affect the schema
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: migrations m2m mysql | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by JR Heard):
* status: assigned => new
* owner: Shun Yu => (none)
Comment:
The issue in this ticket is still happening for me on Django 2.1.9 on
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 :)
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:16>
Django
attributes that don't affect the schema
--------------------------------------+------------------------------------
Reporter: Thomas Recouvreux | Owner: (none)
Component: Migrations | Version: 2.1
Severity: Normal | Resolution:
Keywords: migrations m2m mysql | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by JR Heard):
* version: 1.8 => 2.1
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:17>
Django
attributes that don't affect the schema
--------------------------------------+------------------------------------
Reporter: Thomas Recouvreux | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 2.1
Severity: Normal | Resolution:
Keywords: migrations m2m mysql | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Phil Krylov):
* cc: Phil Krylov (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:18>
Django
attributes that don't affect the schema
--------------------------------------+------------------------------------
Reporter: Thomas Recouvreux | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 2.1
Severity: Normal | Resolution:
Keywords: migrations m2m mysql | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Adam (Chainz) Johnson):
* cc: Adam (Chainz) Johnson (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:19>
Django
attributes that don't affect the schema
Reporter: Thomas Recouvreux | Owner: Çağıl
Type: | Uluşahin
Cleanup/optimization | Status: assigned
Component: Migrations | Version: 2.1
Severity: Normal | Resolution:
mysql |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Çağıl Uluşahin):
* owner: (none) => Çağıl Uluşahin
* status: new => assigned
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:20>
Django
attributes that don't affect the schema
-------------------------------------+-------------------------------------
Reporter: Thomas Recouvreux | Owner: Çağıl
Type: | Uluşahin
Cleanup/optimization | Status: assigned
Component: Migrations | Version: 2.1
Severity: Normal | Resolution:
mysql | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:23>
Django
attributes that don't affect the schema
-------------------------------------+-------------------------------------
Reporter: Thomas Recouvreux | Owner: Çağıl
Type: | Uluşahin
Component: Migrations | Version: 2.1
Severity: Normal | Resolution: fixed
Keywords: migrations m2m | Triage Stage: Ready for
mysql | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"9159d173c3822312c653db7ff5b9a94b14af1dca" 9159d17]:
{{{
#!CommitTicketReference repository=""
revision="9159d173c3822312c653db7ff5b9a94b14af1dca"
Fixed #25253 -- Made AlterField operation a noop when changing attributes
that don't affect the schema.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25253#comment:24>