Opened 4 years ago

Last modified 14 months ago

#18174 new Bug

Model inheritance pointers doesn't refer to parent to refer to grandparents

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

Description

I Create the Following Models:

class Base1( models.Model ):
  b1_id = models.AutoField( primary_key=True )
  b1_desc = models.CharField( max_length=100 )

class Base2( models.Model ):
  b2_id = models.AutoField( primary_key=True )
  b2_desc = models.CharField( max_length=100 )

class Base3( models.Model ):
  b3_id = models.AutoField( primary_key=True )
  b3_desc = models.CharField( max_length=100 )

class Middle( Base1, Base2, Base3 ):
  m_desc = models.CharField( max_length=100 )

class Top( Middle ):
  t_desc = models.CharField( max_length=100 )

I run this:

d = Top1.objects.all()
print d.query

I get:

SELECT "Test_base1"."b1_id", "Test_base1"."b1_desc", "Test_base2"."b2_id", "Test_base2"."b2_desc", "Test_base3"."b3_id", "Test_base3"."b3_desc", "Test_middle"."base3_ptr_id", "Test_middle"."base2_ptr_id", "Test_middle"."base1_ptr_id", "Test_middle"."m_desc", "Test_top1"."middle_ptr_id", "Test_top1"."t1_desc" 
  FROM "Test_top1"
  INNER JOIN "Test_base1" ON ("Test_top1"."middle_ptr_id" = "Test_base1"."b1_id")
  INNER JOIN "Test_base2" ON ("Test_top1"."middle_ptr_id" = "Test_base2"."b2_id")
  INNER JOIN "Test_base3" ON ("Test_top1"."middle_ptr_id" = "Test_base3"."b3_id")
  INNER JOIN "Test_middle" ON ("Test_top1"."middle_ptr_id" = "Test_middle"."base1_ptr_id")

It refers the top object directly to the base objects. I would expect:

  FROM "Test_top1"
  INNER JOIN "Test_middle" ON ("Test_top1"."middle_ptr_id" = "Test_middle"."base1_ptr_id")
  INNER JOIN "Test_base1" ON ("Test_middle"."base1_ptr_id" = "Test_base1"."b1_id") 
  INNER JOIN "Test_base2" ON ("Test_middle"."base2_ptr_id" = "Test_base2"."b2_id") 
  INNER JOIN "Test_base3" ON ("Test_middle"."base3_ptr_id" = "Test_base3"."b3_id")

This would normally not be a problem, however if I create a Top2 object or a Middle, now the ptr_ids will not all be incrementing at the same rate.

Attachments (2)

ticket_18174_tests.diff (2.4 KB) - added by Anssi Kääriäinen 4 years ago.
ticket_18174_patch.diff (7.1 KB) - added by phowe 4 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 4 years ago by Andrew Godwin

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Resolution: invalid
Status: newclosed

Are you sure the ptr_ids are going to increment at "different rates"? Those two queries are functionally equivalent - since ptr_id is always the same across all objects - and only the base class will define the PK value used for a new object.

I'm closing this as invalid - it needs a code/console example on what exactly the problem is with IDs before I can understand what the problem is supposed to be.

comment:2 Changed 4 years ago by phowe

Resolution: invalid
Status: closedreopened

Here is an Example model and unit test to demonstrate the issue

models.py

from django.db import models

class Base1( models.Model ):
  b1_id = models.AutoField( primary_key=True )
  b1_desc = models.CharField( max_length=100 )

class Base2( models.Model ):
  b2_id = models.AutoField( primary_key=True )
  b2_desc = models.CharField( max_length=100 )

class Middle1( Base1, Base2 ):
  m1_desc = models.CharField( max_length=100 )

class Middle2( Base2 ):
  m2_desc = models.CharField( max_length=100 )

class Top1( Middle1 ):
  t1_desc = models.CharField( max_length=100 )

class Top2( Middle2 ):
  t2_desc = models.CharField( max_length=100 )

test.py

from django.test import TestCase
from demo.demoapp.models import *

class DemoAppTests( TestCase ):
    def top1_only( self ):
      tmp = Top1()
      tmp.b1_desc = 'b1 1'
      tmp.b2_desc = 'b2 1'
      tmp.m1_desc = 'm1 1'
      tmp.t1_desc = 't1 1'
      tmp.save()

      tmp = Top2()
      tmp.b2_desc = 'b2 2'
      tmp.m2_desc = 'm2 2'
      tmp.t2_desc = 't2 2'
      tmp.save()

      tmp = Top1()
      tmp.b1_desc = 'b1 3'
      tmp.b2_desc = 'b2 3'
      tmp.m1_desc = 'm1 3'
      tmp.t1_desc = 't1 3'
      tmp.save()

      tmp = Top1.objects.get( b1_desc='b1 1' )
      print '%s %s %s %s' % ( tmp.b1_desc, tmp.b2_desc, tmp.m1_desc, tmp.t1_desc )

      tmp = Top2.objects.get( b2_desc='b2 2' )
      print '%s %s %s' % ( tmp.b2_desc, tmp.m2_desc, tmp.t2_desc )

      tmp = Top1.objects.get( b1_desc='b1 3' )
      print '%s %s %s %s' % ( tmp.b1_desc, tmp.b2_desc, tmp.m1_desc, tmp.t1_desc )

      self.failUnlessEqual( tmp.b1_desc, 'b2 3' )

comment:3 in reply to:  2 ; Changed 4 years ago by phowe

Replying to phowe:

Here is an Example model and unit test to demonstrate the issue

The output is:
b1 1 b2 1 m1 1 t1 1
b2 2 m2 2 t2 2
b1 3 b2 2 m1 3 t1 3

it should be
b1 1 b2 1 m1 1 t1 1
b2 2 m2 2 t2 2
b1 3 b2 3 m1 3 t1 3

comment:4 in reply to:  3 Changed 4 years ago by phowe

Replying to phowe:

Replying to phowe:

Sorry for the bad formmating, trying again.

The output is:
b1 1 b2 1 m1 1 t1 1
b2 2 m2 2 t2 2
b1 3 b2 2 m1 3 t1 3

it should be:
b1 1 b2 1 m1 1 t1 1
b2 2 m2 2 t2 2
b1 3 b2 3 m1 3 t1 3

comment:5 Changed 4 years ago by Melvyn Sopacua

Cc: Melvyn Sopacua added

comment:6 Changed 4 years ago by Anssi Kääriäinen

Triage Stage: UnreviewedAccepted

Verified the issue, and added a patch containing tests for this.

Changed 4 years ago by Anssi Kääriäinen

Attachment: ticket_18174_tests.diff added

comment:7 Changed 4 years ago by anonymous

Has patch: set

This patch seems to satisfy the unit test. The patch includes the test, and also includes an extra test to make sure all the ancestors are joined in. I added another parameter to get_ancestor_link in options.py. I noticed that the only other place it is used is django/contrib/gis/db/models/sql/compiler.py, I don't know if that code also needs to be updated to handle grandparent joining correctly or not.

Changed 4 years ago by phowe

Attachment: ticket_18174_patch.diff added

comment:8 Changed 4 years ago by pnhowe

Any word if this patch will be considered and Merged?

comment:9 in reply to:  8 Changed 4 years ago by Carl Meyer

Replying to pnhowe:

Any word if this patch will be considered and Merged?

The next step here is for someone other than the patch author to review the patch and try it out. If they think the code looks ok, and can verify that it solves the problem and all tests pass, they can mark it Ready For Checkin and that will help bring it to the attention of a core developer for commit. Feel free to recruit someone to do that if you want to move things along!

comment:10 Changed 4 years ago by Aymeric Augustin

Status: reopenednew

comment:11 Changed 3 years ago by pyriku

I just checked and the patch doesn't apply now as some tests got moved from tests/regressiontests/queries to tests/queries

comment:12 Changed 2 years ago by Tim Graham

Patch needs improvement: set

comment:13 Changed 14 months ago by Tim Graham

According to a comment on #17695, this patch fixes that issue so I marked it as a duplicate.

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