Opened 8 years ago

Closed 3 years ago

#27021 closed New feature (fixed)

Add explicit support for Q object annotations

Reported by: Josh Smeaton Owned by: Ian Foote
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: josh.smeaton@…, Ryan Hiebert, Balazs Endresz, Matthijs Kooijman, Petr Přikryl, Ian Foote 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

A common pattern that has arisen with expressions is to annotate a Q object to the query without going through the Case/When ceremony.

Model.objects.annotate(boolfield=ExpressionWrapper(Q(field__gte=4), output_field=BooleanField()))

We've tried to discourage this pattern because the Case/When structure is a lot more powerful and correct. Sometimes the power isn't required and users really just want to annotate a boolean expression. Q objects and the WhereNode it resolves to aren't really proper expressions, but they could be. Case/When uses Q objects internally, which is why it's important to test Case/When for any major changes to Q and WhereNode.

Explicit support for Q object annotations should implement an implicit BooleanField as the output_field. I believe this is only required on the WhereNode since that is what is ultimately resolved. Once this happens, the above query becomes a lot nicer.

Model.objects.annotate(boolfield=Q(field__gte=4))

For this to work the following is definitely required:

  • Add an output_field=BooleanField() to WhereNode
  • Add a resolve_expression() to WhereNode (unsure what this should return - requires significant testing with Case/When)

Might be required:

  • Add an output_field=BooleanField() to Q

Tests:

  • Adding a subquery containing a Q annotation to a parent query
  • Adding a subquery containing a Case/When annotation to a parent query (if it's not already tested)
  • Standard tests that most other annotations already implement
  • Nullable fields used in Q annotations

Change History (17)

comment:1 by Ian Foote, 8 years ago

Owner: changed from nobody to Ian Foote
Status: newassigned

comment:2 by Ian Foote, 8 years ago

I took a quick look at this and ran into circular import problems when setting output_field=BooleanField() or importing Expression (both from django.db.models). This will need some careful thought about how WhereNode and Q interact.

Last edited 8 years ago by Ian Foote (previous) (diff)

comment:3 by Ian Foote, 8 years ago

Owner: Ian Foote removed
Status: assignednew

comment:4 by Sergey Fedoseev, 6 years ago

Owner: set to Sergey Fedoseev
Status: newassigned

comment:5 by Ryan Hiebert, 5 years ago

Cc: Ryan Hiebert added

comment:6 by Balazs Endresz, 4 years ago

Cc: Balazs Endresz added

comment:7 by Matthijs Kooijman, 4 years ago

Cc: Matthijs Kooijman added

comment:8 by Petr Přikryl, 3 years ago

Cc: Petr Přikryl added

comment:9 by Sergey Fedoseev, 3 years ago

Owner: Sergey Fedoseev removed
Status: assignednew

comment:10 by Ian Foote, 3 years ago

While working on #470, I've created a proof of concept implementation that allows annotating lookups without requiring a Q object. (See https://github.com/django/django/commit/d67c858198903839b6b5bf15e04cbf7a7072190e.)

Instead of Model.objects.annotate(boolfield=Q(field__gte=4)), we would instead write something like Model.objects.annotate(boolfield=F('field') >= 4).

This is more flexible than the original proposal here because it allows comparing expressions other than fields. For example: Model.objects.annotate(boolfield=Random() > 0.5).

If this proposed API is accepted, I'll assign this ticket to myself again and polish up my proof of concept.

comment:11 by Ian Foote, 3 years ago

Cc: Ian Foote added

comment:12 by Ian Foote, 3 years ago

Owner: set to Ian Foote
Status: newassigned

comment:13 by Jacob Walls, 3 years ago

Has patch: set

comment:14 by Mariusz Felisiak, 3 years ago

Needs documentation: unset

comment:15 by Mariusz Felisiak, 3 years ago

Needs tests: unset

comment:16 by Mariusz Felisiak, 3 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

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

Resolution: fixed
Status: assignedclosed

In f42ccdd8:

Fixed #27021 -- Allowed lookup expressions in annotations, aggregations, and QuerySet.filter().

Thanks Hannes Ljungberg and Simon Charette for reviews.

Co-authored-by: Mariusz Felisiak <felisiak.mariusz@…>

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