Opened 3 months ago

Last modified 6 weeks ago

#29408 new Cleanup/optimization

Add validation of ordering by a field from related model.

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


When the ordering class member in Meta of a model contains a field from a related model, and that field does not exist, django's makemigrations does not throw an error. However, if it is a direct field member of the same class, makemigrations does throw an error.
Example below tested on Django 2.0.5

from django.db import models

# Create your models here.

class Agreement(models.Model):
    agreement_id = models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)
    #class Meta:
      # generates error in makemigrations
      # app.Agreement: (models.E015) 'ordering' refers to the nonexistent field 'id'.
      # ordering = ['id']

class Order(models.Model):
    agreement = models.ForeignKey(Agreement, models.DO_NOTHING)
    class Meta:
      # does not generate error in makemigrations
      # but does so during runtime
      # e.g. [x for x in Order.objects.all()]
      ordering = ['agreement__id']

Change History (3)

comment:1 Changed 3 months ago by Carlton Gibson

Component: UncategorizedDatabase layer (models, ORM)
Summary: ordering by field from related model does not validate if field existsAdd validation of ordering by a field from related model.
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization
Version: 2.0master

I'm going to accept this provisionally. There's a `FIXME` in `models/` specifically about this:

        # Skip ordering in the format field1__field2 (FIXME: checking
        # this format would be nice, but it's a little fiddly).
        fields = (f for f in fields if LOOKUP_SEP not in f)

Added in d818e0c9b2b88276cc499974f9eee893170bf0a8.

Either we should address this, or remove the comment and close as wontfix if "fiddly" turns out to be more effort than it's worth.

A test case and a patch showing what "fiddly" actually entails would be great.

comment:2 Changed 3 months ago by Windson yang

I think we can just address this in the document and don't fix it.

comment:3 Changed 6 weeks ago by Jeff

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