Opened 8 years ago

Closed 8 years ago

#26703 closed Bug (duplicate)

field automatically choosen as primary_key (primary_key=False do not work)

Reported by: Nicolas BREMOND Owned by: nobody
Component: Database layer (models, ORM) Version: 1.9
Severity: Normal Keywords: multi-table inheritance, primary_key, OneToOneField
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I tried to use multi-table inheritance with a OneToOne field to the parent class in the child class but this field seems to be detected as primary_key by Django (there is only one row in the database).

The field has the same type as the parent link automatically created by Django and
the doc on inheritance tells that we can choose to use a such field for the inheritance relation with parent_link=True but I need the inheritance relation AND my other relation.
I tried to add parent_link=False but this don't change anything.
If I add null=True I got an error :

SystemCheckError: System check identified some issues:

ERRORS:
testapp.A.other: (fields.E007) Primary keys must not have null=True.
	HINT: Set null=False on the field, or remove primary_key=True argument.
from django.db import models

So I tried to add primary_key=False but that don't change anything.

There is a minimal code wich reproduce the error

models.py :

class Base(models.Model):
    pass

class A(Base):
    other=models.OneToOneField(Base,
            parent_link=False,#do not change anything
            primary_key=False,#do not change anything
            null=True,#raise a SystemCheckError
            related_name='other_base',
            )

I can continue my project if I replace the OneToOneField by a ForeignKey with unique=True but I have a warning with a hint to replace the ForeignKey by a OneToOneField :

System check identified some issues:

WARNINGS:
testapp.A.other: (fields.W342) Setting unique=True on a ForeignKey has the same effect as using a OneToOneField.
	HINT: ForeignKey(unique=True) is usually better served by a OneToOneField.

Change History (7)

comment:1 by Tim Graham, 8 years ago

Is this a duplicate of #26320?

comment:2 by Simon Charette, 8 years ago

Resolution: duplicate
Status: newclosed

I'm fairly confident it is.

During the deprecation period you'll have to declare both OneToOneFields:

class A(Base):
    base_ptr = models.OneToOneField(Base, parent_link=True, primary_key=True)
    other = models.OneToOneField(Base, null=True, related_name='other_base')

Where the base_ptr field should be the named after the lower cased name of the parent-linked base followed by the '_ptr' suffix.

comment:3 by Simon Charette, 8 years ago

@timgraham, It makes me wonder if we should adjust the deprecation warning raised on implicit non-parent_link OneToOneField promotion to mention either to add parent_link=True or how to declare the usual implicit parent_link as the actions to take in order to have both fields available is non-trivial from the deprecation message.

For example, in the reported case the message would be:

Implicit parent link is deprecated, either add parent_link=True to A.other or declare base_ptr = models.OneToOneField(Base, parent_link=True, primary_key=True) before it during the deprecation period

comment:4 by Tim Graham, 8 years ago

Seems like it wouldn't hurt.

comment:5 by Nicolas BREMOND, 8 years ago

Yes, this is a duplicate of #26320, sorry…
Thanks for the help !

comment:6 by Nicolas BREMOND, 8 years ago

Resolution: duplicate
Status: closednew

I don't know if I have to reply here, on #26320 or on a new ticket, but the solution given by charette on comment:2 does not really work.

The problem on fields is solved but I think there is still a bug during objects instanciation.

#models.py
from django.db import models

class Base(models.Model):
    def __str__(self):#indicate if the object is a member of a subclass
        try:
            a=self.a
            return "a A pk="+str(self.pk)+" (other="+str(a.other.pk)+")"
        except A.DoesNotExist:
            pass
        try:
            z=self.z
            return "not a A but a Z pk="+str(self.pk)
        except Z.DoesNotExist:
            pass
        return "not a A neither a Z pk="+str(self.pk)

class A(Base):
    base_ptr = models.OneToOneField(Base, parent_link=True, primary_key=True)
    other = models.OneToOneField(Base, related_name='other_base') #I do not have null=True

class Z(Base):
    pass

I can manipulate the Z objects :
adding a new Z :

>>> z=Z()
>>> z.save()
>>> Base.objects.all()
[<Base: not a A but a Z pk=1>]

if I try to do the same thing with a A (with a new database):

>>> other=Base()
>>> other.save()
>>> a=A(other=other)
>>> a.save()
>>> a
<A: not a A neither a Z pk=None> #a is supposed to have a pk which is not None 
>>> a.other
<Base: a A pk=1 (other=1)>#other is supposed to be a base and not a A
>>> Base.objects.all()
[<Base: a A pk=1 (other=1)>]#I was expecting two objects

It seems to be possible to change a Base to a A:

>>> other=Base()
>>> other.save()
>>> b=Base()
>>> b.save()
>>> Base.objects.all()
[<Base: not a A neither a Z pk=1>, <Base: not a A neither a Z pk=2>]
>>> a=A(other=other,base_ptr=b)
>>> a.save()
>>> a
<A: not a A neither a Z pk=2>#str(a) should return "a A" but this work if we reload the object
>>> a.other
<Base: not a A neither a Z pk=1>
>>> Base.objects.all()
<Base: not a A neither a Z pk=1>, <Base: a A pk=2 (other=1)>]

This is maybe also deprecated but it can be a bug in the next version and I didn't see a fix in PR (I quickly look the pull request, maybe I'm wrong)

comment:7 by Tim Graham, 8 years ago

Resolution: duplicate
Status: newclosed

Yes, a new ticket is more appropriate. #9982 may be related.

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