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()
toWhereNode
- Add a
resolve_expression()
toWhereNode
(unsure what this should return - requires significant testing with Case/When)
Might be required:
- Add an
output_field=BooleanField()
toQ
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 , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 8 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:4 by , 7 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:5 by , 6 years ago
Cc: | added |
---|
comment:6 by , 5 years ago
Cc: | added |
---|
comment:7 by , 5 years ago
Cc: | added |
---|
comment:8 by , 4 years ago
Cc: | added |
---|
comment:9 by , 4 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:10 by , 4 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 , 4 years ago
Cc: | added |
---|
comment:12 by , 4 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:13 by , 3 years ago
Has patch: | set |
---|
comment:14 by , 3 years ago
Needs documentation: | unset |
---|
comment:15 by , 3 years ago
Needs tests: | unset |
---|
comment:16 by , 3 years ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
I took a quick look at this and ran into circular import problems when setting
output_field=BooleanField()
or importingExpression
(both fromdjango.db.models
). This will need some careful thought about howWhereNode
andQ
interact.