#34838 closed Bug (fixed)
GeoDjango database functions incompatible with GeneratedField
| Reported by: | Paolo Melchiorre | Owned by: | Paolo Melchiorre |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Release blocker | Keywords: | field, database, generated, output_field |
| Cc: | 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
GeoDjango model functions raise an incompatibility error when invoked on generated fields.
Steps
Steps to reproduce the error.
Model
from django.contrib.gis.db import models
class Area(models.Model):
polygon = models.PolygonField()
centroid = models.GeneratedField(
db_persist=True,
expression=models.functions.Centroid("polygon"),
output_field=models.PointField(),
)
Query
>>> from django.contrib.gis.geos import Polygon
>>> Area.objects.create(polygon=Polygon(((0,0), (2,0), (2,2), (0,2), (0,0))))
>>> Area.objects.values_list(models.functions.AsWKT("centroid"), models.functions.AsWKT("polygon"))
Traceback
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/home/paulox/Projects/django/django/db/models/manager.py", line 87, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/paulox/Projects/django/django/db/models/query.py", line 1629, in annotate
return self._annotate(args, kwargs, select=True)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/paulox/Projects/django/django/db/models/query.py", line 1677, in _annotate
clone.query.add_annotation(
File "/home/paulox/Projects/django/django/db/models/sql/query.py", line 1185, in add_annotation
annotation = annotation.resolve_expression(self, allow_joins=True, reuse=None)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/paulox/Projects/django/django/contrib/gis/db/models/functions.py", line 80, in resolve_expression
raise TypeError(
TypeError: AsWKT function requires a GeometryField in position 1, got GeneratedField.
Patch
diff --git a/django/contrib/gis/db/models/functions.py b/django/contrib/gis/db/models/functions.py
index 19da355d28..90ca87a051 100644
--- a/django/contrib/gis/db/models/functions.py
+++ b/django/contrib/gis/db/models/functions.py
@@ -76,6 +76,8 @@ class GeoFuncMixin:
source_fields = res.get_source_fields()
for pos in self.geom_param_pos:
field = source_fields[pos]
+ if field.generated:
+ field = field.output_field
if not isinstance(field, GeometryField):
raise TypeError(
"%s function requires a GeometryField in position %s, got %s."
@@ -86,7 +88,7 @@ class GeoFuncMixin:
)
)
- base_srid = res.geo_field.srid
+ base_srid = res.geo_field.srid if not res.geo_field.generated else res.geo_field.output_field.srid
for pos in self.geom_param_pos[1:]:
expr = res.source_expressions[pos]
expr_srid = expr.output_field.srid
Change History (10)
comment:1 by , 2 years ago
| Needs tests: | set |
|---|---|
| Owner: | changed from to |
| Status: | new → assigned |
| Triage Stage: | Unreviewed → Accepted |
comment:3 by , 2 years ago
Replying to Simon Charette:
Maybe this can be done at the
GeneratedField.get_collevel where the returnedColinstance defaults tooutput_field=self.output_fieldinstead ofself.
django/db/models/fields/generated.py
diff --git a/django/db/models/fields/generated.py b/django/db/models/fields/generated.py index 0980be98af..948d11d003 100644
a b def contribute_to_class(self, *args, **kwargs): 48 48 for lookup_name, lookup in self.output_field.get_class_lookups().items(): 49 49 self.register_lookup(lookup, lookup_name=lookup_name) 50 50 51 def get_col(self, alias, output_field=None): 52 if output_field is None: 53 output_field = self.output_field 54 return super().get_col(alias, output_field) 55 51 56 def generated_sql(self, connection): 52 57 return self._resolved_expression.as_sql( 53 58 compiler=connection.ops.compiler("SQLCompiler")(
Thanks for the suggestion Simon. After reading your comment I had defined the get_col function exactly as you then sent it. I think I also wrote a couple of correct tests. I'm going to open the PR.
comment:6 by , 2 years ago
| Component: | GIS → Database layer (models, ORM) |
|---|---|
| Keywords: | output_field added; gis feodjango removed |
comment:7 by , 2 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
I suspect we'll want to avoid adding many
output_field = field; if field.generated: field.output_fieldall over the codebase and that we should favour an approach where expression resolving (seesql.query.Query.resolve_ref) returns aColwith the properoutput_field.Maybe this can be done at the
GeneratedField.get_collevel where the returnedColinstance defaults tooutput_field=self.output_fieldinstead ofself.django/db/models/fields/generated.py