Opened 6 years ago

Last modified 4 years ago

#12002 new New feature

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 vzima 5 years ago.
Diff to delete correct ManyToMany relations

Download all attachments as: .zip

Change History (11)

comment:1 Changed 6 years ago by vlastimil.zima@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 Changed 6 years ago by thejaswi_puthraya

  • Component changed from Uncategorized to Database layer (models, ORM)

comment:3 Changed 6 years ago by parxier

  • Cc bas@… added

comment:4 Changed 5 years ago by russellm

  • Triage Stage changed from Unreviewed to Design 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 Changed 5 years ago by vzima

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.

Changed 5 years ago by vzima

Diff to delete correct ManyToMany relations

comment:6 Changed 5 years ago by shauncutts

  • 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 Changed 5 years ago by russellm

  • Triage Stage changed from Design decision needed to Accepted

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 Changed 4 years ago by julien

  • Severity set to Normal
  • Type set to New feature

comment:9 Changed 4 years ago by kmtracey

  • Easy pickings unset

#15993 reported this again.

comment:10 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

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