#27558 closed Bug (fixed)
Setting db_index=False on existing ForeignKey causes constraint to be recreated on MySQL
Reported by: | Ed Morley | Owned by: | Ed Morley |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.10 |
Severity: | Normal | Keywords: | db-indexes, mysql |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Using:
- Django 1.10.3
- MySQL 5.6
STR:
1) Start with this models.py:
class Musician(models.Model): first_name = models.CharField(max_length=50) last_name = models.CharField(max_length=50) class Album(models.Model): artist = models.ForeignKey(Musician) name = models.CharField(max_length=100) class Meta: unique_together = ('artist', 'name')
2) Run ./manage.py makemigrations && ./manage.py migrate
to generate and apply the initial migration.
3) Add db_index=False
to the ForeignKey
(to work around the duplicate index issue discussed here) - making it:
artist = models.ForeignKey(Musician, db_index=False)
4) Run ./manage.py makemigrations
5) Run ./manage.py sqlmigrate <foo> 0002
to display the generated SQL
Expected:
Just the index is dropped.
BEGIN; -- -- Alter field artist on album -- DROP INDEX `foo_album_ca949605` ON `foo_album`; COMMIT;
Actual:
The constraint is removed and re-added, which is time consuming since the constraint is an index in itself.
BEGIN; -- -- Alter field artist on album -- ALTER TABLE `foo_album` DROP FOREIGN KEY `foo_album_artist_id_66b4953c_fk_foo_musician_id`; DROP INDEX `foo_album_ca949605` ON `foo_album`; ALTER TABLE `foo_album` ADD CONSTRAINT `foo_album_artist_id_66b4953c_fk_foo_musician_id` FOREIGN KEY (`artist_id`) REFERENCES `foo_musician` (`id`); COMMIT;
Additional context:
- The explicit
db_index=False
is needed to work around the duplicate index issue discussed here. - The duplicate index issue appears to be fixed on master (not sure by what) causing this not to repro there.
- I'm presuming the duplicate index isn't severe enough to warrant backporting, however dropping and re-adding the constraint seems pretty bad, so perhaps more worthy of a backport to 1.10.x?
Change History (9)
comment:1 by , 8 years ago
comment:2 by , 8 years ago
If it's a big performance problem, we can likely consider a backport as long as the patch isn't too complex. Could you try bisecting to find the commit that fixed it?
comment:3 by , 8 years ago
Ah thank you!
Bisecting shows the redundant index issue was fixed as of:
https://github.com/django/django/commit/3f76d1402dac9c2993d588f996dc1c331edbc9a7
Whilst the change to add_field()
in django/db/backends/base/schema.py
might look like a no-op (since the _field_should_be_indexed()
method in that file does the same thing as the lines being replaced), it actually wasn't - since mysql overrides the base self._field_should_be_indexed()
with it's own that handles ForeignKey
properly:
https://github.com/django/django/blob/3f76d1402dac9c2993d588f996dc1c331edbc9a7/django/db/backends/mysql/schema.py#L54-L67
comment:4 by , 8 years ago
Summary: | Setting db_index=False on existing ForeignKey causes constraint to be recreated → Setting db_index=False on existing ForeignKey causes constraint to be recreated on MySQL |
---|---|
Triage Stage: | Unreviewed → Accepted |
A less invasive fix for 1.10 is probably needed as the changing and adding of methods in that commit looks a bit risky for a stable release. The regression test can also be added to master.
comment:5 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
I have a reduced testcase for master + 1.10.x branch, and a 1 liner to fix the issue on 1.10.x.
Will clean up soon and open PRs.
As a workaround in the meantime, I was thinking of replacing the auto-generated migration with something like:
However I haven't yet figured out the best way to get the existing index name from Django.