Opened 4 weeks ago

Closed 3 weeks ago

#37278 closed Cleanup/optimization (fixed)

QuerySet.totally_ordered property doesn't fathom aliases of pure fields

Reported by: Jacob Walls Owned by: Jacob Walls
Component: Database layer (models, ORM) Version: 6.1
Severity: Normal Keywords:
Cc: VIZZARD-X 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

In #36857 we added a public totally_ordered property on QuerySet by porting some logic from the admin.

We didn't check whether F() objects might refer to annotations.

class PhotoOfTheDay(models.Model):
    day = models.DateField(unique=True)

>>> PhotoOfTheDay.objects.order_by("day").totally_ordered
True
>>> PhotoOfTheDay.objects.annotate(día=models.F("day")).order_by("día").totally_ordered
False  # expected True

I'm cooking up a solution 🍲

Change History (5)

comment:1 by Simon Charette, 4 weeks ago

Triage Stage: Unreviewed → Accepted
Type: Bug → Cleanup/optimization

QuerySet.totally_ordered will always have false negatives (e.g. order_by(Upper("pk")) is totally ordered for case-insensitive pks) but as long as this ticket is limited to resolving string references to .annotation entries that are Col objects then I think this ticket is acceptable as an optimization.

Since the return type of totally_ordered is False even when it's unknown (but potentially True) I consider this is an optimization and not a bug.

Last edited 4 weeks ago by Simon Charette (previous) (diff)

comment:2 by Jacob Walls, 4 weeks ago

Severity: Release blocker → Normal
Summary: QuerySet.totally_ordered property doesn't fathom annotations → QuerySet.totally_ordered property doesn't fathom aliases of pure fields

comment:3 by Jacob Walls, 4 weeks ago

Has patch: set

​PR

comment:4 by Jacob Walls, 3 weeks ago

Triage Stage: Accepted → Ready for checkin

Simon reviewed and approved on GitHub.

comment:5 by Jacob Walls <jacobtylerwalls@…>, 3 weeks ago

Resolution: → fixed
Status: assigned → closed

In 1a00120:

Fixed #37278 -- Made QuerySet.totally_ordered understand aliases to pure Col/ColPairs.

Thanks Simon Charette for the review.

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