[Django] #17002: ManyToManyField through a model which extends some other model
Django
----------------------------------------------+--------------------
Reporter: mitar | Owner: nobody
Type: New feature | Status: new
Component: Database layer (models, ORM) | Version: 1.3
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
It seems that currently it is impossible to create a ManyToManyField
through a model which extends some other model. Django assumes that all
fields are accessible in the given model and does not traverse the model
hierarchy. This means that it is impossible to use inheritance to prevent
data (and code) duplication and have a normalized database.
--
Ticket URL: <https://code.djangoproject.com/ticket/17002>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: mitar | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: SVN
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Design
Keywords: | decision needed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by sebastian):
* needs_better_patch: => 0
* needs_tests: => 0
* version: 1.3 => SVN
* needs_docs: => 0
* type: New feature => Bug
* stage: Unreviewed => Design decision needed
Comment:
Django expects the foreign key attributes to be present in the
intermediary model given as the `through=` argument (and not some non-
abstract parent) when constructing the related queries. On the other hand,
model validation and table creation works even when the foreign key
attributes are in the base class, or split over base and derived class as
in e.g.
{{{
class A(models.Model):
pass
class B(models.Model):
a_s = models.ManyToManyField(A, through='Derived')
class Base(models.Model):
a = models.ForeignKey(A)
class Derived(Base):
b = models.ForeignKey(B)
}}}
The example validates just fine and even generates the appropriate
database structure, but related queries fail during runtime, e.g.
`B.objects.get(pk=1).a_s.all()` throws "DatabaseError: no such column:
app_derived.a_id".
Can you give an example where foreign keys in the base class of the
intermediary model (as opposed to arbitrary attributes, or methods) are
necessary or would be useful?
In any case, I think the behavior here should be consistent. If the query
cannot be created (because attributes are expected to be present in the
derived table), then the validation of the model should not succeed. Or
otherwise, the query should take attributes imported from base classes
into account and insert the necessary joins.
--
Ticket URL: <https://code.djangoproject.com/ticket/17002#comment:1>
Django
Reporter: mitar | Owner: nobody
Component: Database layer | Version: SVN
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Design
Keywords: | decision needed
Has patch: 0 | Needs documentation: 0
Comment (by mitar):
The concrete example where we discovered this is when we wanted to make a
schema other users could extend. For exmaple, we have defined something
like;
{{{
class Node(models.Model):
children = models.ManyToManyField(Child, through='Link')
class Child(models.Model):
pass
class Link(models.Model):
source = models.ForeignKey(Node)
destination = models.ForeignKey(Child)
}}}
And then we wanted to provide users a way to extend our schema with their
own applications/modules. So that they can instead derive their own
schema, like:
{{{
class BetterNode(Node):
better_children = models.ManyToManyField(Child, through='BetterLink')
class BetterLink(Link):
weight = models.IntegerField()
}}}
And then old applications could still use the old fields, while users'
extended applications could know how to handle this additional fields.
--
Ticket URL: <https://code.djangoproject.com/ticket/17002#comment:2>
Django
Reporter: mitar | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by Alex):
* stage: Design decision needed => Accepted
Comment:
Yeah this is a bug (at the very minimum a docs one, although I think it's
definitely a "true" bug), I'm not sure how much of a pain it'll be to fix
off hand.
--
Ticket URL: <https://code.djangoproject.com/ticket/17002#comment:3>
Django
Reporter: mitar | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by InvalidInterrupt):
* cc: gm.lawr@… (added)
* version: master => 1.9
Comment:
I recently encountered this bug on a project I am working on.
I think that we may be able to fix the issue by changing
{{{django.db.models.fields.related.ManyToManyField._get_path_info}}} to
use some logic from {{{ django.db.models.sql.query.Query.names_to_path}}}
(lines 1334-1349 in commit 698be78). To keep things maintainable, I'd like
to split that logic out into a separate function (e.g.
{{{get_path_to_parent_model(model, parent)}}}). Could someone with more
familiarity with the codebase please suggest an appropriate place for this
function to live?
--
Ticket URL: <https://code.djangoproject.com/ticket/17002#comment:4>
Django
Reporter: mitar | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Possibly `django/db/models/query_utils.py`, but it'll be easier to tell
upon seeing the pull request so don't worry too much about it now.
--
Ticket URL: <https://code.djangoproject.com/ticket/17002#comment:5>
Django
Reporter: mitar | Owner:
| InvalidInterrupt
Type: Bug | Status: assigned
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by InvalidInterrupt):
* status: new => assigned
* owner: nobody => InvalidInterrupt
--
Ticket URL: <https://code.djangoproject.com/ticket/17002#comment:6>
Django
Reporter: mitar | Owner:
| InvalidInterrupt
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by InvalidInterrupt):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* version: 1.9 => master
Comment:
Patch work is occurring on my topic branch at
[https://github.com/InvalidInterrupt/django/tree/ticket_17002]. I need to
add tests to confirm proxy models are handled correctly.
--
Ticket URL: <https://code.djangoproject.com/ticket/17002#comment:7>
Django
Reporter: mitar | Owner:
| InvalidInterrupt
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by InvalidInterrupt):
* needs_better_patch: 1 => 0
Comment:
I've run the test suite for the py3, py2, and py3-posgtres environments
successfully.
--
Ticket URL: <https://code.djangoproject.com/ticket/17002#comment:8>
Django
Reporter: mitar | Owner:
| InvalidInterrupt
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by InvalidInterrupt):
* needs_better_patch: 0 => 1
Comment:
Found that prefetch_related is also broken.
--
Ticket URL: <https://code.djangoproject.com/ticket/17002#comment:9>
Django
Reporter: mitar | Owner:
| InvalidInterrupt
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by InvalidInterrupt):
* needs_better_patch: 1 => 0
Comment:
[https://github.com/django/django/pull/7130 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/17002#comment:10>
Django
Reporter: Mitar | Owner:
| InvalidInterrupt
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
Comment:
I left some comments for improvement. Please uncheck "Patch needs
improvement" after updating the pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/17002#comment:11>
Django
Reporter: Mitar | Owner:
| InvalidInterrupt
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by InvalidInterrupt):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/17002#comment:12>
Django
Reporter: Mitar | Owner:
| InvalidInterrupt
Type: Bug | Status: assigned
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 Tim Graham):
* stage: Accepted => Ready for checkin
Comment:
I've made a few cosmetic edits to the PR and plan to merge this after
#27579 as I've used part of that patch .
--
Ticket URL: <https://code.djangoproject.com/ticket/17002#comment:13>
Django
Reporter: Mitar | Owner:
| InvalidInterrupt
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"98359109eb0ed68a5821476bcd797455723aaaba" 9835910]:
{{{
#!CommitTicketReference repository=""
revision="98359109eb0ed68a5821476bcd797455723aaaba"
Fixed #17002 -- Allowed using a ManyToManyField through model that
inherits another.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/17002#comment:14>