[Django] #18012: Propagate reverse foreign keys from proxy models to base class
Django
----------------------------------------------+--------------------
Reporter: akaariai | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: 1.4
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
The idea of proxy model is that the proxy subclass changes behavior, but
the base class and all proxy models derived from it (call this "proxy
equivalence class") are identical in the database representation. Foreign
keys defined to proxy models break this idea:
{{{
class A(models.Model):
pass
class ProxyA(A):
class Meta:
proxy = True
class B(models.Model):
a = models.ForeignKey(ProxyA)
}}}
The above code is allowed currently, but A does not have a reverse
relation to B, while ProxyA has that. It would be nice from
maintainability perspective to have exactly same field set for every model
in the proxy equivalence class. In addition, adding the reverse relation
to the equivalence class would make the database representation equivalent
to the Python representation.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: akaariai | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.4
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by carljm):
* needs_docs: => 0
* needs_better_patch: => 0
* type: Uncategorized => Cleanup/optimization
* needs_tests: => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:1>
Django
Reporter: akaariai | Owner: nobody
Cleanup/optimization | Version: 1.4
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: charette.s@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:2>
Django
Reporter: akaariai | Owner: nobody
Cleanup/optimization | Version: 1.4
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by Alex):
It doesn't seem correct, IMO, for A to have the FKey reverse descriptor on
it.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:3>
Django
Reporter: akaariai | Owner: nobody
Cleanup/optimization | Version: 1.4
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by akaariai):
The main point here is that in the DB representation A has b_set (there is
a foreign key from B to A).
The original problem IIRC was that some users want to use the foreign key
to proxy model so that they can get proxy model instances instead of the
concrete class instance (auth.User was the use case, again IIRC).
The foreign key to proxy class doesn't make sense from data modeling
perspective (there really can't be a foreign key to proxy class in the DB
layer). It is a hack to get the related models as a proxy class. Now, if
there was an easy way to achieve retrieval as proxy, then the whole FK to
proxy could be deprecated (unless I am missing some other important use
case). Whole another matter is that I don't have any idea what that API
might be. You would need to handle select_related, prefetch_related, and
fetch through attribute at least.
Still, I have no need to change the current behavior. Should I just close
this ticket?
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:4>
Django
Reporter: akaariai | Owner: nobody
Cleanup/optimization | Version: 1.4
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by charettes):
IMHO when I explicitly create a FK to a proxy model I expect this
behaviour. Maybe we should just document this?
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:5>
Django
Reporter: akaariai | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by akaariai):
* component: Database layer (models, ORM) => Documentation
Comment:
Seems like I am in the opposition here. So, lets not change the current
code.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:6>
Django
Reporter: akaariai | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* cc: charette.s@… (removed)
* cc: charettes (added)
* component: Documentation => Database layer (models, ORM)
Comment:
Rereading this ticket three years later I think I agree with Anssi at
last.
Just to make sure, given the scenario defined above, is this what you have
in mind?
{{{#!python
a = A.objects.create()
a.b_set.all() # Not actually possible
proxya = ProxyA.objects.get()
proxy.b_set.all() # Actually possible
b = B.objects.create(a=proxya) # Should we allow assigning an A instance
here?
b.refresh_from_db()
assert isinstance(b.a, ProxyA)
assert A.objects.get(b_set=b) == a # Not actually possible
assert ProxyA.objects.get(b_set=b) == proxya
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:7>
Django
Reporter: akaariai | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
Here's a [https://github.com/django/django/pull/5378/files PR] with what
looks like the required changes.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:8>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* owner: nobody => charettes
* status: new => assigned
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:9>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I think Anssi's point about "if there was an easy way to achieve retrieval
as proxy, then the whole FK to proxy could be deprecated (unless I am
missing some other important use case)" is an interesting one. Maybe this
could be achieved if proxy models added an `as_<proxy_name>()` method to
the proxied class. For example, instead of `B` defining the foreign key to
`ProxyA`, it would defined a foreign key to `A` and if you want to access
`ProxyA` instead:
{{{
>>> b = B.objects.get()
>>> b.a.as_proxya()
<ProxyA>
}}}
I'm not a big user of proxy models so there could very well be problems
with this approach, but I think we should be sure to thoroughly reject
that option before continuing down the road of enhanced support for
relationships to proxy models.
A related issue is "#10961 - Allow users to override forward and reverse
relationships on proxy models with ForeignKey fields".
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:10>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
ISTM that by far the most intuitive API for "I want to get a proxy object
back from following this relationship" is to simply point the FK at the
proxy class instead of the base class, like you do now. Any other API
feels hacky to me by comparison, and is one more new thing (without
parallel elsewhere) that people have to learn.
I don't really see that anyone has yet provided a strong motivation for
this ticket; the current behavior seems fine and right to me. It seems to
me that this ticket is mostly a result of seeking too much equivalence
between the DB schema and the models API.
That said, I'm not strongly opposed to making the reverse descriptor
available on the base class too, it just seems a bit odd to me.
I am strongly opposed to deprecating FKs to proxy classes and replacing
them with a new API that people have to learn to get a proxy instance back
from an FK.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:11>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
I also agree with Carl. I gave a try at writing a patch for this in order
to see if anything would break by making the reverse descriptor and
reverse field available on the base class.
It looks like the only the backward incompatible change would be the at
the `_meta` API level where the moved related objects would be returned.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:12>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
To clarify what I meant by "this ticket is mostly a result of seeking too
much equivalence between the DB schema and the models API": the entire
point of proxy classes is that they are _not_ equivalent to their base
class in terms of their Python behavior and API. FK reverse descriptors
(although their data is provided by an underlying schema in which there is
no distinction between a proxy and its base) are part of the Python
behavior and API, so it is entirely reasonable and expected that a proxy
class might have different relation descriptors attached to it than its
base class does.
That's why I believe that this ticket should simply be closed without
changes, though (as I said) if others feel strongly that the behavior
should be changed, I won't stand in the way.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:13>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by akaariai):
The place where "reverse relations to proxy models are only a Python
thing" breaks is deletion of models. Deletion of a model that points to a
proxy child of a concrete model will delete the concrete model, too.
Another way to think of this is that the foreign key in the database
representation is to the concrete model, hence the concrete model does
have the related model set.
Maybe we could either add a Meta method to get reverse relations from
proxy children, or just document how to do this properly. This way, if you
need to get the proxy children, there is some way to actually do that.
Still, a generic solution to this problem might be something along the
lines:
{{{
fk = models.ForeignKey(ConcreteModel,
reverse_queryset=ProxyModel.objects.all())
}}}
This is a new thing to learn. But reverse_queryset would have other uses,
too. You could for example use a custom queryset with custom methods, add
annotations, order_by and so on to it. If you filter in the
reverse_queryset, then there might be problems, but we could just document
that you shouldn't do that. This would also fix
Manager.use_for_related_fields flag issues (the flag doesn't work at all
consistently).
All this being said I don't feel at all strongly about this issue, and if
nobody else does, then we should default to not change existing behavior.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:14>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
Replying to [comment:14 akaariai]:
> The place where "reverse relations to proxy models are only a Python
thing" breaks is deletion of models. Deletion of a model that points to a
proxy child of a concrete model will delete the concrete model, too.
Deletion also cascades up MTI inheritance chains, but we don't hoist the
reverse descriptor for FKs to MTI child models onto the parent model. Do
you see something "broken" there also? I don't.
I don't think these two things (cascade deletion and the Python class on
which reverse descriptors live) are connected in the way you're
suggesting. The mental model for both types of inheritance (MTI and proxy)
is that any instance in a given inheritance chain really represents the
"same object" -- the non-leaf instances are just generic / incomplete
representations of that object. So naturally, if you delete that thing
it's gone (and of course that includes its incomplete representations).
There's nothing inconsistent with that mental model (or with the deletion
behavior, in either the proxy or MTI case) in having a reverse FK
descriptor on a child class that isn't there on the parent; it's just one
of (possibly many) ways in which a parent instance is an incomplete
representation of the full object.
> Still, a generic solution to this problem might be something along the
lines:
> {{{
> fk = models.ForeignKey(ConcreteModel,
reverse_queryset=ProxyModel.objects.all())
> }}}
> This is a new thing to learn. But reverse_queryset would have other
uses, too. You could for example use a custom queryset with custom
methods, add annotations, order_by and so on to it. If you filter in the
reverse_queryset, then there might be problems, but we could just document
that you shouldn't do that.
I'm not necessarily opposed to a feature like this if it has other uses
that can stand on their own to justify the feature, but for the specific
case of having an FK return proxy model instances, I think this API is a
step backwards in usability from just pointing the FK at the proxy model,
which already works fine.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:15>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by akaariai):
My example wasn't that great, sorry for that.
What I'm after is that if we add proxy relations to the parent models we
don't break anything (except for backwards incompatibility in the Meta
API). The same isn't true for MTI inheritance, where a lot would break if
we added child model relations to parent models.
We do support an inheritance chain of Concrete -> ProxyConcrete ->
ProxyConcreteChild. Interestingly this chain actually creates a foreign
key from ProxyConcreteChild directly to Concrete in the model definition.
This is inconsistent, the foreign key should be to ProxyConcrete. But we
can't do that, as then
Concrete.objects.select_related('proxyconcretechild') wouldn't work, as
Concrete doesn't have the reverse descriptor for the relation from
ProxyConcreteChild to ProxyConcrete.
Also, my reverse_queryset example is broken. This assumes you want to get
the *reverse* relation as proxy instances, whereas the foreign key to
proxy feature is about getting the *direct* relation as proxy instances.
While maybe an useful feature, it doesn't change anything for this ticket.
Instead we should have fk = models.ForeignKey(ConcreteModel,
fetch_as=ProxyModel) or something like that. This might actually make some
sense if done automatically. The user writes ForeignKey(ProxyModel),
internally Django interprets it as ForeignKey(ConcreteModel,
fetch_as=ProxyModel). But I guess we do have more important problems to
solve.
Even if I see a slight advantage in publishing reverse relations to
concrete parents, I guess I'm slightly in favor of closing this one and
moving on to something more productive.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:16>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
Replying to [comment:16 akaariai]:
> What I'm after is that if we add proxy relations to the parent models we
don't break anything (except for backwards incompatibility in the Meta
API). The same isn't true for MTI inheritance, where a lot would break if
we added child model relations to parent models.
Yep, agreed. That's why, as I said above, I'm not strongly opposed to this
ticket; -0 at worst. Just don't think there's much wrong with the status
quo either.
> We do support an inheritance chain of Concrete -> ProxyConcrete ->
ProxyConcreteChild. Interestingly this chain actually creates a foreign
key from ProxyConcreteChild directly to Concrete in the model definition.
This is inconsistent, the foreign key should be to ProxyConcrete. But we
can't do that, as then
Concrete.objects.select_related('proxyconcretechild') wouldn't work, as
Concrete doesn't have the reverse descriptor for the relation from
ProxyConcreteChild to ProxyConcrete.
Yeah, that is slightly odd. But given that the FK (OneToOne, really,
right?) in this case is an internal implementation detail that the user
isn't likely to ever need to follow directly, it doesn't seem like much of
a problem.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:17>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
Looks like #25505 might be related to the o2o to proxy model scenario
described by Anssi.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:18>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jmurty):
* cc: james@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:19>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
This also fixes #14887 which I closed as a duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:20>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by akaariai):
Seems like there is something to gain from propagating reverse descriptors
to concrete parents. Notably we aren't deprecating foreign keys to proxy
models. I don't see any big downsides in propagating the descriptors. So,
I move to +1 camp for this change.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:21>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
If it fixes bugs, +1 from me.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:22>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_docs: 0 => 1
Comment:
#24762 is duplicate.
Marking as "Needs docs" for the release notes that are needed.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:23>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* needs_docs: 1 => 0
* version: 1.4 => master
Comment:
Just added the required release notes.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:24>
Django
Reporter: akaariai | Owner: charettes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:25>
Django
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: charettes
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette <charette.s@…>):
In [changeset:"5b980897f2da3c048d88029af554e0fc4be68a8e" 5b980897]:
{{{
#!CommitTicketReference repository=""
revision="5b980897f2da3c048d88029af554e0fc4be68a8e"
Refs #18012 -- Made proxy and concrete model reverse fields consistent.
Prior to this change proxy models reverse fields didn't include the
reverse fields pointing to their concrete model.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:28>
Django
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: charettes
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette <charette.s@…>):
In [changeset:"a82e21b0f9ac613db3354d46539cb97c897ea663" a82e21b]:
{{{
#!CommitTicketReference repository=""
revision="a82e21b0f9ac613db3354d46539cb97c897ea663"
Refs #18012 -- Removed the now unused proxied_children model option.
Thanks to Tim for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:29>
Django
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: charettes
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"6c9f37ea9eafb0ca1b02eb71ae8d375672043824" 6c9f37e]:
{{{
#!CommitTicketReference repository=""
revision="6c9f37ea9eafb0ca1b02eb71ae8d375672043824"
Fixed #18012 -- Propagated reverse foreign keys from proxy to concrete
models.
Thanks to Anssi for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:26>
Django
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: charettes
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette <charette.s@…>):
In [changeset:"8bdfabed6563f2ae136ad43e05bb254c9c15811a" 8bdfabed]:
{{{
#!CommitTicketReference repository=""
revision="8bdfabed6563f2ae136ad43e05bb254c9c15811a"
Refs #18012 -- Removed special casing for proxy models deletion.
This isn't required anymore now that reverse foreign keys
from proxy models are propagated to their concrete model.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:27>
Django
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: charettes
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette <charette.s@…>):
In [changeset:"63f0e2df2a3af7b7225150a4ba6d63985b405855" 63f0e2d]:
{{{
#!CommitTicketReference repository=""
revision="63f0e2df2a3af7b7225150a4ba6d63985b405855"
Refs #18012 -- Accounted for reverse proxy relations in migrations.
Thanks to Markus for the suggestion and Tim for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:30>
Django
-------------------------------------+-------------------------------------
Type: | Charette
Cleanup/optimization | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Shai Berger):
For the record and for future considerations, this did carry a backwards
incompatibility. Consider:
{{{
class User(models.Model):
pass # Really all sorts of details, which do not matter
class Teacher(User):
class Meta:
proxy=True
class Student(User):
class Meta:
proxy=True
class Class(models.Model):
teacher = models.ForeignKey(Teacher, related_name='classes')
students = models.ManyToMany(Student, related_name='classes')
}}}
This works before this was fixed; after the fix, the related name
`'classes'` gets propagated to `User` twice, and it breaks.
(I admit that this design is somewhat flawed, but it used to be valid, and
it isn't anymore)
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:31>
Django
-------------------------------------+-------------------------------------
Reporter: Anssi Kääriäinen | Owner: Simon
Type: | Charette
Cleanup/optimization | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by steve yeago):
Without M2Ms this feature seems kind of incomplete, since an M2M from a
proxy model is basically the same thing, except that you lose a lot of
related-name accessors, admin inline support, perhaps some other things.
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:32>
Django
-------------------------------------+-------------------------------------
Reporter: Anssi Kääriäinen | Owner: Simon
Type: | Charette
Cleanup/optimization | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ivan):
{{{
class Worker(models.Model):
fio = models.CharField(u'ФИО', unique=True, max_length=250)
active = models.BooleanField(default=True, help_text="Активный")
def __str__(self):
return self.fio
class WorkerActive(Worker):
def __str__(self):
if not self.active:
return "%s (Не кативен)" % (self.fio,)
return self.fio
class Meta:
proxy = True
class License(models.Model):
target_workers = models.ManyToManyField(WorkerActive,
verbose_name="Покупалась для", blank=True)
def __str__(self):
return u"License for %s (%s)" % (self.soft.name, self.number or
'')
}}}
I have error (1054, "Unknown column
'inventory_license_target_workers.workeractive_id' in 'field list'")
WHY!?
I believe that this is a gross error of the concept of using Proxy models
--
Ticket URL: <https://code.djangoproject.com/ticket/18012#comment:33>