Opened 10 years ago

Closed 10 years ago

Last modified 6 years ago

#10766 closed (fixed)

Annotate() doesn't error if given a previously defined annotation alias

Reported by: aseering@… Owned by:
Component: Database layer (models, ORM) Version: 1.0
Severity: 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 (last modified by Alex Gaynor)

I recently wrote the following code:

classes = classes.annotate(num_students=Count('anchor__child_set__userbit_qsc'))
classes = classes.annotate(total_max_student=Sum('sections__num_students'))

The second line above (correctly) generated an error for me because the model referenced by "sections" doesn't contain a field named "num_students".

However, the error message returned by the exception was as follows:

Cannot resolve keyword 'num_students' into field. Choices are: [long valid list of fields], num_students

This confused me for a good while (until I realized that it was my mistake for using the wrong field name). On experimentation, the "num_students" in the list of fields does in fact come from the first annotate call.

It's hardly a critical failure; but it seems like poor form to declare a field as "invalid" then promptly list it as one of the valid options... More generally, it seems like that list just lists all annotated fields, which is only correct if you're not doing a join first.

I suspect that the fix is on line 1678 or so of django/db/models/sql/ (in particular, maybe it needs to use a different/new function on the Options class to get the keys properly?); but I don't know enough of those internals to know that I'm doing the right thing there. I could work up a patch that does this if no one more-experienced feels like it or knows of a good reason not to.

Change History (5)

comment:1 Changed 10 years ago by Alex Gaynor

Description: modified (diff)

Please use preview :)

comment:2 Changed 10 years ago by Russell Keith-Magee

Component: UncategorizedORM aggregation
milestone: 1.1
Owner: nobody deleted
Summary: Confusing error message on invalid annotate aggregate fieldAnnotate() doesn't error if given a previously defined annotation alias
Triage Stage: UnreviewedAccepted

You're partially correct here. The example you provide _is_ in error - your second annotation referecnes 'sectionsnum_students', not 'num_students'. The error is being raised because 'sections' isn't a field on your model that can be followed; 'num_students' is on of the list of options because it was a previously added annotation.

However, you are correct in saying that num_students shouldn't be on the list of valid options. It _should_ be on the list of options when looking up filters or F() clauses, but you shouldn't be able to add an annotation based on a previous annotation. If you update your example to use 'num_students' on the second line, you get an SQL error (under postgres: ProgrammingError: column "num_authors" does not exist LINE 1)

I've updated the ticket title to reflect the actual problem; marking for v1.1.

comment:3 Changed 10 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

(In [10521]) Fixed #10766 -- Raise an error when annotate() references another aggreagte(). Thanks to aseering@… for the report.

comment:4 Changed 7 years ago by Jacob

milestone: 1.1

Milestone 1.1 deleted

comment:5 Changed 6 years ago by Anssi Kääriäinen

Component: ORM aggregationDatabase layer (models, ORM)
Note: See TracTickets for help on using tickets.
Back to Top