Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#31659 closed Bug (fixed)

Django loses information regarding the type of grouping columns

Reported by: Thodoris Sotiropoulos Owned by: Mariusz Felisiak
Component: Database layer (models, ORM) Version: 3.1
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have the following query

expr = ExpressionWrapper((Value(4) * Value(1)), output_field=FloatField())
aggr = ExpressionWrapper(Avg(F('col'), output_field=FloatField()), output_field=FloatField())
Model.objects.using('default').annotate(expr=expr).annotate(aggr=aggr)

This produces the following exception

Traceback (most recent call last%):
  File "driver_sqlite.py", line 19, in <module>
    for r in ret2:
  File "/dir/.env/lib/python3.6/site-packages/Django-3.2-py3.6.egg/django/db/models/query.py", line 287, in __iter__
    self._fetch_all()
  File "/dir/.env/lib/python3.6/site-packages/Django-3.2-py3.6.egg/django/db/models/query.py", line 1305, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/dir/.env/lib/python3.6/site-packages/Django-3.2-py3.6.egg/django/db/models/query.py", line 111, in __iter__
    for row in compiler.results_iter(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size):
  File "/dir/.env/lib/python3.6/site-packages/Django-3.2-py3.6.egg/django/db/models/sql/compiler.py", line 1108, in results_iter
    results = self.execute_sql(MULTI, chunked_fetch=chunked_fetch, chunk_size=chunk_size)
  File "/dir/.env/lib/python3.6/site-packages/Django-3.2-py3.6.egg/django/db/models/sql/compiler.py", line 1143, in execute_sql
    sql, params = self.as_sql()
  File "/dir/.env/lib/python3.6/site-packages/Django-3.2-py3.6.egg/django/db/models/sql/compiler.py", line 498, in as_sql
    extra_select, order_by, group_by = self.pre_sql_setup()
  File "/dir/.env/lib/python3.6/site-packages/Django-3.2-py3.6.egg/django/db/models/sql/compiler.py", line 60, in pre_sql_setup
    group_by = self.get_group_by(self.select + extra_select, order_by)
  File "/dir/.env/lib/python3.6/site-packages/Django-3.2-py3.6.egg/django/db/models/sql/compiler.py", line 142, in get_group_by
    sql, params = expr.select_format(self, sql, params)
  File "/dir/.env/lib/python3.6/site-packages/Django-3.2-py3.6.egg/django/db/models/expressions.py", line 386, in select_format
    return self.output_field.select_format(compiler, sql, params)
  File "/dir/.env/lib/python3.6/site-packages/Django-3.2-py3.6.egg/django/utils/functional.py", line 48, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
  File "/dir/.env/lib/python3.6/site-packages/Django-3.2-py3.6.egg/django/db/models/expressions.py", line 270, in output_field
    raise FieldError('Cannot resolve expression type, unknown output_field')
django.core.exceptions.FieldError: Cannot resolve expression type, unknown output_field

This issue manifests on the latest master version of Django and I think it's due to the fix of https://code.djangoproject.com/ticket/31651.
Specifically, Django loses the type of the grouping column (i.e., "expr") and that's why it throws this expression.

Maybe a patch to this issue is the following

--- a/django/db/models/expressions.py
+++ b/django/db/models/expressions.py
@@ -864,7 +864,8 @@
         return [self.expression]

     def get_group_by_cols(self, alias=None):
-        return self.expression.get_group_by_cols(alias=alias)
+        return [ExpressionWrapper(e, output_field=self.output_field)
+                for e in self.expression.get_group_by_cols(alias=alias)]

     def as_sql(self, compiler, connection):
         return self.expression.as_sql(compiler, connection)

Change History (7)

comment:1 by Thodoris Sotiropoulos, 5 years ago

Summary: Django loses the type of grouping columnsDjango loses information regarding the type of grouping columns

comment:2 by Simon Charette, 5 years ago

Component: UncategorizedDatabase layer (models, ORM)
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

I wonder if a better approach would be to make BaseExpression.select_format return sql, params when self.output_field is None.

I assume you don't get a crash when adding output_field=IntegerField() to your Value instances? If it's the case this ticket is related to #30446 (PR).

comment:3 by Thodoris Sotiropoulos, 5 years ago

I assume you don't get a crash when adding output_field=IntegerField() to your Value instances?

Yes, if I add this information, I don't get a crash.

comment:4 by Mariusz Felisiak, 5 years ago

Severity: NormalRelease blocker
Version: master3.1

comment:5 by Mariusz Felisiak, 5 years ago

Has patch: set
Owner: changed from nobody to Mariusz Felisiak
Status: newassigned

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

Resolution: fixed
Status: assignedclosed

In aeb8996a:

Fixed #31659 -- Made ExpressionWrapper preserve output_field for combined expressions.

Regression in df32fd42b84cc6dbba173201f244491b0d154a63.

Thanks Simon Charette for the review.

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

In 42f5f2d7:

[3.1.x] Fixed #31659 -- Made ExpressionWrapper preserve output_field for combined expressions.

Regression in df32fd42b84cc6dbba173201f244491b0d154a63.

Thanks Simon Charette for the review.

Backport of aeb8996a6706cad3e96d8221760c1cb408ee7ed9 from master

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