Code

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#10251 closed (fixed)

Problem with inheritance plus explicit primary_key in child model

Reported by: kmtracey Owned by: mtredinnick
Component: Database layer (models, ORM) Version: 1.0
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Given these models:

class BaseM(models.Model):
  base_name = models.CharField(max_length=100)
  def __unicode__(self):
    return self.base_name

class DerivedM(BaseM):
  customPK = models.IntegerField(primary_key=True)
  derived_name = models.CharField(max_length=100)
  def __unicode__(self):
    return "PK = %d, base_name = %s, derived_name = %s" % \
        (self.customPK, self.base_name, self.derived_name) 

The ORM can't seem to properly link parent with child models (reformatted for readability):

Python 2.5.1 (r251:54863, Jul 31 2008, 23:17:40)
[GCC 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from ttt.models import DerivedM
>>> from django.db import connection
>>> DerivedM.objects.create(customPK=44, base_name="b1", derived_name="d1")

<DerivedM: PK = 44, base_name = b1, derived_name = d1>
>>> DerivedM.objects.all()
[]
>>> connection.queries[-1]

{'time': '0.006',
'sql': u'SELECT
          `ttt_basem`.`id`,
          `ttt_basem`.`base_name`,
          `ttt_derivedm`.`basem_ptr_id`,
          `ttt_derivedm`.`customPK`,
          `ttt_derivedm`.`derived_name`
       FROM `ttt_derivedm` INNER JOIN `ttt_basem`
       ON (`ttt_derivedm`.`customPK` = `ttt_basem`.`id`) LIMIT 21'}

This was originally tracked down in this thread but the OP doesn't seem interested in pursuing:

http://groups.google.com/group/django-users/browse_thread/thread/5ee5d5dacb16b90/b133b55a9a92520d

Seems to me either an error should be reported on trying to override primary_key in the child model, or the parent-child link should work.

Attachments (0)

Change History (7)

comment:1 Changed 5 years ago by mtredinnick

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

I wouldn't cry if we made this an error.

However, since multi-inheritance is permitted, it's clearly not a requirement of the internals that all relations to parent models are primary keys. So maybe we can accommodate this.

comment:2 Changed 5 years ago by Alex

I'm of the opinion that the value in supporting this is severely out weighed by the value in not supporting it, specifically if we don't support it we can do nifty optimizations like what I posted in #9394 all the time. As best I can tell the only point of supporting this would essentially be for the model to have 2 primary keys(2 not null unique fields) you can still keep the extra field, put an index on it, whatever, but giving it the primary key semantic is more trouble than it's worth.

comment:3 Changed 5 years ago by mtredinnick

You can't have two primary keys on a database table (by definition). However, we already support non-primary-key links to parent models (due to supporting multiple inheritance), so that's why this probably should just work. It's just an oversight in an edge-case and should be relatively simple to fix. This isn't the place to advocate for the "nifty optimisation" in the other ticket. That's independent of this.

comment:4 Changed 5 years ago by Alex

I'm not advocating the optimization here(well I suppose I am, but it's incidental) the point is optimizations of that nature are easier to do if this is disallowed, because otherwise we have to trace the entire chain of pks and find if any of them aren't actually the parent link.

comment:5 Changed 5 years ago by mtredinnick

  • Owner changed from nobody to mtredinnick

comment:6 Changed 5 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(In [9970]) Fixed #10251 -- Fixed model inheritance when there's also an explicit pk field.

comment:7 Changed 5 years ago by mtredinnick

(In [9972]) [1.0.X] Fixed #10251 -- Fixed model inheritance when there's also an explicit pk field.

Backport of r9970 from trunk.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.