Opened 6 years ago

Closed 14 months ago

#14096 closed Bug (invalid)

Insert code generated by models using multiple inheritance is incorrect and fails in postgresql

Reported by: Arthur Pemberton Owned by: nobody
Component: Database layer (models, ORM) Version: 1.2
Severity: Normal Keywords: orm
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

When a model with multiple concrete parents is saved (inserted) the parents are saved first. Currently, the first parent is saved and its primary key is also set as the primary key of the yet unsaved top model. Any parent following uses the new primary key of 'self' and explicitly inserts the PK into the parent models table

There is no reason to explicitly set the PK of any of the parent classes, the code already handles retrieval of the DB assigned PK.

While other DBs let this pass, PostgreSQL does not. The problem comes not on the offending .save() but subsequent inserts to the table, this time without an explicit PK. The DB's internal counter has not advanced, so the next chosen PK already exists and yields a database error due to duplicate of a key field.

There is no apparent need to explicitly set any of the PK on insert. I believe that changing the following code block within source:django/trunk/django/db/models/base.py#13538 , line 533 , Model.save_base(...) can solve the problem:

from

                if update_pk:
                    setattr(self, meta.pk.attname, result)

to

                if update_pk and cls == self.__class__:
                    setattr(self, meta.pk.attname, result)

I have attached a sample site ('minherit') with app ('data') and a test case in a ZIP file.

Attachments (3)

minherit.zip (7.8 KB) - added by Arthur Pemberton 6 years ago.
same site with test case
minherit.2.zip (7.8 KB) - added by Arthur Pemberton 6 years ago.
sample site, fewer models, two tests cases
base.py.patch (2.1 KB) - added by Arthur Pemberton 6 years ago.
more complete, tested patch

Download all attachments as: .zip

Change History (15)

Changed 6 years ago by Arthur Pemberton

Attachment: minherit.zip added

same site with test case

comment:1 Changed 6 years ago by Arthur Pemberton

Has patch: set

Changed 6 years ago by Arthur Pemberton

Attachment: minherit.2.zip added

sample site, fewer models, two tests cases

comment:2 Changed 6 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

(In [13579]) Fixed #14096 -- Corrected Python 2.4 syntax issue. Thanks to PaulM for the report.

comment:3 Changed 6 years ago by Russell Keith-Magee

Resolution: fixed
Status: closedreopened

Fat fingers; [13579] closed #14086.

Changed 6 years ago by Arthur Pemberton

Attachment: base.py.patch added

more complete, tested patch

comment:4 Changed 6 years ago by Russell Keith-Magee

Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

comment:5 Changed 6 years ago by Julien Phalip

Severity: Normal
Type: Bug

comment:6 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:7 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:8 Changed 4 years ago by Aymeric Augustin

Status: reopenednew

comment:9 Changed 2 years ago by la_iscla@…

I'm experiencing the same issue here. Annoying. Any chance to see this fixed?

comment:10 Changed 2 years ago by Tim Graham

Update the patch and add a test if you'd like to get it fixed.

comment:11 Changed 2 years ago by la_iscla@…

Well, I've made a workaround with Postgres by having the model tables rely on the same SEQUENCE counter:

ALTER TABLE mymodule_a ALTER COLUMN id SET DEFAULT nextval('mymodule_c_id_seq');

I'll dig further into this issue whenever I have more time.

comment:12 Changed 14 months ago by Tim Graham

Resolution: invalid
Status: newclosed

Closing as invalid given the provided models don't validate:

data.Company: (models.E005) The field 'id' from parent model 'data.account' clashes with the field 'id' from parent model 'data.contact'.
data.Employee.company: (models.E006) The field 'company' clashes with the field 'company' from model 'data.contact'.
Note: See TracTickets for help on using tickets.
Back to Top