#32675: Migration autodetector detects unnecessary changes.
----------------------------------+--------------------------------------
Reporter: Mariusz Felisiak | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 4.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------------------------
Comment (by David Wobrock):
Hi,
Indeed it seems that we didn't anticipate that this will generate new
migrations because of the missing `to_field`.
A lot of the modified tests in the initial PR are linked to the absence of
`to_field`
([https://github.com/django/django/commit/aa4acc164d1247c0de515c959f7b09648b57dc42
#diff-
506caa00017053ff8278de6efc2e59cc0c5cea22da9461482bdf16a9fc50af9eL1650 here
for instance])
My question would be: does this actually change any behaviour of the
existing migrations?
1/ If not, I guess we could stop taking `to_field` into account and
basically consider
{{{
models.ForeignKey(
to_field='id',
on_delete=models.SET_NULL,
blank=True, null=True,
to='contenttypes.ContentType',
verbose_name='content type',
)
}}}
and
{{{
models.ForeignKey(
on_delete=models.SET_NULL,
blank=True, null=True,
to='contenttypes.ContentType',
verbose_name='content type',
)
}}}
(without `to_field`) as identical. Which therefore wouldn't generate new
migrations.
2/ If it makes a behavioural difference, we would have to compute again
the `to_field` for the new model state.
When we dig into the technical whys, when deconstructing the old and new
fields (and
[https://github.com/django/django/commit/aa4acc164d1247c0de515c959f7b09648b57dc42
#diff-50cc08301fdd233fd64196aa90bc684d96ef557d99af67692995559646de567aL966
checking if they are different]), we have this one difference of
`to_field`:
{{{
(Pdb) p old_field_dec
('django.db.models.ForeignKey', [], {'verbose_name': 'content type',
'blank': True, 'null': True, 'on_delete': <function SET_NULL at
0x7fa5f297f9d0>, 'to': 'contenttypes.contenttype', 'to_field': 'id'})
(Pdb) p new_field_dec
('django.db.models.ForeignKey', [], {'verbose_name': 'content type',
'blank': True, 'null': True, 'on_delete': <function SET_NULL at
0x7fa5f297f9d0>, 'to': 'contenttypes.contenttype'})
}}}
So we can dig into how this `to_field` is generated for a `ForeignKey` =>
https://github.com/django/django/blob/main/django/db/models/fields/related.py#L889
{{{
to_meta = getattr(self.remote_field.model, "_meta", None)
if self.remote_field.field_name and (
not to_meta or (to_meta.pk and
self.remote_field.field_name != to_meta.pk.name)):
kwargs['to_field'] = self.remote_field.field_name
}}}
where `self.remote_field` is a `ManyToOneRel` that has a None `field_name`
attribute.
And if I follow the `ManyToOneRel` creation back correctly, we end up in
the `ForeignKey` constructor:
https://github.com/django/django/blob/main/django/db/models/fields/related.py#L786-L822
Where we can either define the `ManyToOneRel.field_name` in two ways:
- pass it explicitly (the way it is defined in the previous migration)
- or we retrieve it when `_meta` is available. However, it was the main
goal of the initial ticket #29899 to remove the rendering of models and
remove availability of `_meta`. (the code there referenced an old ticket
#12190, maybe that can help understand its purpose?)
I hope that helps in some way, and I'd be more than happy to work on a
patch if we can define what we want to do for this case :) (I mean, I
introduced this bug, so I definitely want to fix it ;-) )
--
Ticket URL: <https://code.djangoproject.com/ticket/32675#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/065.37de59b50822ce599dc6db5c687b48e5%40djangoproject.com.
