Opened 5 years ago
Closed 5 years ago
#31064 closed Bug (fixed)
Migration doesn't detect precision changes in fields that ManyToMany points to.
Reported by: | zapililirad | Owned by: | Simon Charette |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Simon Charette | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
In my case was:
models.py:
class Vulnerability(models.Model): cve_id = models.CharField(max_length=15, primary_key=True) app = models.ManyToManyField(AppVersion) class Meta: managed = True
Later, i changed cve_id max_length to 100 and did migration:
operations = [ migrations.AlterField( model_name='vulnerability', name='cve_id', field=models.CharField(max_length=100, primary_key=True, serialize=False), ), ]
In result:
cve_id field length was changed, but vulnerability_id field length in table vulnerability_app remain unchanged
Change History (13)
follow-up: 3 comment:1 by , 5 years ago
Component: | Database layer (models, ORM) → Migrations |
---|---|
Summary: | Wrong db migration in case of change PK which in many to many relationship → Migration doesn't detect precision changes in fields that ManyToMany points to. |
Triage Stage: | Unreviewed → Accepted |
Version: | 2.2 → master |
comment:2 by , 5 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 5 years ago
I have not been able to reproduce this on my machine. Could you kindly provide me with a minimal code sample, so that I could get a hint of what's going wrong here?
Replying to felixxm:
Thanks I was able to reproduce this issue on SQLite (
ForeignKey
s are not affected).
follow-up: 5 comment:4 by , 5 years ago
Sanskar it's possible the issue is only present on SQLite because it deals with column alterations in a different way since it doesn't support ALTER TABLE
.
The idea is that is you have the follow models
class Author(models.Model): pass class Book(models.Model): isbn = models.CharField(max_length=10, primary_key=True) authors = models.ManyToManyField(Author)
Django will automatically created a trough
model
# Automatically created by Django class Book_Authors(models.Model): book= models.ForeignKey(Book) # This is a models.CharField(max_length=10) referring to Book.isbn author = models.ForeignKey(Author)
Now the reported issue is that if you If you change Book.isbn.max_length
to 13 the Book_Authors.book
field in the intermediary table that backs the Book.authors
many-to-many relationship currently doesn't change to max_length=13
.
The re-pointing logic currently lives in BaseDatabaseSchemaEditor._alter_field
but it's possible that it's only an issue with SQLite schema editor.
By a quick glance at the code it looks like it could be a SQLite only issue due to this particular line.
follow-up: 8 comment:5 by , 5 years ago
Replying to Simon Charette:
The re-pointing logic currently lives in
BaseDatabaseSchemaEditor._alter_field
but it's possible that it's only an issue with SQLite schema editor.
By a quick glance at the code it looks like it could be a SQLite only issue due to this particular line.
After I did a bit of digging, it seems like this particular line was indeed the culprit. Instead of calling related_objects
on new_field.model._meta
, if we call _get_fields(forward=False, reverse=True, include_hidden=True)
on the same, the ForeignKey
in the through
Model changes its type according to the ManyToManyField
.
comment:6 by , 5 years ago
Cc: | added |
---|
That makes sense, good old model level cache biting us again. If related_objects
gets out of sync it's likely because we forgot to clear something in AlterField.state_forwards
and my money is on is_referenced_by_foreign_key
not taking into account ManyToManyField
defined without an explicit through
since that won't create any entry in state.models
.
comment:7 by , 5 years ago
Owner: | changed from | to
---|
Dart I'll assign this to me if you don't mind since I have an idea of how to address this issue thanks to Sanskar's investigation.
comment:8 by , 5 years ago
Replying to Simon Charette:
That makes sense, good old model level cache biting us again. If
related_objects
gets out of sync it's likely because we forgot to clear something inAlterField.state_forwards
and my money is onis_referenced_by_foreign_key
not taking into accountManyToManyField
defined without an explicitthrough
since that won't create any entry instate.models
.
Hey Simon,
I don't know if this helps, but while I was playing around with this issue, calling related_objects
on new_field.model._meta
, always gave me an empty ImmutableList
, but calling _get_fields(forward=False, reverse=True, include_hidden=True)
gave me an ImmutableList
of a ManyToOneRel
. If we look at related_objects right now, it only returns Relation objects which are not hidden or their field's many_to_many
attribute is True
. I am not quite sure whether this is intended or a bug though.
Cheers
Sanskar
comment:10 by , 5 years ago
Patch needs improvement: | set |
---|
comment:11 by , 5 years ago
Patch needs improvement: | unset |
---|
comment:12 by , 5 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Thanks I was able to reproduce this issue on SQLite (
ForeignKey
s are not affected).