Opened 10 years ago

Closed 8 years ago

#24530 closed Bug (duplicate)

Index names are inconsistent on PostgreSQL (at least)

Reported by: Aymeric Augustin Owned by: nobody
Component: Migrations Version: 1.7
Severity: Normal Keywords:
Cc: Markus Holtermann, aksheshdoshi@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

1) Create a test_idx with the following model:

from django.db import models

class TestIndex(models.Model):
    foo = models.BooleanField(default=True, db_index=True)

2) Create initial migrations:

% ./manage.py makemigrations test_idx
Migrations for 'test_idx':
  0001_initial.py:
    - Create model TestIndex
% ./manage.py sqlmigrate test_idx 0001
BEGIN;
CREATE TABLE "test_idx_testindex" ("id" serial NOT NULL PRIMARY KEY, "foo" boolean NOT NULL);
CREATE INDEX "test_idx_testindex_acbd18db" ON "test_idx_testindex" ("foo");

COMMIT;
% ./manage.py migrate test_idx
Operations to perform:
  Apply all migrations: test_idx
Running migrations:
  Applying test_idx.0001_initial... OK

The index is called test_idx_testindex_acbd18db.

3) Remove db_index=True, create migrations:

% ./manage.py makemigrations test_idx
Migrations for 'test_idx':
  0002_auto_20150324_1417.py:
    - Alter field foo on testindex
% ./manage.py sqlmigrate test_idx 0002
BEGIN;
DROP INDEX "test_idx_testindex_acbd18db";

COMMIT;
% ./manage.py migrate test_idx
Operations to perform:
  Apply all migrations: test_idx
Running migrations:
  Applying test_idx.0002_auto_20150324_1417... OK

4) Restore db_index=True, create migrations:

% ./manage.py makemigrations test_idx
Migrations for 'test_idx':
  0003_auto_20150324_1417.py:
    - Alter field foo on testindex
% ./manage.py sqlmigrate test_idx 0003
BEGIN;
CREATE INDEX "test_idx_testindex_foo_685e905b0e683e77_uniq" ON "test_idx_testindex" ("foo");

COMMIT;
% ./manage.py migrate test_idx 0003
Operations to perform:
  Target specific migration: 0003_auto_20150324_1417, from test_idx
Running migrations:
  Applying test_idx.0003_auto_20150324_1417... OK

The index is now called test_idx_testindex_foo_685e905b0e683e77_uniq.

This is wrong: the index isn't unique.

5) Remove db_index=True, create migrations:

% ./manage.py makemigrations test_idx
Migrations for 'test_idx':
  0004_auto_20150324_1418.py:
    - Alter field foo on testindex
% ./manage.py sqlmigrate test_idx 0004
BEGIN;
DROP INDEX "test_idx_testindex_foo_9b7cff514ca041_uniq";

COMMIT;
% ./manage.py migrate test_idx
Operations to perform:
  Apply all migrations: test_idx
Running migrations:
  Applying test_idx.0004_auto_20150324_1418... OK

Interestingly Django still manages to drop the correct index. It looks at the index names it generated


This may minor, but things can go downhill very quickly if you squash migrations:

  • add db_index=True to an existing model, this creates an index as in 4)
  • squash migrations, the squashed migration will create an index as in 2)
  • remove db_index=True, Django will try to drop the index as in 3) — because it takes the name from the output of the squashed migration — while it should do it as in 5)

Also I'm checking that my production schema actually matches what my migrations generate and that would be easier if index names stayed consistent.

Change History (7)

comment:1 by Aymeric Augustin, 10 years ago

I think this is the correct fix:

--- django/db/backends/schema.py.orig	2015-03-24 14:05:55.000000000 +0100
+++ django/db/backends/schema.py	2015-03-24 14:05:57.000000000 +0100
@@ -632,7 +632,7 @@
         if (not old_field.db_index and new_field.db_index and
                 not new_field.unique and not
                 (not old_field.unique and new_field.unique)):
-            self.execute(self._create_index_sql(model, [new_field], suffix="_uniq"))
+            self.execute(self._create_index_sql(model, [new_field]))
         # Type alteration on primary key? Then we need to alter the column
         # referring to us.
         rels_to_update = []

I have verified that it produces the right index name if I add and remove db_index=True again.

% ./manage.py makemigrations test_idx
Migrations for 'test_idx':
  0005_auto_20150324_1434.py:
    - Alter field foo on testindex
% ./manage.py sqlmigrate test_idx 0005
BEGIN;
CREATE INDEX "test_idx_testindex_acbd18db" ON "test_idx_testindex" ("foo");

COMMIT;
% ./manage.py migrate test_idx
Operations to perform:
  Apply all migrations: test_idx
Running migrations:
  Applying test_idx.0005_auto_20150324_1434... OK

The index is called test_idx_testindex_acbd18db as if it had been created when the model was added.

% ./manage.py makemigrations test_idx
Migrations for 'test_idx':
  0006_auto_20150324_1434.py:
    - Alter field foo on testindex
% ./manage.py sqlmigrate test_idx 0006
BEGIN;
DROP INDEX "test_idx_testindex_acbd18db";

COMMIT;
% ./manage.py migrate test_idx
Operations to perform:
  Apply all migrations: test_idx
Running migrations:
  Applying test_idx.0006_auto_20150324_1434... OK

comment:2 by Aymeric Augustin, 10 years ago

One last thing — my production database contains indexes of the same form as in 2) even though they were created by AlterField operations on migrations. This leads me to think that this problem didn't exist in Django 1.7 but was introduced somewhere in a 1.7.x release.

Version 0, edited 10 years ago by Aymeric Augustin (next)

comment:3 by Markus Holtermann, 10 years ago

Cc: Markus Holtermann added
Triage Stage: UnreviewedAccepted

This indeed looks like a bug though it doesn't lead to crashes or data loss. I'm +1 for applying the patch to 1.8 but given the soon-to-be-unsupported state of 1.7 only +0 for backport to 1.7.

The patch looks clean to me.

comment:4 by Aymeric Augustin, 10 years ago

Has patch: set
Needs tests: set

(Patch is in comment 1.)

Markus: Django 1.7 will still be supported for 6 to 9 months, until 1.9 is released.

comment:5 by Markus Holtermann, 10 years ago

Oh, right, soon-to-be-unsupported is the wrong wording. 1.7 will only receive security fixes when 1.8 is out.

Do you want to open a PR and add a test case.

comment:6 by Akshesh Doshi, 9 years ago

Cc: aksheshdoshi@… added

comment:7 by Tim Graham, 8 years ago

Resolution: duplicate
Status: newclosed

Duplicate of #25694 which is fixed in 16614dcd5cad50648ef75021b919fc90dd449312:

Fixed #25694 -- Removed incorrect _uniq suffix on index names during migrations.

Note: See TracTickets for help on using tickets.
Back to Top