order_by('pk') is not obeyed if superclass has default ordering

[Andrew Dodd]

Using 'pk' as an order_by() target does not work if the superclass has a default ordering specified on its 'Meta' class.

For example, with the following classes:

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


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

    class Meta:
        ordering = ('name', 'extra')


class SubFromSuperWithoutOrdering(SuperWithoutOrdering):
    anything = models.CharField(max_length=200)


class SubFromSuperWithOrdering(SuperWithOrdering):
    anything = models.CharField(max_length=200)

    def __str__(self):
        # so test output is readable
        return 'SubWithOrdering - name:{} extra:{} anything:{}'.format(
                self.name, self.extra, self.anything)

Then:

SubFromSuperWithoutOrdering.objects.all().order_by('pk') # works, ordering by id
SubFromSuperWithOrdering.object.all().order_by('pk') # does not work, it just orders by 'name' then 'extra'

I have two tests that demonstrate this.

One that compares the 'ORDER BY' clause of the actual SQL queries:

class TestOrderByClause(TestCase):
    def test_it_orders_by_pk_and_id_are_the_same_if_super_does_not_have_ordering(self):
        qs = SubFromSuperWithoutOrdering.objects.all()
        by_pk = str(qs.order_by('pk').query).split('ORDER BY')[1]
        by_id = str(qs.order_by('id').query).split('ORDER BY')[1]
        self.assertEquals(by_pk, by_id) # PASSES

    def test_it_orders_by_pk_and_id_are_the_same_if_super_does_have_ordering(self):
        qs = SubFromSuperWithOrdering.objects.all()
        by_pk = str(qs.order_by('pk').query).split('ORDER BY')[1]
        by_id = str(qs.order_by('id').query).split('ORDER BY')[1]
        self.assertEquals(by_pk, by_id) # FAILS

    # Alternative form?
    def test_order_by_clause_is_the_same_if_using_pk_or_id(self):
        for test_class in [SubFromSuperWithoutOrdering,
                           SubFromSuperWithOrdering]:
            qs = test_class.objects.all()
            by_pk = str(qs.order_by('pk').query).split('ORDER BY')[1]
            by_id = str(qs.order_by('id').query).split('ORDER BY')[1]
            self.assertEquals(by_pk, by_id, 'Failed for {}'.format(test_class))

And one that compares the order of the items actually returned from executing the query:

class TestOrderingOfQuerySetWhenSuperclassHasDefaultOrdering(TestCase):
    def setUp(self):
        def make_obj(name, extra, anything):
            return SubFromSuperWithOrdering.objects.create(name=name, extra=extra, anything=anything)

        self.zoot2 = make_obj(    'Zoot', 'ZZZ - Unknown',    'fourth by super ordering, first by pk')
        self.zoot1 = make_obj(    'Zoot', 'AAA - Handmaiden', 'third by super ordering, second by pk')
        self.arthur2 = make_obj('Arthur', 'ZZZ - Imposter',   'second by super ordering, third by pk')
        self.arthur1 = make_obj('Arthur', 'AAA - The King',   'first by super ordering, fourth by pk')

    def test_it_uses_superclass_ordering_by_default(self):
        expected = [self.arthur1, self.arthur2, self.zoot1, self.zoot2]
        actual = list(SubFromSuperWithOrdering.objects.all())
        self.assertEqual(expected, actual)

    def test_it_correctly_orders_with_id(self):
        expected = [self.zoot2, self.zoot1, self.arthur2, self.arthur1]
        actual = list(SubFromSuperWithOrdering.objects.all().order_by('id'))
        self.assertEqual(expected, actual)

    def test_it_correctly_orders_with_pk(self):
        expected = [self.zoot2, self.zoot1, self.arthur2, self.arthur1]
        actual = list(SubFromSuperWithOrdering.objects.all().order_by('pk'))
        self.assertEqual(expected, actual)

I'm not very sure where to go looking to fix this. The workaround is to specify the name of the field used as the PK instead of using 'pk'.

[Simon Charette]

Hi Andrew,

Thanks for your detailed report.

While confusing ordering by a foreign key relies on the referenced model _meta.ordering if defined. In the case of MTI a OneToOneField to the parent is used as a primary key which makes ordering by pk follows the pk -> parent_ptr -> Parent._meta.ordering chain.

See #26195 resolution for more details.

[Andrew Dodd]

Sorry I didn't find that pre-existing ticket when I looked.

It is a shame it is a won't fix, as I think it is pretty clearly a bug (the behaviour is extremely astonishing).

I also find it hard to think of a place where 'pk' is being included in the list for order_by() that would actually break if this were fixed (as far as I can tell these places have a bug right now, they just don't know it).

Thanks for the response.

[Simon Charette]

I agree with you that It's definitely an unexpected result, just like the whole ordering by a foreign key orders by the referenced model's ordering feature is.

However changing this now would most likely break a lot of code without offering an alternative solution for the actual supporter of the feature. I personally think special casing the MTI pk case would make it even worse as that would be another rule to be aware of. Think of what should be done in regard to models using a foreign key as a primary key without relying on MTI?

I wouldn't be opposed to such a change as I've been affected by this issue myself in past. That's what motivated me to work on allowing ordering by _id fields to bypass this behavior in #19195 but I'd suggest you take this discussion to the mailing list to gather some support and feedback if you're interested in working toward removing this feature.