select_related and multiple inheritance

[Shaun Cutts]

I have a "Profile" model that inherits both from auth.User and Person -- representing people who are users.
Such people also have accounts -- Account has a OneToOneField relating it to a Profile.

A particular user has an account.

>>> profile = Profile.objects.get( pk = 9819 )
>>> profile
<Profile: bob test>
>>> profile.account
<Account: bob test>

---

But when I use a select_related, an error occurs:

In [36]: profile = Profile.objects.select_related( 'account' ).get( pk = 9819 )
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/Users/shauncutts/<ipython console> in <module>()

/Users/shauncutts/dev/django/db/models/query.pyc in get(self, *args, **kwargs)
    334         if self.query.can_filter():
    335             clone = clone.order_by()
--> 336         num = len(clone)
    337         if num == 1:
    338             return clone._result_cache[0]

/Users/shauncutts/dev/django/db/models/query.pyc in __len__(self)
     79                 self._result_cache = list(self._iter)
     80             else:
---> 81                 self._result_cache = list(self.iterator())
     82         elif self._iter:
     83             self._result_cache.extend(list(self._iter))

/Users/shauncutts/dev/django/db/models/query.pyc in iterator(self)
    267 
    268         compiler = self.query.get_compiler(using=self.db)
--> 269         for row in compiler.results_iter():
    270             if fill_cache:
    271                 obj, _ = get_cached_row(self.model, row,

/Users/shauncutts/dev/django/db/models/sql/compiler.pyc in results_iter(self)
    670         resolve_columns = hasattr(self, 'resolve_columns')
    671         fields = None
--> 672         for rows in self.execute_sql(MULTI):
    673             for row in rows:
    674                 if resolve_columns:

/Users/shauncutts/dev/django/db/models/sql/compiler.pyc in execute_sql(self, result_type)
    715         """
    716         try:
--> 717             sql, params = self.as_sql()
    718             if not sql:
    719                 raise EmptyResultSet

/Users/shauncutts/dev/django/db/models/sql/compiler.pyc in as_sql(self, with_limits, with_col_aliases)
     53         in the query.
     54         """
---> 55         self.pre_sql_setup()
     56         out_cols = self.get_columns(with_col_aliases)
     57         ordering, ordering_group_by = self.get_ordering()

/Users/shauncutts/dev/django/db/models/sql/compiler.pyc in pre_sql_setup(self)
     27             self.query.setup_inherited_models()
     28         if self.query.select_related and not self.query.related_select_cols:
---> 29             self.fill_related_selections()
     30 
     31     def quote_name_unless_alias(self, name):

/Users/shauncutts/dev/django/db/models/sql/compiler.pyc in fill_related_selections(self, opts, root_alias, cur_depth, used, requested, restricted, nullable, dupe_set, avoid_set)
    608                 alias = root_alias
    609                 alias_chain = []
--> 610                 chain = opts.get_base_chain(f.rel.to)
    611                 if chain is not None:
    612                     for int_model in chain:

/Users/shauncutts/dev/django/db/models/options.pyc in get_base_chain(self, model)
    427             return [model]
    428         for parent in self.parents:
--> 429             res = parent._meta.get_base_chain(model)
    430             if res:
    431                 res.insert(0, parent)

/Users/shauncutts/dev/django/db/models/options.pyc in get_base_chain(self, model)
    432                 return res
    433         raise TypeError('%r is not an ancestor of this model'
--> 434                 % model._meta.module_name)
    435 
    436     def get_parent_list(self):

TypeError: 'profile' is not an ancestor of this model

Examination of the stack trace shows that the problem may be related to the fact that Profile inherits from both User and Person

>>> pdb.pm()
> /Users/shauncutts/dev/django/db/models/options.py(434)get_base_chain()
-> % model._meta.module_name)
(Pdb) u
> /Users/shauncutts/dev/django/db/models/options.py(429)get_base_chain()
-> res = parent._meta.get_base_chain(model)
(Pdb) print parent
<class 'cranedata.db.base.models.Person'>

[Russell Keith-Magee]

Multiple inheritance of models isn't officially supported (see #12002, #10808, amongst others). It *may* work under certain conditions, but it's certainly not guaranteed to work.

The closest you'll be able to get is to do single inheritance, then use a OneToOne field to point at the second class.

Closing wontfix since it's a bug in a configuration that we don't support.

[Shaun Cutts]

Um .. ok: but may I suggest that you have a documentation bug then? See:

​http://docs.djangoproject.com/en/1.2/topics/db/models/#multiple-inheritance

It certainly doesn't say its not supported.

[Russell Keith-Magee]

Huh. Ok... that's interesting. Thanks for setting me straight -- I wasn't aware we explicitly said that in the docs.

In that case, there's definitely a bug here.

[srmerritt]

I am getting a separate error from the same issue (select_related failing to handle multiple inheritance), in v1.2.
My traceback looks like this:

• /django/db/models/query.py", line 335, in get\n num = len(clone)\n',
• /django/db/models/query.py", line 80, in len\n self._result_cache = list(self.iterator())\n',
• /django/db/models/query.py", line 273, in iterator\n only_load=only_load)\n',
• /django/db/models/query.py", line 1279, in get_cached_row\n setattr(rel_obj, rel_field.attname, getattr(obj, rel_field.attname))\n',

And my error is of this format: AttributeError: "'Parent2' object has no attribute 'Parent1Attribute'". This section of get_cached_row clearly assumes it is dealing with single inheritance, whereas in my case 'obj' is not an instance of 'rel_model'.

[David Bennett]

test demonstrating error

[David Bennett]

I've created a couple tests. The version with select_related fails, and the version without doesn't.

Description: If Child1 has a OneToOneField to Parent1 and if Child1 is a subclass of Parent2, doing Parent1.objects.select_related('child1') will produce this traceback:

======================================================================
ERROR: test_subclass_with_select_related (modeltests.select_related.tests.SelectRelatedTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/david/Projects/django-trunk/tests/modeltests/select_related/tests.py", line 171, in test_subclass_with_select_related
    p = Parent1.objects.select_related('child1').get(name="Parent1")
  File "/Users/david/Projects/django-trunk/django/db/models/query.py", line 361, in get
    num = len(clone)
  File "/Users/david/Projects/django-trunk/django/db/models/query.py", line 85, in __len__
    self._result_cache = list(self.iterator())
  File "/Users/david/Projects/django-trunk/django/db/models/query.py", line 294, in iterator
    offset=len(aggregate_select))
  File "/Users/david/Projects/django-trunk/django/db/models/query.py", line 1420, in get_cached_row
    setattr(rel_obj, rel_field.attname, getattr(obj, rel_field.attname))
AttributeError: 'Parent1' object has no attribute 'subname'

----------------------------------------------------------------------

The same issue occurs if you subclass from both Parent1 and Parent2, but I wanted to demonstrate that there is still a problem with just a single subclass.

[David Bennett]

I've attached a patch that resolves the issue. Included are updated tests that check for the following:

Child1 is a subclass of both Parent1 and Parent2; Parent1.objects.select_related('child1') should not throw an error (see traceback in comment:10).

Child2 is a subclass of Parent1 and has a OneToOneField to Parent2; doing Parent2.objects.select_related('child2') should not throw an error (same traceback).

[David Bennett]

I've encountered an issue with my patch. If Child1 is a subclass of Parent1 and Child2 is a subclass of both Parent1 and Parent2, doing a Parent1.objects.select_related('child1', 'child2') will not return any results for Child1 because an INNER JOIN is being used with Parent2.

So, I need to figure out how to force it to a LEFT OUTER join in this instance. At this point I might just try a different approach for my project.

[David Bennett]

Tests and patch (trunk)

[David Bennett]

Tests and patch (1.3.X)

[David Bennett]

Tests and patch (1.2.X)

[David Bennett]

Ok, I've created an additional test for the last issue I found and also figured out how to resolve it.

This patch should be ready for review.

[Tomáš Ehrlich]

Diff against latest master

[Tomáš Ehrlich]

I've applied the select_related_subclass_patch.diff​ and resolved two merge conflicts against latest master branch. Tests passed.

[Anssi Kääriäinen]

Updated patch at: ​https://github.com/akaariai/django/commit/7ed2c5f16caa9c413002c6d9c120c657bd37fc96

I think this one is now ready for commit.

I added reference to the model from options, that is SomeM == SomeM._meta.model. This seems useful and is innocent as far as cyclic references go. There is already a cyclic reference by SomeM._meta.local_fields[0].model == SomeM.

For the record, I don't think this one was exactly easy pickings...

[Anssi Kääriäinen]

I updated the work, and fixed a couple of issues affecting longer inheritance chains. Work available here: ​https://github.com/akaariai/django/commits/ticket_13781

The problem here is that when you fix one issue, you find two more broken issues. See added @expectedFailure tests. I think I will just go with the approach in the above branch and be done with this ticket. Fixing the @expectedFailure issues will lead to larger refactorings than what I am ready to do currently.

[Anssi Kääriäinen]

I rebased the ticket_13781 branch for final commit. As the commit is somewhat complex there is also somewhat big risk of regressions so I will not backpatch this to 1.5, at least not initially. The cases where this patch is needed aren't that common.

[Anssi Kääriäinen <akaariai@…>]

In f51e409a5fb34020e170494320a421503689aea0:

Fixed #13781 -- Improved select_related in inheritance situations

The select_related code got confused when it needed to travel a
reverse relation to a model which had different parent than the
originally travelled relation.

Thanks to Trac aliases shauncutts for report and ungenio for original
patch (committed patch is somewhat modified version of that).

[Anssi Kääriäinen]

The commit above didn't contain some important lines in get_cached_row(), fixed in 1194a9699932088385f9f88869be28a251597f45.

[Simon Charette]

Is this going to be backported to 1.5?

[Simon Charette]

FWIW f51e409a5fb34020e170494320a421503689aea0 also fixed #16572 for me.

[Anssi Kääriäinen]

OK, I will see about adding the tests of #16572. I am planning of committing a couple more ORM fixes and then go through all the ORM tickets and close those which aren't relevant any more.

I am not yet sure if I should backpatch this to 1.5. The problem is that the ORM is somewhat fragile due to all the different query combinations and thus I don't feel too comfortable about backpatching...

[Simon Charette]

Alright, didn't want to push you to backpatch, just wanted to know if you were planing to.

Good work on those patches!

[Anssi Kääriäinen <akaariai@…>]

In 7cfb567e457379f52a80fe0e8d98dd8191391c6e:

Another regression fix for select_related handling

This time gis compiler.get_default_columns() wasn't up to date. Thanks
to CI another regression fixed.

Refs #13781

[Preston Holmes <preston@…>]

In a52794bc21404bc9079877db7ef5e1b8d1cafb8a:

Fixed #13781 -- Improved select_related in inheritance situations

The select_related code got confused when it needed to travel a
reverse relation to a model which had different parent than the
originally travelled relation.

Thanks to Trac aliases shauncutts for report and ungenio for original
patch (committed patch is somewhat modified version of that).

[Preston Holmes <preston@…>]

In af1e499be089deebe64bd272b5c0fb80e582398a:

Another regression fix for select_related handling

This time gis compiler.get_default_columns() wasn't up to date. Thanks
to CI another regression fixed.

Refs #13781