Opened 6 years ago

Closed 3 years ago

Last modified 20 months ago

#29538 closed Bug (fixed)

Query Expression in ordering of a related object fails

Reported by: wilhelmhb Owned by: Eduardo Rivas
Component: Database layer (models, ORM) Version: 2.0
Severity: Normal Keywords: ordering, query expression, related object
Cc: wilhelmhb, colons, Eduardo Rivas 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 (last modified by Tim Graham)

Since 2.0, according to the doc (https://docs.djangoproject.com/en/2.0/ref/models/options/#ordering), we can use QueryExpression objects in the Model.Meta.ordering field.
Using:

from django.db import models

class Musician(models.Model):
    first_name = models.CharField(max_length=50)
    last_name = models.CharField(max_length=50)
    instrument = models.CharField(max_length=100, null=True, blank=True)

    class Meta:
        ordering = [models.F('instrument').asc(nulls_last=True)]

class Album(models.Model):
    artist = models.ForeignKey(Musician, on_delete=models.CASCADE)
    name = models.CharField(max_length=100)
    release_date = models.DateField()
    num_stars = models.IntegerField()

    class Meta:
        ordering = ['artist']
>>> Album.objects.all()
...
TypeError: 'OrderBy' does not support indexing

When reaching https://github.com/django/django/blob/master/django/db/models/sql/compiler.py#L669, the compiler tries to use the related model, but at line 679, item can be an OrderBy object. Thus the failure.

Change History (17)

comment:1 by wilhelmhb, 6 years ago

Cc: wilhelmhb added

comment:2 by Tim Graham, 6 years ago

Description: modified (diff)
Summary: QueryExpression in ordering of a related object failsQuery Expression in ordering of a related object fails
Triage Stage: UnreviewedAccepted

comment:3 by Alexandre Laplante, 6 years ago

Owner: changed from nobody to Alexandre Laplante
Status: newassigned

Started working on a fix.

comment:4 by Alexandre Laplante, 6 years ago

Has patch: set

PR Looking for feedback on this patch.

comment:5 by Carlton Gibson, 6 years ago

Patch needs improvement: set

comment:6 by colons, 6 years ago

Cc: colons added

comment:7 by Mariusz Felisiak, 3 years ago

After 8c5f9906c56ac72fc4f13218dd90bdf9bc8a248b it crashes with:

django.core.exceptions.FieldError: Cannot resolve keyword 'instrument' into field. Choices are: artist, artist_id, id, name, num_stars, release_date

comment:8 by Mariusz Felisiak, 3 years ago

#33678 was a duplicate for functions in related Meta.ordering.

comment:9 by Simon Charette, 3 years ago

If anyone is interested in addressing this ticket there's a possible implementation detailed in https://code.djangoproject.com/ticket/33678#comment:3

comment:10 by Eduardo Rivas, 3 years ago

Cc: Eduardo Rivas added
Patch needs improvement: unset

comment:11 by Eduardo Rivas, 3 years ago

Owner: changed from Alexandre Laplante to Eduardo Rivas

comment:12 by Mariusz Felisiak, 3 years ago

Needs tests: set

comment:13 by Simon Charette, 3 years ago

Needs tests: unset

comment:14 by Mariusz Felisiak, 3 years ago

Triage Stage: AcceptedReady for checkin

comment:15 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In 2798c937:

Fixed #29538 -- Fixed crash of ordering by related fields when Meta.ordering contains expressions.

Thanks Simon Charette for the review.

comment:16 by a-p-f, 20 months ago

Sorry for my ignorance, but is there a simple way for users to see, from this page, which minor versions of Django have received this patch? I followed the link to the github commit page, and from what I could tell there, the fix was implemented in 4.1. That doesn't tell me, however, if the commit was cherry-picked into any other branches.

At first I had assumed that since 3.2 was the latest LTS release, it would receive this fix. I upgraded to 3.2.18, though, and this bug isn't fixed. Looking closer at Django's docs, I see that 3.2 is only in extended support, and this bug does not qualify for extended support. Fair enough. I just think it would help developers to make it clear which of the current versions have received a given fix.

in reply to:  16 comment:17 by Mariusz Felisiak, 20 months ago

Replying to a-p-f:

Sorry for my ignorance, but is there a simple way for users to see, from this page, which minor versions of Django have received this patch? I followed the link to the github commit page, and from what I could tell there, the fix was implemented in 4.1. That doesn't tell me, however, if the commit was cherry-picked into any other branches.

At first I had assumed that since 3.2 was the latest LTS release, it would receive this fix. I upgraded to 3.2.18, though, and this bug isn't fixed. Looking closer at Django's docs, I see that 3.2 is only in extended support, and this bug does not qualify for extended support. Fair enough. I just think it would help developers to make it clear which of the current versions have received a given fix.

You can click commit (2798c937deb6625a4e6a36e70d4d60ce5faac954) which fixed this issue and check tags, 4.1a1 so it's fixed in Django 4.1+.

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