Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#26195 closed Bug (wontfix)

order_by pk doesn't work in multi-table inheritance models if parent has default ordering

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


Using django 1.9.2, given the example

class ModelA ( models.Model ):
	class Meta ( object ):
		ordering = ("x",)
	x = models.TextField ()

class ModelB ( ModelA ):

The following query should order by modela_ptr_id:

>>> str ( ModelB.objects.order_by ( "pk" ).query )
'SELECT "dummy_modela"."id", "dummy_modela"."x", "dummy_modelb"."modela_ptr_id" FROM "dummy_modelb" INNER JOIN "dummy_modela" ON ( "dummy_modelb"."modela_ptr_id" = "dummy_modela"."id" ) ORDER BY "dummy_modela"."x" ASC'

But doesn't, instead falling back to ModelA's default.

This is particularly annoying as it prevents you from doing a .distinct ( "pk" ).order_by ( "pk" )

Change History (5)

comment:1 by ris, 8 years ago

Note however doing .order_by ( "id" ) or .order_by ( "modela_ptr_id" ) works as expected:

'SELECT "dummy_modela"."id", "dummy_modela"."x", "dummy_modelb"."modela_ptr_id" FROM "dummy_modelb" INNER JOIN "dummy_modela" ON ( "dummy_modelb"."modela_ptr_id" = "dummy_modela"."id" ) ORDER BY "dummy_modelb"."modela_ptr_id" ASC'

comment:2 by Simon Charette, 8 years ago

Resolution: wontfix
Status: newclosed

While it might not be intuitive ModelB.objects.order_by('pk') is equivalent to ModelB.objects.order_by('modela_ptr') which will use the ordering defined on ModelA as documented.

Note that before #19195 was fixed it was not even possible to bypass the order by related model's ordering behavior.

At this point I'm afraid we'll have to stick to the actual behavior and keep requiring an explicit order by id because of the backward incompatibilities such a change would introduce.

If we wanted to special case order_by('pk') to prevent the usage of the referenced model ordering it would have to go through a deprecation period where a warning is raised if pk is a field pointing to a model with a defined ordering option.

In short I'm -0 on introducing such a change as it will require a deprecation period and will introduce yet another edge case in the handling of ordering by the ORM in order to expose a non-intuitive behavior to users familar with the actual ordering behavior.

I will close the ticket as wontfix but feel free to post to the developer's mailing list to gather support for such a change.

comment:3 by ris, 8 years ago

Well, at least it's documented here now.

Though I would argue that ordering by "pk" would be a very useful thing for the ORM to support universally - especially if you want to work generically with models and .distinct () is going to insist on distinctifying on everything-and-its-mother by default.

comment:4 by ris, 8 years ago

Just adding a note to this: it seems there are actually a good few things that will cause an MTI model to throw a hissy fit and silently ignore .order_by() calls. I've now found cases where specifying a related model to .order_by() will cause this behaviour whereas simply adding __pk to that model lookup will allow it to work fine.

Last edited 8 years ago by ris (previous) (diff)

in reply to:  4 comment:5 by Simon Charette, 8 years ago

Replying to ris:

I've now found cases where specifying a related model to .order_by() will cause this behaviour whereas simply adding __pk to that model lookup will allow it to work fine.

Again, this is just like documented.

class Author(models.Model):
    name = models.CharField(max_length=100)

    class Meta:
        ordering = ['name']

class Book(models.Model):
    author = models.ForeignKey(Author)

# Equivalent of order_by('author__name')

# Equivalent of order_by('author__id').
# It bypasses the _meta.ordering but results in a JOIN.

# Used to be equivalent of order_by('author__name') before #19195 was fixed.
Last edited 8 years ago by Simon Charette (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top