#31967 closed Bug (fixed)
Resolving an output_field for mixed types crashes in database functions.
Reported by: | Matt Hegarty | Owned by: | Hasan Ramezani |
---|---|---|---|
Component: | Documentation | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Matt Hegarty, Simon Charette | 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
This is related to #31919. Please see attached for full stack trace.
This is a regression because it does not occur in 3.1.
To reproduce:
git clone https://github.com/matthewhegarty/rest-framework-tutorial cd rest-framework-tutorial git checkout bug-expression-contains-mixed-types python3 -m venv env source env/bin/activate pip install -r requirements.txt ./manage.py migrate ./manage.py runserver
browse to http://localhost:8000/snippets/
Attachments (1)
Change History (16)
by , 4 years ago
Attachment: | stack_trace.txt added |
---|
comment:1 by , 4 years ago
Cc: | added |
---|
comment:2 by , 4 years ago
Cc: | added |
---|---|
Component: | Uncategorized → Database layer (models, ORM) |
Severity: | Normal → Release blocker |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Bug |
Thanks for this report. It's a regression in 1e38f1191de21b6e96736f58df57dfb851a28c1f but it's not related with fields subclasses. I was able to reproduce it with a builtin DecimalField()
, e.g.
>>> list(MyModel.objects.annotate(x=Coalesce(F('field_decimal'), 0)) ... File "django/django/db/models/expressions.py", line 306, in _resolve_output_field source.__class__.__name__, django.core.exceptions.FieldError: Expression contains mixed types: DecimalField, IntegerField. You must set output_field.
comment:3 by , 4 years ago
Summary: | Django fails with FieldError: Expression contains mixed types: MoneyField, IntegerField. You must set output_field. → Resolving an output_field for mixed types crashes in database functions. |
---|
comment:4 by , 4 years ago
Component: | Database layer (models, ORM) → Documentation |
---|---|
Severity: | Release blocker → Normal |
We didn't resolve output fields for Value()
expressions before 1e38f1191de21b6e96736f58df57dfb851a28c1f, so they were ignored when resolving an output field for functions. Now, we take them into account, that's why an error is raised. IMO it's an expected behavior. It should be enough to mention this in release notes.
You can fix this by using Decimal('0')
, e.g. Coalesce(F('field_decimal'), Decimal('0')))
follow-up: 6 comment:5 by , 4 years ago
When I try to set the second arg to Decimal('0'), I get:
django.db.utils.ProgrammingError: can't adapt type 'DecimalField'
Also, I think this workaround will be confusing, because the error message tells you to set the output_field. I did try this when I first encountered the error and it didn't work. So it's very likely that others will do the same rather than checking the release notes.
comment:6 by , 4 years ago
Replying to Matt Hegarty:
When I try to set the second arg to Decimal('0'), I get:
django.db.utils.ProgrammingError: can't adapt type 'DecimalField'
True, sorry, setting output_field
is the best way.
comment:7 by , 4 years ago
I confirm that this call does not crash:
Coalesce( SubquerySum( "price__price" ), 0, output_field=DecimalField() ),
Thank you for your help.
comment:8 by , 4 years ago
So we've got two approach we could take here.
Determine that it was never meant to work and document it or use a clemency based approach where we deprecate this behaviour by ignoring the output_field
of Value
source expressions in Expression._resolve_output_field
when it fails with FieldError
.
I'm surprised Coalesce(F('field_decimal'), Decimal('0')))
doesn't actually work.
comment:9 by , 4 years ago
I think it's enough to document this change in release notes, because it's easy to fix on the users side, it's explicit, and it was never meant to work.
comment:10 by , 4 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:13 by , 15 months ago
Hello! I see that this is fixed, but I still don't understand how to workaround this issue.
I'm working on updating a Django app using this library: https://github.com/annemacedo-tw/django-spillway
I get this error when using it:
django.core.exceptions.FieldError: Expression contains mixed types: MultiPolygonField, FloatField. You must set output_field.
However I don't get the same error on Django 3.1
As far as I debugged, this line here yields these two fields https://github.com/annemacedo-tw/django-spillway/blob/fa008ef0db5f3434c1ef438f66231f35a90b1e6d/spillway/query.py#L153
SimplifyPreserveTopology(F(geometry), Value(0.0439453125))
The first one being a MultiPolygonField and the other one being a FloatField. I tried setting the output_field and recreating the source_expressions but it didn't really work.
sql = SimplifyPreserveTopology(sql, tilew) sql_field = sql.source_expressions[0] sql_value = sql.source_expressions[1] sql_value.output_field = models.MultiPolygonField() sql.set_source_expressions([sql_field, sql_value])
(I understand that I'm setting an output field for MultiPolygonField here instead of FloatField as should be expected, I set it up to avoid the mixed types error)
File "/usr/local/lib/python3.9/site-packages/django/contrib/gis/db/models/fields.py", line 194, in get_prep_value raise ValueError('Cannot use object with type %s for a spatial lookup parameter.' % type(obj).__name__) ValueError: Cannot use object with type float for a spatial lookup parameter.
TL;DR how can I work around this issue when using GeoDjango?
Another thing I noticed is that ever since GeoManager was deprecated way back in 1.10, django.contrib.gis.db.models.FloatField resolves to django.db.models.fields.FloatField. I wonder if the gis FloatField would play nicely with MultiPolygonField.
(Pdb) from django.contrib.gis.db import models (Pdb) models.FloatField <class 'django.db.models.fields.FloatField'>
I wish I could open a separate ticket for this, but I'm working on legacy code that I don't understand so well, so I don't know how to do a minimal version out of it.
follow-up: 15 comment:14 by , 15 months ago
I suggest posting your question in the GeoDjango category of the forum instead.
I suspect you only have to pass output_field=models.MultiPolygonField()
when initializing your SimplifyPreserveTopology
as hinted by the error message: You must set output_field.
SimplifyPreserveTopology( F(geometry), 0.0439453125, output_field=models.MultiPolygonField() )
comment:15 by , 15 months ago
Replying to Simon Charette:
I suggest posting your question in the GeoDjango category of the forum instead.
I suspect you only have to pass
output_field=models.MultiPolygonField()
when initializing yourSimplifyPreserveTopology
as hinted by the error message:You must set output_field.
SimplifyPreserveTopology( F(geometry), 0.0439453125, output_field=models.MultiPolygonField() )
I believe it worked! Been trying to fix this bug for a whole week :)
stack trace