Primary key not set correctly for concrete->abstract->concrete model inheritance.

[Aram Dulyan]

When a concrete model inherits from an abstract model which in turn inherits from a concrete model, the "primary_key" attribute on the OneToOneField in the child model is not set, and the SQL definition for that model does not define a primary key. However, any subsequent children inheriting from the abstract class will have a correctly set primary key. For example:

class ConcreteParent(models.Model):
	concrete_field = models.TextField()

class AbstractParent(ConcreteParent):
	abstract_field = models.TextField()
	
	class Meta:
		abstract = True

class FirstChild(AbstractParent):
	child_field = models.TextField()

class SecondChild(AbstractParent):
	child_field = models.TextField()

The output of "sqlall" for these definitions is:

CREATE TABLE "testapp_concreteparent" (
    "id" serial NOT NULL PRIMARY KEY,
    "concrete_field" text NOT NULL
);
CREATE TABLE "testapp_firstchild" (
    "concreteparent_ptr_id" integer NOT NULL UNIQUE REFERENCES "testapp_concreteparent" ("id") DEFERRABLE INITIALLY DEFERRED,
    "abstract_field" text NOT NULL,
    "child_field" text NOT NULL
);
CREATE TABLE "testapp_secondchild" (
    "concreteparent_ptr_id" integer NOT NULL PRIMARY KEY REFERENCES "testapp_concreteparent" ("id") DEFERRABLE INITIALLY DEFERRED,
    "abstract_field" text NOT NULL,
    "child_field" text NOT NULL
);

Note the absence of "PRIMARY KEY" in the first child and its presence in the second child.

We can also go in the shell to see what's happening:

>>> FirstChild._meta.get_field_by_name('concreteparent_ptr')[0].primary_key
False
>>> SecondChild._meta.get_field_by_name('concreteparent_ptr')[0].primary_key
True

>>> FirstChild._meta.get_field_by_name('concreteparent_ptr')[0] == FirstChild._meta.pk
True
>>> FirstChild._meta.get_field_by_name('concreteparent_ptr')[0] is FirstChild._meta.pk
False

>>> SecondChild._meta.get_field_by_name('concreteparent_ptr')[0] == SecondChild._meta.pk
True
>>> SecondChild._meta.get_field_by_name('concreteparent_ptr')[0] is SecondChild._meta.pk
True

I believe what is happening is that in db.models.options.py, when initialising a Child object in Options._prepare(), the first parent link is selected to be the primary key, its primary key attribute is set to True, and it is then passed to setup_pk(). However, the field fetched through the parent link is the field on the AbstractParent model, and is thus not properly registered with the Child model. But in this process, its primary_key attribute is set to True, which allows the second Child to set it as its PK on the initial add_field() call.

The simplest fix for this is to initialise the OneToOneField in ModelBase.__new__() with primary_key=True. Alternately, and perhaps preferably, Options._prepare() can be altered to fetch the OneToOneField for the Child model, rather than the AbstractParent model. I've attached two patches, one for each solution, and perhaps someone more qualified than me can decide which one is the better one.

[Aram Dulyan]

1st possible fix: adding a primary_key=True to the OneToOneField initialisation.

[Aram Dulyan]

2nd possible fix: fetching the field for current model when "_preparing" it.

[Aram Dulyan]

2nd (superior) fix variant, with regression tests.

[Aram Dulyan]

After doing some testing, it seems that the 1st variant of the fix breaks some tests, so I've attached a new diff that includes the 2nd variant of the fix (which doesn't break anything), along with regression tests for this bug.

[mike@…]

And this kind of bug is causing much pain with something like south migrations. Is it really scheduled to be fixed in version 1.3 only?

[Russell Keith-Magee]

"Milestone 1.3" means that it's on the schedule to be looked at in the 1.3 development timeframe. All bugs fixed in that timeframe are backported to the previous release branch.

[Daishiman]

Tested problem description with second fix. Works as advertised.

[Aram Dulyan]

Updated patch to bring test into line with the changes in the test suite.

[Julien Phalip]

The new patch applies and the whole test suite passes on sqlite.

[Russell Keith-Magee]

In [15498]:

Fixed #13987 -- Ensure that the primary key is set correctly for all models that have concrete-abstract-concrete inheritance, not just the first model. Thanks to Aramgutang for the report and patch.

[Russell Keith-Magee]

In [15499]:

[1.2.X] Fixed #13987 -- Ensure that the primary key is set correctly for all models that have concrete-abstract-concrete inheritance, not just the first model. Thanks to Aramgutang for the report and patch.

Backport of r15498 from trunk.

[Ramiro Morales]

In [15525]:

[1.2.X] Tweaked some asserts not present in older unittest or deprecated from tests added in r15499. Refs #13987

[Jacob]

Milestone 1.3 deleted