Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#24174 closed Bug (fixed)

Extra order by ignores descending orderings

Reported by: Bertrand Bordage Owned by: Josh Smeaton
Component: Database layer (models, ORM) Version: 1.8alpha1
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Consider this simple model:

class Test(Model):
    name = CharField(max_length=50)
    class Meta(object):
        db_table = 'test'

Test.objects.extra(order_by=['-name']) should be converted to something like SELECT * FROM test ORDER BY name DESC, but instead it becomes SELECT * FROM test ORDER BY name ASC. The minus is ignored.

It wasn’t happening in Django 1.6 and 1.7.

(This issue was discovered in the test suite of django-cachalot)

Change History (6)

comment:1 by Josh Smeaton, 9 years ago

Owner: changed from nobody to Josh Smeaton
Status: newassigned
Triage Stage: UnreviewedAccepted

Based on looking at the models of the linked test suite I'm guessing that this is hitting the "duplicate" order by clause. It's keeping the default ordering of the model, which is by name ascending, and removing the extra which is by name descending. Extra ordering should prevent the default ordering from being applied, and the negative sign should be preserved.

comment:2 by Bertrand Bordage, 9 years ago

I tested without default ordering, the issue is still there.

But I wasn’t precise enough in my report, it’s only happening when we specify the table name:

>>> print Test.objects.all().query
SELECT […] FROM "test"
>>> print Test.objects.extra(order_by=['-name']).query
SELECT […] FROM "test" ORDER BY "test"."name" DESC
>>> print Test.objects.extra(order_by=['-test.name']).query
SELECT […] FROM "test" ORDER BY ("test".name) ASC
>>> print Test.objects.extra(order_by=['-"test"."name"']).query
SELECT […] FROM "test" ORDER BY ("test"."name") ASC

comment:3 by Josh Smeaton, 9 years ago

Thanks for the follow up. That makes it even easier to track down.

https://github.com/django/django/blob/6e13c0490d67cdf210411f08feca3b78a49645ea/django/db/models/sql/compiler.py#L259

This is the block responsible, and should be fairly trivial to fix.

comment:4 by Josh Smeaton, 9 years ago

Has patch: set
Triage Stage: AcceptedReady for checkin

comment:5 by Josh Smeaton <josh.smeaton@…>, 9 years ago

Resolution: fixed
Status: assignedclosed

In 69c6a6868f0b4137bb293ff4326ecf4681506c37:

Fixed #24174 -- Fixed extra order by descending

comment:6 by Josh Smeaton <josh.smeaton@…>, 9 years ago

In 0c910823c166b827b090df5c5c8be9e502a14d8c:

[1.8.x] Fixed #24174 -- Fixed extra order by descending

Backport of 69c6a6868f0b4137bb293ff4326ecf4681506c37 from master

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