Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#31910 closed Bug (fixed)

Django geo model aggregate union fails with distinct

Reported by: Eran Keydar Owned by: Simon Charette
Component: GIS Version: 3.1
Severity: Normal Keywords:
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 (last modified by Eran Keydar)

The following code works ok on django 2.2, and fails on django 3 and 3.1
(without the distinct if works ok)

Database is postgis

class PolyModel(models.Model):
    name = models.CharField(max_length=50)
    geom = models.PolygonField()


def run_example():
    PolyModel.objects.all().delete()
    for x in range(1, 10):
        PolyModel.objects.create(
            name="poly_{}".format(x),
            geom=Point(34+x, 34+x).buffer(0.01)
        )
    print(PolyModel.objects.distinct().aggregate(u=Union('geom')))

Output:

<ipython-input-5-c368ff40e50b> in <module>
----> 1 PolyModel.objects.distinct().aggregate(u=Union('geom'))

~/.virtualenvs/bug1/lib/python3.6/site-packages/django/db/models/query.py in aggregate(self, *args, **kwargs)
    396             if not query.annotations[alias].contains_aggregate:
    397                 raise TypeError("%s is not an aggregate expression" % alias)
--> 398         return query.get_aggregation(self.db, kwargs)
    399 
    400     def count(self):

~/.virtualenvs/bug1/lib/python3.6/site-packages/django/db/models/sql/query.py in get_aggregation(self, using, added_aggregate_names)
    503 
    504         converters = compiler.get_converters(outer_query.annotation_select.values())
--> 505         result = next(compiler.apply_converters((result,), converters))
    506 
    507         return dict(zip(outer_query.annotation_select, result))

~/.virtualenvs/bug1/lib/python3.6/site-packages/django/db/models/sql/compiler.py in apply_converters(self, rows, converters)
   1096                 value = row[pos]
   1097                 for converter in convs:
-> 1098                     value = converter(value, expression, connection)
   1099                 row[pos] = value
   1100             yield row

~/.virtualenvs/bug1/lib/python3.6/site-packages/django/contrib/gis/db/backends/postgis/operations.py in converter(value, expression, connection)
    385 
    386         def converter(value, expression, connection):
--> 387             return None if value is None else GEOSGeometryBase(read(value), geom_class)
    388         return converter
    389 

~/.virtualenvs/bug1/lib/python3.6/site-packages/django/contrib/gis/geos/prototypes/io.py in read(self, wkb)
    151             return wkb_reader_read(self.ptr, wkb_s, len(wkb_s))
    152         elif isinstance(wkb, (bytes, str)):
--> 153             return wkb_reader_read_hex(self.ptr, wkb, len(wkb))
    154         else:
    155             raise TypeError

~/.virtualenvs/bug1/lib/python3.6/site-packages/django/contrib/gis/geos/libgeos.py in __call__(self, *args)
    150 
    151     def __call__(self, *args):
--> 152         return self.func(*args)
    153 
    154     @cached_property

~/.virtualenvs/bug1/lib/python3.6/site-packages/django/contrib/gis/geos/prototypes/threadsafe.py in __call__(self, *args)
     45         # Call the threaded GEOS routine with the pointer of the context handle
     46         # as the first argument.
---> 47         return self.cfunc(self.thread_context.handle.ptr, *args)
     48 
     49     def __str__(self):

ArgumentError: argument 3: <class 'TypeError'>: wrong type


Thanks

Change History (12)

comment:1 by Eran Keydar, 4 years ago

Description: modified (diff)

comment:2 by Eran Keydar, 4 years ago

Description: modified (diff)

comment:3 by Eran Keydar, 4 years ago

Description: modified (diff)

These are the queries from django 2.2 and django 3.1

Version 0, edited 4 years ago by Eran Keydar (next)

comment:4 by Eran Keydar, 4 years ago

Same error with the following expression (ok on 2.2)

PolyModel.objects.annotate(c=BoundingCircle('geom')).aggregate(u=Union('c'))

comment:5 by Simon Charette, 4 years ago

Owner: changed from nobody to Simon Charette
Severity: NormalRelease blocker
Status: newassigned
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Confirmed regression caused by fff5186d3215e0ba06e47090226169f2230786b0

comment:6 by Simon Charette, 4 years ago

Has patch: set

comment:7 by Mariusz Felisiak, 4 years ago

Severity: Release blockerNormal

PR

This is a regression introduced in Django 3.0 which is in extended support, so it's not a release blocker.

comment:8 by Mariusz Felisiak, 4 years ago

Patch needs improvement: set

comment:9 by Simon Charette, 4 years ago

Patch needs improvement: unset

comment:10 by Mariusz Felisiak, 4 years ago

Triage Stage: AcceptedReady for checkin

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

Resolution: fixed
Status: assignedclosed

In c2d49267:

Fixed #31910 -- Fixed crash of GIS aggregations over subqueries.

Regression was introduced by fff5186 but was due a long standing issue.

AggregateQuery was abusing Query.subquery: bool by stashing its
compiled inner query's SQL for later use in its compiler which made
select_format checks for Query.subquery wrongly assume the provide
query was a subquery.

This patch prevents that from happening by using a dedicated
inner_query attribute which is compiled at a later time by
SQLAggregateCompiler.

Moving the inner query's compilation to SQLAggregateCompiler.compile
had the side effect of addressing a long standing issue with
aggregation subquery pushdown which prevented converters from being
run. This is now fixed as the aggregation_regress adjustments
demonstrate.

Refs #25367.

Thanks Eran Keydar for the report.

comment:12 by GitHub <noreply@…>, 4 years ago

In 13b6fff:

Refs #31910 -- Fixed GeoQuerySetTest.test_geoagg_subquery() test on Oracle 18c.

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