Opened 5 years ago
Closed 5 years ago
#30548 closed Cleanup/optimization (fixed)
Improve exceptions about mixed types in Expressions.
Reported by: | Keryn Knight | Owned by: | Shubham Bhagat |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | expressions orm |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
The source which raises the exception has enough information to say both what types were found, and which of those were unexpected, and probably have a useful repr()
In the test suite, the unexpected output types encountered seem to be DurationField
and IntegerField
, so a more thorough message might be something like:
Expression repr(self) contained mixed types: DateField, DurationField. DurationField was unexpected; you must set the output_field= for this Expression to either DurationField(), DateField() or ...
(??? I dunno, some concrete explanation of what the output_field has to be/implement if you're not going to use any of the builtins)
The merit of including the repr is arguable, as the Expression
may not what the user put in (eg: in the test suite it always seems to be a CombinedExpression(lhs, connector, rhs)
) but it gives more of a hint in a query which contains multiple expressions (either nested or separate) as to which one is actually causing the problem vs just being told "something was wrong. Put an output_field= everywhere until it ceases, your guess is as good as mine"; at the very least the word Expression could be replaced with the class name which is actually raising it.
Change History (4)
comment:1 by , 5 years ago
Easy pickings: | set |
---|---|
Summary: | Improve exception message when BaseExpression raises "'Expression contains mixed types. You must set output_field.'" → Improve exceptions about mixed types in Expressions. |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 5 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Agreed, we can add types to this exception but I would like to keep it simple, e.g. "Expression contains mixed types: FloatField, IntegerField. You must set output_field to IntegerField."