Opened 5 years ago

Closed 4 years ago

#25694 closed Bug (fixed)

Migrate command creates a default index for a CharField with a wrong '_uniq' suffix in the name

Reported by: Federico Frenguelli Owned by: nobody
Component: Migrations Version: 1.8
Severity: Normal Keywords:
Cc: jon.dufresne@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This issue relates to #25412.

You can reproduce the error using this sample project: https://github.com/synasius/foo

Checkout the project and run ./manage.py sqlmigrate bar 0002.

You should see something like
CREATE INDEX "bar_car_name_32db83d8bfa014c3_uniq" ON "bar_car" ("name");

Change History (9)

comment:1 Changed 5 years ago by Simon Charette

Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

comment:2 Changed 5 years ago by varun naganathan

I came across 2 points:
1.The error occurs on all databases except an sqlite3 one.
2.If db_index=True in the first initial migration, _uniq is not suffixed.Whenever it is later being added in another migration _uniq is appended.
Now,database indexes have to be unique in the table.Maybe since we are later altering the field , the _uniq has to be appended?

comment:3 Changed 4 years ago by Jon Dufresne

Cc: jon.dufresne@… added
Has patch: set

comment:4 Changed 4 years ago by Markus Holtermann

Patch needs improvement: set

How do you suggest to take care of existing indexes that would now need to be renamed?

comment:5 Changed 4 years ago by Jon Dufresne

How do you suggest to take care of existing indexes that would now need to be renamed?

Is this a required component of this bug fix?

With the current implementation, migrations don't actually care about the name of the index. It is just for human aesthetics. When removing an index, Django will remove all non-unique indexes regardless of the name. I actually bumped into this while working on another ticket and thought it would be a quick fix.

If this is required, maybe there could be something like Field.index_name_version that could be inspected during migrations. If the version has changed, drop and recreate indexes. Then in the upgrade docs, recommend people run makemigrations after updating. Thoughts on this approach?

comment:6 in reply to:  5 Changed 4 years ago by Markus Holtermann

Replying to jdufresne:

How do you suggest to take care of existing indexes that would now need to be renamed?

Is this a required component of this bug fix?

Yes. If we suddenly end up with either 2 indexes on a field applications become slow. And, if not handled correctly, when we have 2 indexes on a field and only drop 1 we might not be able to drop the field given a stale index.

With the current implementation, migrations don't actually care about the name of the index. It is just for human aesthetics. When removing an index, Django will remove all non-unique indexes regardless of the name. I actually bumped into this while working on another ticket and thought it would be a quick fix.

So, you're saying Django already dropps all indexes on a field. Good. That makes the proposal redundant.

comment:7 Changed 4 years ago by Jon Dufresne

So, you're saying Django already dropps all indexes on a field. Good. That makes the proposal redundant.

Yes, it drops all indexes when there is a change in field's db_index state. A link to the code:

https://github.com/django/django/blob/ca77b509059831b055a3b735ff77e042f8e1c0eb/django/db/backends/base/schema.py#L537-L544

Notice it loops over all indexes, instead of looking them up by name. The comment "no strict check, as multiple indexes are possible" even suggests this is intentional.

However, if there is no change to the field's state, the indexes will not be dropped/recreated. So upgrading to the next Django version will leave some of the old names with _uniq around until the field's state changes. This is why I proposed the "version" idea. That could be recognized as a change in state.

With this new understanding, is no action acceptable? Can I unset "Patch needs improvement"? Or should I got ahead with the "version" proposal?

comment:8 Changed 4 years ago by Jon Dufresne

Patch needs improvement: unset

Good. That makes the proposal redundant.

Ok thanks. Then I will proceed.

I have updated the test to work with Oracle.

comment:9 Changed 4 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 16614dc:

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

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