#16508: Provide real support for virtual fields
-------------------------------------+-------------------------------------
Reporter: vzima | Owner: pirosb3
Type: New feature | Status: assigned
Component: Database layer | Version: master
(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 koniiiik):
I tried to dig into item number 1 from comment:10, but I've hit a snag. I
created [https://github.com/django/django/pull/6477 PR 6477] with the
work-in-progress patch.
The patch breaks the tests for `GenericRelatedObjectManager.add`, and the
reason is that before the change, `instance_pre_init` only sets the values
of the content type and primary key, but it does not cache the target
instance in `cache_attr`. However, with the change, the regular `__set__`
is called, which caches the instance as well.
The problem with this is that `GenericRelatedObjectManager.add` only
assigns the values of the content type, and the primary key, but as far as
I can see, `GenericRelatedObjectManager` does not hold any reference to
the `GenericForeignKey` field on the other side of the relationship, not
even its name (either of the two would make this problem go away). It does
not really need to, as long as it only operates with the basic values, but
it makes it impossible for me to invalidate the cached instance.
Arguably, this might be considered a bug in the current implementation,
since it is always possible to change the raw primary key of the target,
but any subsequent reads of the target object will still return the old
instance.
I can see the following ways out:
1. Make `GenericRelation` hold a reference to the corresponding
`GenericForeignKey`. This would mean that a lack of `GenericForeignKey` on
the other side would become a hard failure; right now, there's a check
that reports this situation as an error, but in theory, the
`GenericRelation` should work just fine anyway, which would no longer be
the case.
2. Reorganize `GenericForeignKey.__get__` a little bit to check that the
cached instance matches the raw values. This would mean that even if there
is already a cached instance, the content type would have to be looked up
on every read. This adds a bit of overhead, although at least content
types are cached, so it shouldn't actually hit the database most of the
time.
Presonally, I would probably prefer option 2, because it solves the more
general problem of stale cached instances, but if anyone disagrees, I'd be
happy to implement the first option, too.
--
Ticket URL: <https://code.djangoproject.com/ticket/16508#comment:15>
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/063.5692a716d74159fc743f17d14dba26ef%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
