Opened 3 months ago

Closed 3 months ago

Last modified 3 months ago

#33678 closed Bug (duplicate)

order_by crash when sorting by a foreign key referencing a Model with an ordering that includes expressions

Reported by: Eduardo Rivas Owned by: nobody
Component: Database layer (models, ORM) Version: 4.0
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


Possibly related to

from django.db import models
from django.db.models.functions import Lower

class Material(models.Model):
    code = models.CharField(max_length=255, unique=True)

    class Meta:
        ordering = (Lower("code"),)

    def __str__(self):
        return self.code

class QuotePartMaterial(models.Model):
    material = models.ForeignKey(Material, on_delete=models.PROTECT)

    def __str__(self):
        return str(self.material)
from django.contrib import admin
from . import models

class QuotePartMaterialAdmin(admin.ModelAdmin):
    list_display = ("material",)

Loading the admin works fine, but If I try to manually sort QuotePartMaterial records in the admin by clicking the "Material" column header, I get:

FieldError at /admin/example/quotepartmaterial/
Cannot resolve keyword 'code' into field. Choices are: id, material, material_id

Removing the Lower() call on Material.Meta.ordering makes the issue go away.

Change History (4)

comment:1 Changed 3 months ago by Simon Charette

Component: contrib.adminDatabase layer (models, ORM)
Summary: Admin crash when sorting by a foreign key that's sorted with functionsorder_by crash when sorting by a foreign key referencing a Model with an ordering that includes expressions
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Thank you for your report. I vaguely remember that this was brought up already previously but I can't find an existing ticket on the subject.

The issue here is not the admin as you'll get the same crash if you try to do list(QuotePartMaterial.objects.order_by('material')).

What we're lacking is a bit of logic that translates order_by('material') to order_by(Lower('material__code')) instead of naively doing .order_by(Lower('code')) when we're not dealing with a field inherited through a parent link. This is something that was overlooked in #30557.

I suggest we take an approach similar to Expression.replace_references in the ungoing work for database constraint validation and introduce Expression.prefix_references that goes along these lines

def prefix_references(self, prefix):
    clone = self.copy()
            if isinstance(expr, F)
            else expr.prefix_references(prefix)
            for expr in self.get_source_expressions()
    return clone

It will then be trivial to adjust SQLCompiler.find_ordering_name to call item.prefix_references(f"{}{LOOKUP_SEP}") when appropriate.

Is this something you'd be interesting in fixing and writing tests for Eduardo? This seems like a good introduction ticket to a few ORM components if this is something you've been curious about.

Last edited 3 months ago by Simon Charette (previous) (diff)

comment:2 Changed 3 months ago by Mariusz Felisiak

Resolution: duplicate
Status: newclosed

IMO it's a duplicate of #29538.

comment:3 Changed 3 months ago by Simon Charette

It is, thanks Mariusz.

comment:4 Changed 3 months ago by Eduardo Rivas

Thanks for the pointer Simon. And yes, I'd like to dive into this

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