Opened 16 years ago
Last modified 11 months ago
#7623 new New feature
Multi-table inheritance: Add the ability create child instance from existing parent
Reported by: | Owned by: | ||
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | model-inheritance, multi-table-inheritance |
Cc: | brooks.travis@…, sirexas@…, anssi.kaariainen@…, mike@…, reza@…, cmawebsite@…, Aron Podrigal, kamandol, Carlos Palol, Matias Rodal, Bel-Shazzar, InvalidInterrupt, JM Barbier, Charlie Denton | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
As it exists now, multi-table inheritance does not allow for the creation of a child model instance that inherits from an existing parent model instance. For example:
Parent Class-
class Place(models.Model): name = models.CharField(max_length=50) address = models.TextField(max_length=150)
Child Classes-
class Restaurant(Place): place = models.OneToOneField(Place, parent_link=True) cuisine = models.CharField(max_length=75) rating = models.IntegerField() class Bar(Place): parent = models.OneToOneField(Place, parent_link=True) happy_hour = models.BooleanField() beers_on_tap = models.ManyToManyField("Beers", null=True, blank=True)
Sample Use-case-
When the system is first deployed, a restaurant instance is created. Later, the restaurant adds a bar to increase revenue, and we now want to create a Bar model instance for the parent Place for the restaurant. I would propose the following interface for doing so:
parentPlace = Restaurant.objects.get(name__iexact="Bob's Place").parent barInstance = Bar(parent=parentPlace, happy_hour=True)
However, if you attempt to create an instance in this manner now, you receive a DatabaseIntegrityError, saying that a Place object with that id already exists.
Attachments (1)
Change History (49)
comment:1 by , 16 years ago
comment:2 by , 16 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
by , 16 years ago
Attachment: | makechild.patch added |
---|
comment:3 by , 16 years ago
Has patch: | set |
---|---|
Patch needs improvement: | set |
comment:4 by , 16 years ago
Cc: | added |
---|
comment:5 by , 16 years ago
milestone: | → post-1.0 |
---|---|
Triage Stage: | Unreviewed → Design decision needed |
comment:6 by , 16 years ago
Cc: | removed |
---|
comment:9 by , 14 years ago
Cc: | added |
---|
comment:10 by , 14 years ago
Component: | Core framework → Database layer (models, ORM) |
---|
comment:11 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:12 by , 13 years ago
Easy pickings: | unset |
---|---|
Resolution: | → needsinfo |
Status: | new → closed |
UI/UX: | unset |
Discussion with Jacob: closing as needsinfo, the modeling in the ticket does not seem like it *should* work, a model cannot be an instance of multiple subclasses.
comment:13 by , 13 years ago
Resolution: | needsinfo |
---|---|
Status: | closed → reopened |
Here's a more reasonable example...
# models.py class Place(models.Model): name = models.CharField(max_length=50) address = models.CharField(max_length=80) class Restaurant(Place): place = models.OneToOneField(Place, parent_link=True, related_name='restaurant') serves_hot_dogs = models.BooleanField() serves_pizza = models.BooleanField()
Now, say we have a bunch of Place objects in the database, and a form where users can inform us what sort of business a particular Place is (OK, this is a contrived example, but I'm sure you can see that this could be a reasonable use case for some types of data).
# somewhere in views.py p = Place.objects.get(pk=1) restaurant = Restaurant(**{ 'place': p, 'serves_hot_dogs': False, 'serves_pizza': True, }) restaurant.save()
comment:14 by , 13 years ago
Resolution: | → wontfix |
---|---|
Status: | reopened → closed |
If multiple related objects are linked to the same object, they should have a foreign key to this object, not "extend" it; that would break the "instance" paradigm.
You can always use a Python mixin if you have related objects of different types, but want to share some methods.
comment:15 by , 13 years ago
Resolution: | wontfix |
---|---|
Status: | closed → reopened |
Please see my example immediately above. This is not about foreign keys or multiple related objects linked to the same object. It's about multi-table inheritance where the child data isn't always known at the time the parent object is created.
If you have a Place object that is not yet a Restaurant object, the ORM will not allow you to add the data to make it a Restaurant object.
comment:16 by , 13 years ago
Resolution: | → worksforme |
---|---|
Status: | reopened → closed |
Ah, I understand. This works:
p = Place.objects.get(pk=1) restaurant = Restaurant(place_ptr=p, ...)
comment:17 by , 13 years ago
Resolution: | worksforme |
---|---|
Status: | closed → reopened |
This leads to the behavior described in #11618 - the parent instance fields get overwritten.
comment:18 by , 13 years ago
Cc: | added |
---|
If you want to extend an existing instance, you need to copy all fields of the parent to the new child manually. Unfortunately this can't be done in a generic way using only public APIs. Although ._meta is semi-public and doing
child = ChildModel(**childfields) # set child field values for field in parent._meta.fields: setattr(child, field.attname, getattr(parent, field.attname)) # set child values from parent child.parent_ptr = parent.pk # not sure if this is strictly necessary, probably so...
or something along those lines should work (not tested). Now, I don't think this can be officially documented without making ._meta.fields part of the public API. I would have use for a helper method that does this.
comment:19 by , 12 years ago
Cc: | added |
---|
follow-up: 21 comment:20 by , 12 years ago
There's actually an easier workaround for this:
parent = Place.objects.get(...) child = Restaurant(place_ptr=parent,...) #Overwrites the parents fields except the primary key with None (because the parent fields in child are not auto filled) child.save() #Restores the parent fields to their previous values parent.save() #hit the database again to fill the proper values for the inherited fields child = Restaurant.objects.get(pk=parent.pk)
comment:21 by , 12 years ago
There is an even easier work around not 100% sure if it works as expected yet though:
child = Restaurant(place_ptr_id=place.pk) child.__dict__.update(place.__dict__) child.save()
Tested it myself for my use case as working.
Thanks to: http://stackoverflow.com/questions/4064808/django-model-inheritance-create-sub-instance-of-existing-instance-downcast
comment:22 by , 12 years ago
I'm not sure it covers all bases but I use:
child = Restaurant(place_ptr=place) child.save_base(raw=True)
It would be great to have an obvious and officially blessed way of doing this though.
comment:23 by , 12 years ago
Status: | reopened → new |
---|
comment:24 by , 12 years ago
Triage Stage: | Design decision needed → Accepted |
---|
I think this makes sense. Unlink child but keep the parent and link child to existing parent would both be useful in some situatios.
comment:25 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:27 by , 11 years ago
I'd like this to be implemented as well. I have the following model relationships and it's just hard to do what django expects me to do when creating objects.
Thread has 1 main post.
Thread has many other posts
Post has 0-1 parent post.
There are multiple type of Threads.
It's just easier to work with when I can create BaseThread and extend it.
Besides, there are cases where I need to switch Thread types.
If it's possible to treat the base and extended separately, I can keep the base part, and create the extension part, and connect them.
comment:28 by , 11 years ago
raw=True
doesn't always work. raw
is used for two different purposes:
The 'raw' argument is telling save_base not to save any parent models and
not to do any changes to the values before save. This is used by fixture loading.
For the purpose of this ticket, we want save_base ignore the first part but we need the second part to be done.
I think raw
argument should be split to two different arguments, as its functionality is already separated. Not as a workaround for this bug, but to make the code more readable.
comment:29 by , 11 years ago
Cc: | added |
---|
comment:30 by , 11 years ago
Hello! I am new in django. I have a problem like this.
I want to do something like this(in inheritance multi-table):
in view.py
restaurant = Restaurant(place_ptr=place)
restaurant.save()
assuming that the attributes of restaurant hold null=True.
but Restaurant(place_ptr=place) returns nothing.
My vercion of django is 1.5
They say about this? Is there any alternative?
That is not to create a new restaurant and it was clear the place, because it is very ugly.
From already thank you very much!
comment:31 by , 10 years ago
Cc: | added |
---|---|
Summary: | Multi-table inheritance does not allow linking new instance of child model to existing parent model instance. → Multi-table inheritance: create child instance from existing parent |
comment:32 by , 9 years ago
Cc: | added |
---|
comment:33 by , 9 years ago
Anything wrong with this implementation? (taken from Tom Tobin's branch referenced in this ticket)
comment:34 by , 9 years ago
It's a bit difficult to review without tests. To get a review of the API design, it's a good idea to offer a high level overview on the DevelopersMailingList.
comment:35 by , 9 years ago
Needs documentation: | unset |
---|---|
Summary: | Multi-table inheritance: create child instance from existing parent → Multi-table inheritance: Add the ability create child instance from existing parent |
Type: | Bug → New feature |
comment:36 by , 9 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:37 by , 7 years ago
Cc: | added |
---|
comment:38 by , 6 years ago
Cc: | added |
---|
comment:39 by , 6 years ago
Cc: | added |
---|
comment:40 by , 5 years ago
Cc: | added |
---|
wow, 11 years.. and it seems so simple to be able to insert a row with 1 FK and the child fields..
comment:41 by , 4 years ago
Cc: | added |
---|
comment:42 by , 3 years ago
Cc: | added |
---|
comment:43 by , 3 years ago
Cc: | removed |
---|
comment:44 by , 2 years ago
Cc: | added |
---|
comment:45 by , 14 months ago
Cc: | added |
---|
comment:46 by , 14 months ago
I'm in the process of refactoring some code to remove use of multi-table inheritance, and the ability to create these tables separately would be very useful to me as a part of the refactor.
comment:47 by , 14 months ago
I wonder if the newly introduced force_insert
feature in Django 5.0 (not released yet) that allows specifying which tables should be inserted happens to address this issue see (#30382 and a40b0103bccb8216c944188d329d8ea5eceb7e92).
To take the initially reported use case
parent = Restaurant.objects.get(name__iexact="Bob's Place").parent bar = Bar(parent=parent, happy_hour=True) bar.save(force_insert=(Bar,))
comment:48 by , 11 months ago
if this method worked:
extended_user = ExtendedUser(user_ptr_id=auth_user.pk) extended_user.__dict__.update(auth_user.__dict__) extended_user.save()
why not simply use it under the hood of django. Too easy. there must be something I've missed. I wonder what it is. do you have an idea? or maybe it's not enough to cover all scenarios.
I have a Bazaar branch available that solves this issue:
https://code.launchpad.net/~theonion/django/makechild
In short, it adds a couple of new methods to
QuerySet
/Manager
:prepare_child()
andcreate_child()
. For an existingPlace
p
that isn't currently aRestaurant
, you could do either of the following:While
create_child()
saves to the database,prepare_child()
does not; the latter allows you to further alter the instance before saving.I tried several alternate routes — supporting setting children directly from parents, supporting setting parents directly from children, a single unified
child()
method — and none worked cleanly. The unifiedchild()
method presented the issue of passing asave
flag; there was no way to avoid a collision with a field namedsave
in the keyword arguments.I'll add an initial patch here based on the Bazaar repository for now; I still need to add tests. Any patches here will lag behind the Bazaar repository, as the latter is where my actual development takes place.