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 Mariusz Felisiak, 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: UnreviewedAccepted

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."

raise FieldError(
    'Expression contains mixed types: %s, %s. You must set output_field to %s.'
    % (output_field.__class__.__name__, source.__class__.__name__, source.__class__.__name__)
)
Last edited 5 years ago by Mariusz Felisiak (previous) (diff)

comment:2 by Shubham Bhagat, 5 years ago

Owner: changed from nobody to Shubham Bhagat
Status: newassigned

comment:3 by Mariusz Felisiak, 5 years ago

Has patch: set

comment:4 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In 34f8eeea:

Fixed #30548 -- Improved exception when expression contains mixed types.

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