#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 , 16 months ago
Needs tests: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 16 months ago
Replying to Simon Charette:
Maybe this can be done at the
GeneratedField.get_col
level where the returnedCol
instance defaults tooutput_field=self.output_field
instead 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 , 15 months ago
Component: | GIS → Database layer (models, ORM) |
---|---|
Keywords: | output_field added; gis feodjango removed |
comment:7 by , 15 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
I suspect we'll want to avoid adding many
output_field = field; if field.generated: field.output_field
all over the codebase and that we should favour an approach where expression resolving (seesql.query.Query.resolve_ref
) returns aCol
with the properoutput_field
.Maybe this can be done at the
GeneratedField.get_col
level where the returnedCol
instance defaults tooutput_field=self.output_field
instead ofself
.django/db/models/fields/generated.py