Opened 9 years ago
Closed 8 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 by , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Bug |
comment:2 by , 9 years ago
comment:4 by , 8 years ago
Patch needs improvement: | set |
---|
How do you suggest to take care of existing indexes that would now need to be renamed?
follow-up: 6 comment:5 by , 8 years ago
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 by , 8 years ago
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 by , 8 years ago
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:
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 by , 8 years ago
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.
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?