[Django] #24749: contribute_to_class(virtual_field=True) is ignored
Django
----------------------------------------------+--------------------
Reporter: hampsterx | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 1 | UI/UX: 0
----------------------------------------------+--------------------
/django/db/models/fields/__init__.py
{{{
def contribute_to_class(self, cls, name, virtual_only=False):
self.set_attributes_from_name(name)
self.model = cls
if virtual_only:
cls._meta.add_field(self, virtual=True)
self.attname, self.column = self.get_attname_column()
self.concrete = self.column is not None
def get_attname_column(self):
attname = self.get_attname()
column = self.db_column or attname
return attname, column
}}}
/django/db/models/options.py
{{{
def concrete_fields(self):
return make_immutable_fields_list("concrete_fields", (f for f in
self.fields if f.concrete)
}}}
concrete_fields() is checking for concrete property but
column = self.db_column or attname
will ensure it's never false :(
--
Ticket URL: <https://code.djangoproject.com/ticket/24749>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: hampsterx | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):
* severity: Normal => Release blocker
* needs_better_patch: => 0
* needs_tests: => 0
* easy: 1 => 0
* needs_docs: => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24749#comment:1>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* severity: Release blocker => Normal
Comment:
I don't see a release-blocking bug here. In every built-in case (e.g. [1],
[2], [3]), non-concrete fields explicitly override this behaviour, so any
built-in non-concrete fields have `concrete = False`. So while it doesn't
make any sense, it is working as advertised for all built-in fields (and
the documentation says nothing about custom fields in that regard).
Now this definitely should be cleaned up and/or properly documented
(especially in the "Writing custom model fields"[4] section). This ties in
to #16508 as well, I think.
[1]
https://github.com/django/django/blob/master/django/db/models/fields/related.py#L1276
[2]
https://github.com/django/django/blob/master/django/db/models/fields/related.py#L1730
[3]
https://github.com/django/django/blob/master/django/contrib/contenttypes/fields.py#L32
[4] https://docs.djangoproject.com/en/1.8/howto/custom-model-
fields/#writing-a-field-subclass
--
Ticket URL: <https://code.djangoproject.com/ticket/24749#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: hampsterx | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timgraham):
Anssi said this parameter may be misnamed:
If I recall correctly the real significance of virtual_only is that
fields added with this flag are copied to child models. Generic relations
need this as they need to know the model they are used on. Normal fields
are not copied, instead they are fetched directly from the parent model
when _meta is queried. [[BR]] [[BR]]
So, it might be possible to rename virtual_only to something like
copy_to_childs, and then virtuality of a field would be determined only on
it having a column.
https://groups.google.com/d/msg/django-developers/nkEuOnIf4Ao/IPp5E11WXDkJ
--
Ticket URL: <https://code.djangoproject.com/ticket/24749#comment:3>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timgraham):
After another look, the `virtual_only` flag controls whether or not the
field is added to `Options.virtual_fields` so maybe it's named
appropriately after all?
--
Ticket URL: <https://code.djangoproject.com/ticket/24749#comment:4>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 1.8
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
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/24749#comment:5>