[Django] #29331: Model fields where the python attribute name does not match the db column are not saved
Django
are not saved
-------------------------------------+-------------------------------------
Reporter: Ben | Owner: nobody
Mathes |
Type: Bug | Status: new
Component: Database | Version: 2.0
layer (models, ORM) | Keywords: models,
Severity: Normal | deferred_field, save
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
When defining a model, if you rename a model's db column, django will
thing that model is deferred and will not {{{save()}}} it.
e.g.
{{{
class SomeModel(models.Model):
"""
a contrived model field where we want a "field" that is stored
in a "field" column, but we use @property getter/setters so
we name the SomeModel class's attribute as "_field".
"""
name = models.TextField(null=True)
_field = models.TextField(name="field")
@property
def field(self):
return self._field.upper()
@field.setter
def field(self, new_value):
self._field = new_value.lower()
}}}
With a renamed db column, {{{"_field"}} is in {{{self.__dict__}}}, but
{{{"field"}}} is not,
{{{
def get_deferred_fields(self):
"""
Return a set containing names of deferred fields for this instance.
"""
return {
f.attname for f in self._meta.concrete_fields
if f.attname not in self.__dict__
}
}}}
So {{{field}}} is not saved in {{{.save()}}}, because django _mistakenly_
thinks {{{"field"}}} is deferred, so it is ignored during {{{.save()}}}
{{{
#
https://github.com/django/django/blob/93331877c81c1c6641b163b97813268f483ede4b/django/db/models/base.py#L712
# ...
# elif not force_insert and deferred_fields and using ==
self._state.db:
# field_names = set()
# for field in self._meta.concrete_fields:
# if not field.primary_key and not hasattr(field, 'through'):
# field_names.add(field.attname)
# -> loaded_fields = field_names.difference(deferred_fields)
# if loaded_fields:
# update_fields = frozenset(loaded_fields)
#
# self.save_base(using=using, force_insert=force_insert,
# force_update=force_update,
update_fields=update_fields)
# ...
}}}
Reproduced in this github django repo:
https://github.com/benmathes/deferred_fields_bug
--
Ticket URL: <https://code.djangoproject.com/ticket/29331>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
are not saved
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: models, | Triage Stage:
deferred_field, save | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/29331#comment:1>
Django
are not saved
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: models, | Triage Stage:
deferred_field, save | Unreviewed
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
{{{
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29331#comment:2>
Django
are not saved
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: models, | Triage Stage:
deferred_field, save | Unreviewed
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> When defining a model, if you rename a model's db column, django will
New description:
{{{
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29331#comment:3>
Django
saved
-------------------------------------+-------------------------------------
Reporter: Ben Mathes | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
deferred_field, save |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_tests: 1 => 0
* stage: Unreviewed => Accepted
Comment:
That's interesting, I didn't know about the `name` argument to `Field`. Is
it documented? The last issue I found for it was #14695.
I'm not sure if this can be fixed. Do you plan to offer a patch? If not, I
suppose the restriction could at least be documented.
--
Ticket URL: <https://code.djangoproject.com/ticket/29331#comment:4>
Django
saved
-------------------------------------+-------------------------------------
Reporter: Ben Mathes | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: models, | Triage Stage: Accepted
deferred_field, save |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ben Mathes):
We're working around it in our specific use case, but not a general
solution. Possible the codebase where we found this was mistaking
{{{name}}} for {{{db_column}}}, as setting the {{{name}}} kwarg does seem
to set the DB column name.
--
Ticket URL: <https://code.djangoproject.com/ticket/29331#comment:5>
Django
saved
-------------------------------------+-------------------------------------
Reporter: Ben Mathes | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: models, | Triage Stage: Accepted
deferred_field, save |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ben Mathes):
I suspect this is related, since the root cause is a change in
{{{get_deferred_attributes}}}
https://github.com/jpwatts/django-positions/issues/49
--
Ticket URL: <https://code.djangoproject.com/ticket/29331#comment:6>
Django
saved
-------------------------------------+-------------------------------------
Reporter: Ben Mathes | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: models, | Triage Stage: Accepted
deferred_field, save |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
FWIW adjusting your descriptor to use `__dict__['field']` instead of
`__dict__['_field']` should work just fine.
{{{#!python
class SomeModel(models.Model):
"""
a contrived model field where we want a "field" that is stored
in a "field" column, but we use @property getter/setters so
we name the SomeModel class's attribute as "_field".
"""
name = models.TextField(null=True)
_field = models.TextField(name="field")
@property
def field(self):
return self.__dict__['field'].upper()
@field.setter
def field(self, new_value):
self.__dict__['field'] = new_value.lower()
}}}
I don't think we can change the assumption that `name` either set
implicitly or explicitly through a parameter the will be present in
`__dict__` without breaking a lot of stuff.
--
Ticket URL: <https://code.djangoproject.com/ticket/29331#comment:7>
Django
saved
-------------------------------------+-------------------------------------
Reporter: Ben Mathes | Owner: nobody
Component: Database layer | Version: 2.0
(models, ORM) |
Keywords: models, | Triage Stage: Accepted
deferred_field, save |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => invalid
--
Ticket URL: <https://code.djangoproject.com/ticket/29331#comment:8>