Opened 3 years ago

Closed 3 years ago

Last modified 3 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 Changed 3 years ago by Josh Smeaton

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 Changed 3 years ago by Bertrand Bordage

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 Changed 3 years ago by Josh Smeaton

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 Changed 3 years ago by Josh Smeaton

Has patch: set
Triage Stage: AcceptedReady for checkin

comment:5 Changed 3 years ago by Josh Smeaton <josh.smeaton@…>

Resolution: fixed
Status: assignedclosed

In 69c6a6868f0b4137bb293ff4326ecf4681506c37:

Fixed #24174 -- Fixed extra order by descending

comment:6 Changed 3 years ago by Josh Smeaton <josh.smeaton@…>

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