Opened 14 years ago

Closed 9 years ago

#12002 closed New feature (invalid)

Models inherited from multiple Models

Reported by: vlastimil.zima@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.1
Severity: Normal Keywords:
Cc: bas@…, shaunc@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In case Model is created as inherited from multiple non-abstract Models there is a problem with automatic primary keys at children model save. Especially when second parent has one more non-proxy children.

Assume we have following models:

class Place(models.Model):
  class Meta:
    abstract = False
    proxy = False
...
class Staff(models.Model):
  class Meta:
    abstract = False
    proxy = False
...
class Restaurant(Place, Staff):
  class Meta:
    abstract = False
    proxy = False
...
class OtherStaff(Staff):
  class Meta:
    abstract = False
    proxy = False
...

where Place and Staff has automatic primary keys:

id = models.AutoField(primary_key = True)

Then on saving new instance of Restaurant the following happens:

  • Restaurant searches for its parents and finds out Place and Staff
  • While inserting new Place instance, it comes out that it has no primary key ("id"), so it is inserted into database with automatic one and primary key is returned
  • Discovered primary key of new Place instance is saved to Restaurant instance under attribute name of Place primary key ("id")
  • Then new Staff instance is inserted. It looks whether it has a primary key and it finds out primary key of Place instance, because it has same attribute name for primary key ("id")
  • New Staff instance is saved with primary key of Place instance instead its own (Hidden problem: If Staff with same primary key already exists then values of that Staff are updated with values from new Restaurant instance)

Following attributes will always be equal: Restaurant.id, Restaurant.place_ptr_id, Restaurant.staff_ptr_id, Place.id, Staff.id

Definitely crashes in case Staff has another children e.g. OtherStaff, then on saving new instance of OtherStaff:

  • OtherStaff searches for its parents and finds out Staff
  • While inserting new Staff instance, it comes out that it has no primary key ("id"), so it is inserted into database with automatic one
  • In database a next primary key from sequence is found out and Staff is supposed to be inserted with that primary key
  • CRASH because Staff with that primary key already exist (created while saving Restaurant, but with specified primary key, so sequence was not updated)

Same situation happened in every case Model parents (non-abstract) has same attribute name for their primary keys and second one has at least one more non-proxy children Model.

I consider this as a bug although it has easy solution (rename attribute name of primary key), because I found no evidence that this situation can happen and therefore it should be considered. Also I see no reason for use in current state.

I my opinion the best (and easiest) solution is Exception raised in case parent classes has same attribute name for their primary key. A way that will solve this entirely and allowed situation that parents can have same attribute name of primary keys would be great, but it would be probably much too complicated.

Attachments (1)

subqueries.diff (669 bytes ) - added by Vlastimil Zíma 14 years ago.
Diff to delete correct ManyToMany relations

Download all attachments as: .zip

Change History (12)

comment:1 by vlastimil.zima@…, 14 years ago

I found another problem with this type of inheriting which is primary keys for ManyToManyFields.

Assume we have following models:

class Place(models.Model):
  class Meta:
    abstract = False
    proxy = False
...
class Staff(models.Model):
  users = models.ManyToManyField(User)

  class Meta:
    abstract = False
    proxy = False
...
class Restaurant(Place, Staff):
  class Meta:
    abstract = False
    proxy = False
...

Every instance of Restaurant has a users property inherited from Staff. But Restaurant.users.all() will not return User instances connected to this Restaurant's Staff, but User instances connected to Staff with same primary key as Restaurant (which is same as Place primary key).

I solved this problem by overriding method __get__ of ReverseManyRelatedObjectsDescriptor class to correct its return value (Manager class instance) properties _pk_val (for correct saving) and core_filters (for correct loading). Then override method contribute_to_class of ManyToManyField class and replace appropriate attribute with modified descriptor.

General solution would be if ManyRelatedObjects descriptors will not use primary key of builded class but primary value of class which defines that field.

This problem would probably happen for OneToOneField and ForeignKey as well.

comment:2 by Thejaswi Puthraya, 14 years ago

Component: UncategorizedDatabase layer (models, ORM)

comment:3 by Vasily Ivanov, 14 years ago

Cc: bas@… added

comment:4 by Russell Keith-Magee, 14 years ago

Triage Stage: UnreviewedDesign decision needed

I'm not sold on the idea that multiple-inheritance should be possible. To my mind, raising an error if there are multiple non-abstract model base classes is the right response here. Requires discussion on django-dev.

comment:5 by Vlastimil Zíma, 14 years ago

Another problem have been found with ManyToManyField for Models with multiple parents:

when deleting objects with multiple parent (e.g. Restaurant) objects related via many-to-many relationship are deleted twice - once for each primary key of parent object (e.g. Staff with pk equal to Restaurant.place_ptr_id and Staff with pk equal to Restaurant.staff_ptr_id). That breaks connections which should not be removed.

For Restaurant with place_ptr_id = 1 and staff_ptr_id = 2 django generates following SQLs:

...
-- one that is not correct !
DELETE FROM "app_staff_users" WHERE "staff_id" IN (1)
-- and one that is correct
DELETE FROM "app_staff_users" WHERE "staff_id" IN (2)
...

Error is generated in [django/db/models/sql/subqueries.py] on line 69, where pk_list is given through instead of generating correct primary keys that are involved in relation.

I append diff that solves this issue and regenerates primary keys before given forward.

by Vlastimil Zíma, 14 years ago

Attachment: subqueries.diff added

Diff to delete correct ManyToMany relations

comment:6 by Shaun Cutts, 14 years ago

Cc: shaunc@… added

A workaround I have used (but with some discomfort -- and it probably works only on some backends) is to change the database default of the id fields of all the base classes to get their next value from the same database sequence object.

comment:7 by Russell Keith-Magee, 14 years ago

Triage Stage: Design decision neededAccepted

As pointed out on #13781 -- the decision has already been made, and at least partially implemented. Multiple model inheritance is apparently ok, but buggy.

comment:8 by Julien Phalip, 13 years ago

Severity: Normal
Type: New feature

comment:9 by Karen Tracey, 13 years ago

Easy pickings: unset

#15993 reported this again.

comment:10 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:11 by Tim Graham, 9 years ago

Resolution: invalid
Status: newclosed

The models in the description no longer validate due to clashing id fields (#17673). If there is another problem described in this ticket, please open a new one.

Note: See TracTickets for help on using tickets.
Back to Top