[Django] #26092: Regression when using order_by() on a ManyToManyField using the 'through' feature

13 views
Skip to first unread message

Django

unread,
Jan 17, 2016, 12:56:22 PM1/17/16
to django-...@googlegroups.com
#26092: Regression when using order_by() on a ManyToManyField using the 'through'
feature
----------------------------------------------+--------------------
Reporter: wjakob | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
Hi,

I'm seeing a serious regression (AttributeError exception thrown) when
using the order_by() feature on a ManyToManyField in combination with the
'through' feature.

This worked perfectly fine in Django 1.8.x, I'm seeing it for the first
time now. IMHO it is a release blocker, but then the release is already
out (so I set the severity as 'Normal').

See below for a minimal example:

{{{#!python
from django.db import models


class Person(models.Model):
name = models.CharField(max_length=200)


class Publication(models.Model):
title = models.CharField(max_length=200)
authors = models.ManyToManyField(Person, through='Author')

def get_authors(self):
return self.authors.order_by('author_to_publication')


class Author(models.Model):
person = models.ForeignKey(Person, verbose_name='Author',
related_name='author_to_publication')
publication = models.ForeignKey(Publication,
verbose_name='Publication')
order_id = models.PositiveSmallIntegerField(default=0)

class Meta:
ordering = ['order_id']
}}}

And here is an example shell session which causes the problem

{{{#!python
% ./manage.py shell
[INS]
Python 3.4.3 (v3.4.3:9b73f1c3e601, Feb 23 2015, 02:52:03)
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>>
>>> person = myapp.models.Person()
>>> person.save()
>>>
>>> publication = myapp.models.Publication(title='test publication')
>>> publication.save()
>>> author = myapp.models.Author(person=person, publication=publication,
order_id=1)
>>> author.save()
>>>
>>> publication.authors.order_by('author_to_publication')
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/Users/wenzel/mysite/lib/python3.4/site-
packages/django/db/models/query.py", line 234, in __repr__
data = list(self[:REPR_OUTPUT_SIZE + 1])
File "/Users/wenzel/mysite/lib/python3.4/site-
packages/django/db/models/query.py", line 258, in __iter__
self._fetch_all()
File "/Users/wenzel/mysite/lib/python3.4/site-
packages/django/db/models/query.py", line 1074, in _fetch_all
self._result_cache = list(self.iterator())
File "/Users/wenzel/mysite/lib/python3.4/site-
packages/django/db/models/query.py", line 52, in __iter__
results = compiler.execute_sql()
File "/Users/wenzel/mysite/lib/python3.4/site-
packages/django/db/models/sql/compiler.py", line 837, in execute_sql
sql, params = self.as_sql()
File "/Users/wenzel/mysite/lib/python3.4/site-
packages/django/db/models/sql/compiler.py", line 382, in as_sql
extra_select, order_by, group_by = self.pre_sql_setup()
File "/Users/wenzel/mysite/lib/python3.4/site-
packages/django/db/models/sql/compiler.py", line 49, in pre_sql_setup
order_by = self.get_order_by()
File "/Users/wenzel/mysite/lib/python3.4/site-
packages/django/db/models/sql/compiler.py", line 295, in get_order_by
field, self.query.get_meta(), default_order=asc))
File "/Users/wenzel/mysite/lib/python3.4/site-
packages/django/db/models/sql/compiler.py", line 571, in
find_ordering_name
if field.is_relation and path and opts.ordering and name !=
field.attname:
AttributeError: 'ManyToOneRel' object has no attribute 'attname'
>>>
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/26092>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jan 19, 2016, 9:09:58 AM1/19/16
to django-...@googlegroups.com
#26092: Regression when using order_by() on a ManyToManyField using the 'through'
feature
-------------------------------------+-------------------------------------

Reporter: wjakob | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0


Comment:

Could you convert the steps to reproduce into a test case for Django's
test suite (`tests/m2m_through` or `m2m_through_regress`, probably, and
reusing existing models, if possible) and
[https://docs.djangoproject.com/en/dev/internals/contributing/triaging-
tickets/#bisecting-a-regression bisect] to find the commit that caused the
breakage?

--
Ticket URL: <https://code.djangoproject.com/ticket/26092#comment:1>

Django

unread,
Jan 19, 2016, 9:15:17 AM1/19/16
to django-...@googlegroups.com
#26092: Regression when using order_by() on a ManyToManyField using the 'through'
feature
-------------------------------------+-------------------------------------

Reporter: wjakob | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by wjakob):

Hi --

unfortunately I don't have the cycles to dig deeper at the moment (I
already gave up and switched back to Django 1.8.x).
My hope was that this would be easy for somebody from Django project to
track down given a tiny self-contained example (which I submitted, see
above).

Best, Wenzel

--
Ticket URL: <https://code.djangoproject.com/ticket/26092#comment:2>

Django

unread,
Jan 19, 2016, 8:11:07 PM1/19/16
to django-...@googlegroups.com
#26092: Regression when using order_by() on a ManyToManyField using the 'through'
feature
-------------------------------------+-------------------------------------

Reporter: wjakob | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):

* stage: Unreviewed => Accepted
* severity: Normal => Release blocker


Old description:

New description:

Hi,

I'm seeing a serious regression (AttributeError exception thrown) when
using the order_by() feature on a ManyToManyField in combination with the
'through' feature.

This worked perfectly fine in Django 1.8.x, I'm seeing it for the first
time now. IMHO it is a release blocker, but then the release is already
out (so I set the severity as 'Normal').

See below for a minimal example:

{{{#!python


class Person(models.Model):
name = models.CharField(max_length=200)


class Publication(models.Model):
title = models.CharField(max_length=200)
authors = models.ManyToManyField(Person, through='Author')

class Author(models.Model):
person = models.ForeignKey(Person,

related_name='author_to_publication')
publication = models.ForeignKey(Publication)
order_id = models.PositiveSmallIntegerField(default=0)

class Meta:
ordering = ['order_id']
}}}

And here is an example test which causes the problem:

{{{#!python
from django.test import TestCase

from .models import Person, Publication, Author


class Test(TestCase):
def test(self):
person = Person()
person.save()
publication = Publication(title='test publication')
publication.save()
author = Author(person=person, publication=publication,
order_id=1)
author.save()
list(publication.authors.order_by('author_to_publication'))

--

Comment:

Bisected to d43aa28f67c52bc6f5ea5868c711df263895713f

--
Ticket URL: <https://code.djangoproject.com/ticket/26092#comment:3>

Django

unread,
Jan 20, 2016, 1:10:05 AM1/20/16
to django-...@googlegroups.com
#26092: Regression when using order_by() on a ManyToManyField using the 'through'
feature
-------------------------------------+-------------------------------------

Reporter: wjakob | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by akaariai):

I'll try to take a look today.

--
Ticket URL: <https://code.djangoproject.com/ticket/26092#comment:4>

Django

unread,
Jan 20, 2016, 3:35:18 AM1/20/16
to django-...@googlegroups.com
#26092: Regression when using order_by() on a ManyToManyField using the 'through'
feature
-------------------------------------+-------------------------------------

Reporter: wjakob | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by akaariai):

I was going to introduce attname for Rel objects, but decided it is safer
to use getattr(field, attname, None) as this will be backpatched. I think
I got the functionality right.

As a side note, a thing to try is to introduce ForeignKey's attname field
as a separate field in the model's meta. This would clean a lot of code.
Instead of having a ForeignKey that also tries to mimic an
integer/text/whatever field, you would have a ForeignKey that defines the
relation, and an Integer/Text/...Field that contains the concrete value
used by the relation. For example, this bug was at least partially caused
by having a ForeignKey represent two different things (the attname check
in compiler is for cases where you want to order by the concrete field
instead of the relation field). Unfortunately changing the representation
of foreign keys will likely be hard to implement in backwards compatible
way. On the other hand, if we get the representation of foreign keys
changed we have solved one of the hardest parts of multicolumn foreign
keys.

--
Ticket URL: <https://code.djangoproject.com/ticket/26092#comment:5>

Django

unread,
Jan 20, 2016, 4:15:14 AM1/20/16
to django-...@googlegroups.com
#26092: Regression when using order_by() on a ManyToManyField using the 'through'
feature
-------------------------------------+-------------------------------------

Reporter: wjakob | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by akaariai):

* has_patch: 0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/26092#comment:6>

Django

unread,
Jan 20, 2016, 7:16:13 PM1/20/16
to django-...@googlegroups.com
#26092: Regression when using order_by() on a ManyToManyField using the 'through'
feature
-------------------------------------+-------------------------------------
Reporter: wjakob | Owner: nobody
Type: Bug | Status: closed

Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham <timograham@…>):

* status: new => closed
* resolution: => fixed


Comment:

In [changeset:"ee596888e1149864e7828f5cf63c0eda395744c3" ee596888]:
{{{
#!CommitTicketReference repository=""
revision="ee596888e1149864e7828f5cf63c0eda395744c3"
Fixed #26092 -- Fixed QuerySet.order_by() regression with an M2M through
model.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/26092#comment:7>

Django

unread,
Jan 20, 2016, 7:21:07 PM1/20/16
to django-...@googlegroups.com
#26092: Regression when using order_by() on a ManyToManyField using the 'through'
feature
-------------------------------------+-------------------------------------
Reporter: wjakob | Owner: nobody

Type: Bug | Status: closed
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"05e8fa83c378c882ab4d4a1012ce954a9dced6de" 05e8fa83]:
{{{
#!CommitTicketReference repository=""
revision="05e8fa83c378c882ab4d4a1012ce954a9dced6de"
[1.9.x] Fixed #26092 -- Fixed QuerySet.order_by() regression with an M2M
through model.

Backport of ee596888e1149864e7828f5cf63c0eda395744c3 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/26092#comment:8>

Django

unread,
Jan 21, 2016, 8:07:17 AM1/21/16
to django-...@googlegroups.com
#26092: Regression when using order_by() on a ManyToManyField using the 'through'
feature
-------------------------------------+-------------------------------------
Reporter: wjakob | Owner: nobody

Type: Bug | Status: closed
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"fb4272f0e6bbdaa3e19ed5fde59fdb5ab5a33baf" fb4272f]:
{{{
#!CommitTicketReference repository=""
revision="fb4272f0e6bbdaa3e19ed5fde59fdb5ab5a33baf"
Refs #26092 -- Added @skipUnlessDBFeature to a test.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/26092#comment:9>

Django

unread,
Jan 21, 2016, 8:46:07 AM1/21/16
to django-...@googlegroups.com
#26092: Regression when using order_by() on a ManyToManyField using the 'through'
feature
-------------------------------------+-------------------------------------
Reporter: wjakob | Owner: nobody

Type: Bug | Status: closed
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"c9d1d5593b5533d970cc1d1fe918de72469920b5" c9d1d55]:
{{{
#!CommitTicketReference repository=""
revision="c9d1d5593b5533d970cc1d1fe918de72469920b5"
[1.9.x] Refs #26092 -- Added @skipUnlessDBFeature to a test.

Backport of fb4272f0e6bbdaa3e19ed5fde59fdb5ab5a33baf from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/26092#comment:10>

Django

unread,
Feb 26, 2016, 3:07:17 PM2/26/16
to django-...@googlegroups.com
#26092: Regression when using order_by() on a ManyToManyField using the 'through'
feature
-------------------------------------+-------------------------------------
Reporter: wjakob | Owner: nobody

Type: Bug | Status: closed
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by lamby):

FYI this was also affecting OneToOne fields, eg.:

{{{
class Parent(models.Model):
pass

class Child(models.Model):
parent = models.OneToOneField(Parent, related_name='child')
ordering = models.IntegerField()

class Meta:
ordering = ('ordering',)

>>> Parent.objects.order_by('child')
# (boom)
}}}

Calling:

{{{
>>> Parent.objects.order_by('child__ordering')
}}}

.. would temporarily "fix" it.

Mentioning it in case you want to expand the release notes and for anyone
else hit by this issue in 1.9.1

--
Ticket URL: <https://code.djangoproject.com/ticket/26092#comment:11>

Reply all
Reply to author
Forward
0 new messages