Opened 3 years ago

Closed 3 years ago

Last modified 3 years 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

Description

Possibly related to https://code.djangoproject.com/ticket/31481

# models.py
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)
# admin.py
from django.contrib import admin
from . import models


admin.site.register(models.Material)


@admin.register(models.QuotePartMaterial)
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 by Simon Charette, 3 years ago

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()
    clone.set_source_expressions(
        [
            F(f"{prefix}{expr.name}")
            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"{field.name}{LOOKUP_SEP}") when appropriate.

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

Version 0, edited 3 years ago by Simon Charette (next)

comment:2 by Mariusz Felisiak, 3 years ago

Resolution: duplicate
Status: newclosed

IMO it's a duplicate of #29538.

comment:3 by Simon Charette, 3 years ago

It is, thanks Mariusz.

comment:4 by Eduardo Rivas, 3 years ago

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