[Django] #30188: Aggregate annotation, Case() - When(): AssertionError No exception message supplied
Django
supplied
-------------------------------------+-------------------------------------
Reporter: Lukas | Owner: nobody
Klement |
Type: Bug | Status: new
Component: Database | Version: 2.1
layer (models, ORM) | Keywords: query, aggregate,
Severity: Normal | case/when
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Aggregating annotations works for simple Sum, Count, etc. operations, but
fails when the Sum() contains a Case() When() operation.
Example:
Objective: sum the number of None values for the annotated field
field_name across a queryset.
{{{
field_name = <name of annotated field>
missing_data_agg=Sum(
Case(
When(**{field_name: None}, then=Value(1)),
default=Value(0),
output_field=IntegerField()
)
)
}}}
Expected behaviour: calling
queryset.aggregate(missing_data=missing_data_agg) returns a dict
{'missing_data': <value>}
Actual behaviour: AssertionError No exception message supplied is
returned.
When field_name is set to a field that exists on the model of the
queryset, the aggregation works. When field_name is set to a field that is
neither in the model of the queryset, nor has it been annotated to the
queryset, the exception returned is as expected: Cannot resolve keyword
<name of the non existing field> into field . Choices are: <list of model
and annotated fields. In this list, the field_name that has been annotated
to the queryset is shown -> hence it should work, hence I assume this is a
Django bug.
Stacktrace:
{{{
<Line with the .aggregate(...)>
return query.get_aggregation(self.db, kwargs) ...
/Users/{{path}}/lib/python3.6/site-packages/django/db/models/sql/query.py
in get_aggregation
expression, col_cnt =
inner_query.rewrite_cols(expression, col_cnt) ...
/Users/{{path}}/lib/python3.6/site-packages/django/db/models/sql/query.py
in rewrite_cols
new_expr, col_cnt = self.rewrite_cols(expr, col_cnt) ...
/Users/{{path}}/lib/python3.6/site-packages/django/db/models/sql/query.py
in rewrite_cols
new_expr, col_cnt = self.rewrite_cols(expr, col_cnt) ...
/Users/{{path}}/lib/python3.6/site-packages/django/db/models/sql/query.py
in rewrite_cols
new_expr, col_cnt = self.rewrite_cols(expr, col_cnt) ...
/Users/{{path}}/lib/python3.6/site-packages/django/db/models/sql/query.py
in rewrite_cols
new_expr, col_cnt = self.rewrite_cols(expr, col_cnt) ...
/Users/{{path}}/lib/python3.6/site-packages/django/db/models/sql/query.py
in rewrite_cols
new_expr, col_cnt = self.rewrite_cols(expr, col_cnt) ...
/Users/{{path}}/lib/python3.6/site-packages/django/db/models/sql/query.py
in rewrite_cols
new_expr, col_cnt = self.rewrite_cols(expr, col_cnt) ...
/Users/{{path}}/lib/python3.6/site-packages/django/db/models/sql/query.py
in rewrite_cols
annotation.set_source_expressions(new_exprs) ...
/Users/{{path}}/lib/python3.6/site-
packages/django/db/models/expressions.py in set_source_expressions
assert not exprs
}}}
The database is a PostgreSQL database, the Django version is 2.1.7, the
Python version is 3.6.4.
--
Ticket URL: <https://code.djangoproject.com/ticket/30188>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage:
case/when | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
Stacktrace:
Python version is 3.6.4. The problem also exists under Django version
2.2b1
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:1>
Django
supplied
-------------------------------------+-------------------------------------
Component: Database layer | Version: 2.1
(models, ORM) |
Keywords: query, aggregate, | Triage Stage:
case/when | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => needsinfo
Comment:
Hey Lukas,
Given the assertion error it looks like there's something wrong here but
because of the limited data you provided it's almost impossible for
volunteers to reproduce and confirm Django is actually at fault.
Please re-open this ticket if you can provide a reduced test case for your
problem. This should include a set of minimal models calls to build the
queryset that crashes on `.aggregate`. It would also be great if you could
test against the current Django 2.2 beta (2.2b1) and confirm whether or
not it's fixed there.
Thank you for your understanding.
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:2>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
case/when | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: needsinfo =>
Old description:
> Python version is 3.6.4. The problem also exists under Django version
> 2.2b1
New description:
Aggregating annotations works for simple Sum, Count, etc. operations, but
fails when the Sum() contains a Case() When() operation.
To reproduce the issue, a simplified scenario ():
Suppose you have recipes (e.g. Pasta dough, Tomato Sauce, etc.), which are
linked to dishes (e.g. Pasta with tomato sauce) and ingredients (e.g.
Flour, Water, Tomatoes). Ingredients are shared across all users, but each
user can add a different purchase price to an ingredient (-> cost). We
want to sum up the cost of a dish (i.e. summing up the cost of all
ingredients across all recipes of a dish).
**models.py**
{{{
class Ingredient(models.Model):
pass
class IngredientDetail(models.Model):
ingredient = models.ForeignKey(Ingredient, on_delete=models.CASCADE,
related_name='ingredient_details')
user = models.ForeignKey(User, limit_choices_to={'role': 0},
on_delete=models.CASCADE)
cost = models.DecimalField(blank=True, null=True, decimal_places=6,
max_digits=13)
cost_unit = models.CharField(choices=UNITS, max_length=15,
default='kg')
class Recipe(models.Model):
dishes = models.ManyToManyField(Dish, related_name='dishes_recipes',
through='RecipeDish')
ingredients = models.ManyToManyField(Ingredient,
related_name='ingredients_recipes', through='RecipeIngredient')
class RecipeDish(models.Model):
recipe = models.ForeignKey(Recipe, on_delete=models.CASCADE,
related_name='recipedish_recipe')
dish = models.ForeignKey(Dish, on_delete=models.CASCADE,
related_name='recipedish_dish')
class RecipeIngredient(models.Model):
recipe = models.ForeignKey(Recipe, on_delete=models.CASCADE,
related_name='recipeingredient_recipe')
ingredient = models.ForeignKey(Ingredient, on_delete=models.CASCADE,
related_name='recipeingredient_ingredient')
quantity = models.DecimalField(blank=True, null=True,
decimal_places=2, max_digits=6)
}}}
Different users can attach different IngredientDetail objects to a shared
Ingredient objects (one-to-many relationship). The objective is to
calculate how many ingredients added to a recipe (many-to-many
relationship through RecipeIngredient) don't have a cost value set. To
achieve this without using .extra(), we annotate the appropriate cost
field to IngredientNutrition, then sum up the missing values using a
Sum(), Case() and When() aggregation.
{{{
def aggregate_nutrition_data(recipes, user):
annotations['cost_value'] = Subquery(
IngredientDetail.objects.filter(Q(ingredient_id=OuterRef('ingredient_id'))
& Q(user=user)).values('cost')[:1]
)
ingredients =
RecipeIngredient.objects.filter(recipe__in=recipes).annotate(**annotations)
aggregators['cost_missing'] = Coalesce(Sum(
Case(
When(**{'cost_value': None}, then=Value(1)),
default=Value(0),
output_field=IntegerField()
)
), Value(0))
data = ingredients.aggregate(**aggregators)
return data
}}}
Expected behaviour: a dictionary is returned: {'cost_missing': <value>},
containing as a value the number of missing cost values.Actual behaviour:
AssertionError No exception message supplied is returned on the line
''data = ingredients.aggregate(**aggregators)'' (see abbreviated
stacktrace below).
When the aggregated field name is set to a field that exists on the model
of the queryset (i.e. is not annotated), the aggregation works. For
example: instead of using cost_value=None in the When() of the
aggregators, using quantity=None works.
Similarly, doing an aggregation over an annotated field without using
Case() and When() works. For example:
{{{
aggregators['cost'] = Coalesce(
Sum(
F('cost_value') * F('quantity') / F('cost_quantity')
output_field=DecimalField()
), Value(0)
}}}
----
**Stacktrace:**
Python version is 3.6.4. The problem also exists under Django version
2.2b1
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:3>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage:
case/when | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Lukas Klement):
I have re-opened the ticket and added a simplified example to reproduce
the issue. The issue is current in Django 2.1 and can be reproduced under
Django version 2.2b1 as well.
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:4>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage:
case/when | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> Aggregating annotations works for simple Sum, Count, etc. operations, but
> fails when the Sum() contains a Case() When() operation.
>
> When(**{'cost_value': None}, then=Value(1)),
> default=Value(0),
> output_field=IntegerField()
> )
> ), Value(0))
>
> data = ingredients.aggregate(**aggregators)
> return data
> }}}
>
> Expected behaviour: a dictionary is returned: {'cost_missing': <value>},
> containing as a value the number of missing cost values.Actual behaviour:
> AssertionError No exception message supplied is returned on the line
> ''data = ingredients.aggregate(**aggregators)'' (see abbreviated
> stacktrace below).
>
> When the aggregated field name is set to a field that exists on the model
> of the queryset (i.e. is not annotated), the aggregation works. For
> example: instead of using cost_value=None in the When() of the
> aggregators, using quantity=None works.
> Similarly, doing an aggregation over an annotated field without using
> Case() and When() works. For example:
> {{{
> aggregators['cost'] = Coalesce(
> Sum(
> F('cost_value') * F('quantity') / F('cost_quantity')
> output_field=DecimalField()
> ), Value(0)
> }}}
>
> ----
>
> **Stacktrace:**
> {{{
> Python version is 3.6.4. The problem also exists under Django version
> 2.2b1
New description:
Aggregating annotations works for simple Sum, Count, etc. operations, but
fails when the Sum() contains a Case() When() operation.
To reproduce the issue, a simplified scenario:
**models.py**
{{{
class Ingredient(models.Model):
pass
When(**{'cost_value': None}, then=Value(1)),
default=Value(0),
output_field=IntegerField()
)
), Value(0))
data = ingredients.aggregate(**aggregators)
return data
}}}
Expected behaviour: a dictionary is returned: {'cost_missing': <value>},
containing as a value the number of missing cost values.Actual behaviour:
AssertionError No exception message supplied is returned on the line
''data = ingredients.aggregate(**aggregators)'' (see abbreviated
stacktrace below).
When the aggregated field name is set to a field that exists on the model
of the queryset (i.e. is not annotated), the aggregation works. For
example: instead of using cost_value=None in the When() of the
aggregators, using quantity=None works.
Similarly, doing an aggregation over an annotated field without using
Case() and When() works. For example:
{{{
aggregators['cost'] = Coalesce(
Sum(
F('cost_value') * F('quantity') / F('cost_quantity')
output_field=DecimalField()
), Value(0)
}}}
----
**Stacktrace:**
{{{
Python version is 3.6.4. The problem also exists under Django version
2.2b1
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:5>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage:
case/when | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> Aggregating annotations works for simple Sum, Count, etc. operations, but
> fails when the Sum() contains a Case() When() operation.
>
> When(**{'cost_value': None}, then=Value(1)),
> default=Value(0),
> output_field=IntegerField()
> )
> ), Value(0))
>
> data = ingredients.aggregate(**aggregators)
> return data
> }}}
>
> Expected behaviour: a dictionary is returned: {'cost_missing': <value>},
> containing as a value the number of missing cost values.Actual behaviour:
> AssertionError No exception message supplied is returned on the line
> ''data = ingredients.aggregate(**aggregators)'' (see abbreviated
> stacktrace below).
>
> When the aggregated field name is set to a field that exists on the model
> of the queryset (i.e. is not annotated), the aggregation works. For
> example: instead of using cost_value=None in the When() of the
> aggregators, using quantity=None works.
> Similarly, doing an aggregation over an annotated field without using
> Case() and When() works. For example:
> {{{
> aggregators['cost'] = Coalesce(
> Sum(
> F('cost_value') * F('quantity') / F('cost_quantity')
> output_field=DecimalField()
> ), Value(0)
> }}}
>
> ----
>
> **Stacktrace:**
> {{{
> Python version is 3.6.4. The problem also exists under Django version
> 2.2b1
New description:
Aggregating annotations works for simple Sum, Count, etc. operations, but
fails when the Sum() contains a Case() When() operation.
To reproduce the issue, a simplified scenario:
Suppose you have recipes (e.g. Pasta dough, Tomato Sauce, etc.), which are
linked to dishes (e.g. Pasta with tomato sauce) and ingredients (e.g.
Flour, Water, Tomatoes). Ingredients are shared across all users, but each
user can add a different purchase price to an ingredient (-> cost). We
want to sum up the cost of a dish (i.e. summing up the cost of all
ingredients across all recipes of a dish).
**models.py**
{{{
class Ingredient(models.Model):
pass
class IngredientDetail(models.Model):
ingredient = models.ForeignKey(Ingredient, on_delete=models.CASCADE,
related_name='ingredient_details')
user = models.ForeignKey(User, limit_choices_to={'role': 0},
on_delete=models.CASCADE)
cost = models.DecimalField(blank=True, null=True, decimal_places=6,
max_digits=13)
class Recipe(models.Model):
When(**{'cost_value': None}, then=Value(1)),
default=Value(0),
output_field=IntegerField()
)
), Value(0))
data = ingredients.aggregate(**aggregators)
return data
}}}
Expected behaviour: a dictionary is returned: {'cost_missing': <value>},
containing as a value the number of missing cost values.Actual behaviour:
AssertionError No exception message supplied is returned on the line
''data = ingredients.aggregate(**aggregators)'' (see abbreviated
stacktrace below).
When the aggregated field name is set to a field that exists on the model
of the queryset (i.e. is not annotated), the aggregation works. For
example: instead of using cost_value=None in the When() of the
aggregators, using quantity=None works.
Similarly, doing an aggregation over an annotated field without using
Case() and When() works. For example:
{{{
aggregators['cost'] = Coalesce(
Sum(
F('cost_value') * F('quantity') / F('cost_quantity')
output_field=DecimalField()
), Value(0)
}}}
----
**Stacktrace:**
{{{
Python version is 3.6.4. The problem also exists under Django version
2.2b1
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:6>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage:
case/when | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> Aggregating annotations works for simple Sum, Count, etc. operations, but
> fails when the Sum() contains a Case() When() operation.
>
> When(**{'cost_value': None}, then=Value(1)),
> default=Value(0),
> output_field=IntegerField()
> )
> ), Value(0))
>
> data = ingredients.aggregate(**aggregators)
> return data
> }}}
>
> Expected behaviour: a dictionary is returned: {'cost_missing': <value>},
> containing as a value the number of missing cost values.Actual behaviour:
> AssertionError No exception message supplied is returned on the line
> ''data = ingredients.aggregate(**aggregators)'' (see abbreviated
> stacktrace below).
>
> When the aggregated field name is set to a field that exists on the model
> of the queryset (i.e. is not annotated), the aggregation works. For
> example: instead of using cost_value=None in the When() of the
> aggregators, using quantity=None works.
> Similarly, doing an aggregation over an annotated field without using
> Case() and When() works. For example:
> {{{
> aggregators['cost'] = Coalesce(
> Sum(
> F('cost_value') * F('quantity') / F('cost_quantity')
> output_field=DecimalField()
> ), Value(0)
> }}}
>
> ----
>
> **Stacktrace:**
> {{{
> Python version is 3.6.4. The problem also exists under Django version
> 2.2b1
New description:
Aggregating annotations works for simple Sum, Count, etc. operations, but
fails when the Sum() contains a Case() When() operation.
To reproduce the issue, a simplified scenario:
**models.py**
{{{
class Ingredient(models.Model):
pass
When(**{'cost_value': None}, then=Value(1)),
default=Value(0),
output_field=IntegerField()
)
), Value(0))
data = ingredients.aggregate(**aggregators)
return data
}}}
Expected behaviour: a dictionary is returned: {'cost_missing': <value>},
containing as a value the number of missing cost values.Actual behaviour:
AssertionError No exception message supplied is returned on the line
''data = ingredients.aggregate(**aggregators)'' (see abbreviated
stacktrace below).
When the aggregated field name is set to a field that exists on the model
of the queryset (i.e. is not annotated), the aggregation works. For
example: instead of using cost_value=None in the When() of the
aggregators, using quantity=None works.
Similarly, doing an aggregation over an annotated field without using
Case() and When() works. For example:
{{{
aggregators['cost'] = Coalesce(
Sum(
F('cost_value') * F('quantity') / F('cost_quantity')
output_field=DecimalField()
), Value(0)
}}}
----
**Stacktrace:**
{{{
Python version is 3.6.4. The problem also exists under Django version
2.2b1
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:7>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage:
case/when | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Carlton Gibson):
Hi Lukas, Any chance you could put this into a sample project and attach
that (or a test case in a PR) so we can run this example and experiment?
(It would save a cycle and speed review if you could.)
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:8>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
case/when |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
Sample project attached. Even if the query isn't valid, a more helpful
error message would be useful.
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:9>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* Attachment "t30188.zip" added.
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/11050 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:10>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Lukas Klement):
Hi Tim, thank you for attaching an example project - why is the query not
valid? Why is it that annotated fields derived from annotated fields that
used Case() / When() cannot be aggregated?
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:11>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
I didn't look at this in any depth. I don't know whether or not the query
is valid. I only meant to say that if it's not valid, there should be a
better message. The proposed wording on the PR (if the query is indeed
invalid) ... 'Related Field got invalid lookup: salary' ... doesn't seem
to provide much explanation.
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:12>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Can Sarıgöl (added)
Comment:
Hi, actually I thought this sample is the same with below so the same
exception message might be ok. Besides, I will check the query.
{{{
ingredients = RecipeIngredient.objects.aggregate(
cost_missing = Coalesce(Sum(
Case(
When(ingredient__cost__isnull=False, then=Value(1)),
default=Value(0),
output_field=IntegerField())
), Value(0))
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:13>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
I agree with Lukas that we should investigate why the ORM crashes here as
it's a completely valid query from my perspective.
The fundamental issue here is that `Subquery` breaks the `Expression` API
by having a `get_source_expressions` method that returns `N > 0`
expressions while having a `set_source_expressions` that accepts none.
I happen to have a local branch I've been playing with locally to make
`Subquery` more a of dumb wrapper around `sql.Query` that happens to
restore this contract. I haven't tried it out against this exact set of
models but the fact the caller
[https://github.com/django/django/blob/b435f82939edf70674856e0e1cd63973c2e0a1d1/django/db/models/sql/query.py#L362-L389
hosts a FIXME] makes me feel like there might be something else going on.
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:14>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Can Sarıgöl):
Query like this:
{{{
SELECT COALESCE(
SUM(CASE WHEN (SELECT U0."cost"
FROM "t30188_ingredientdetail"
U0
WHERE U0."ingredient_id" =
(subquery."ingredient_id") LIMIT 1) IS NULL THEN 1 ELSE 1 END), 1)
FROM (SELECT "t30188_recipeingredient"."id" AS Col1,
(SELECT U0."cost"
FROM "t30188_ingredientdetail" U0
WHERE U0."ingredient_id" =
("t30188_recipeingredient"."ingredient_id") LIMIT 1) AS "cost_value",
"U0"."ingredient_id" AS "__col1"
FROM "t30188_recipeingredient"
GROUP BY "t30188_recipeingredient"."id",
(SELECT U0."cost"
FROM "t30188_ingredientdetail" U0
WHERE U0."ingredient_id" =
("t30188_recipeingredient"."ingredient_id") LIMIT 1),
"U0"."ingredient_id") subquery
}}}
the problem is {{{"U0"."ingredient_id" AS "__col1"}}}, It should have been
{{{"t30188_recipeingredient"."ingredient_id"}}}. to solve this, I will
also check your point @Simon Charette thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:15>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Simon Charette (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:16>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:17>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
The assertion error is addressed by
https://github.com/django/django/pull/11062 but it unveiled another issue
regarding aggregation over annotations.
I managed to get it to work by adjusting the ''hacky''
`Query.rewrite_cols` function but then I noticed the subquery was
annotated twice which might hurt performance. I guess the suggested
approach at least works and the double annotation issue seems to be
generalized anyway. I suggest we open a following ticket to address the
`rewrite_cols` extra annotation issue and proceed with the current
approach for now.
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:18>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Can Sarıgöl):
Can we fix twice annotated problem in here? I thought that we can remove
the expr if it is already in annotations ({{{expr == old_expr}}})
{{{
elif isinstance(expr, (Col, Subquery)) or (expr.contains_aggregate and not
expr.is_summary):
...
new_exprs.append(Ref(col_alias, expr))
if isinstance(expr, Subquery):
keys = {
k for k, old_expr in self.annotations.items()
if k.startswith('__col') is False and expr == old_expr
}
for _k in keys:
del self.annotations[_k]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:19>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Can, I guess we could somehow but I feel like we'd need a more elegant
solution than that.
Note that the double annotation issue doesn't only happen for subqueries,
you might have noticed that the `company_id` which is first annotated as
`Col0` gets annotated as `__col0` as well. This makes me believe we might
need to do it for `Col` instances as well?
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:20>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
I just pushed another commit that addresses the double aliasing issue. I
credited you in the commit message Can.
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:21>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Can Sarıgöl):
Thanks for that. This is a better solution.
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:22>
Django
supplied
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"35431298226165986ad07e91f9d3aca721ff38ec" 35431298]:
{{{
#!CommitTicketReference repository=""
revision="35431298226165986ad07e91f9d3aca721ff38ec"
Refs #27149 -- Moved subquery expression resolving to Query.
This makes Subquery a thin wrapper over Query and makes sure it respects
the Expression source expression API by accepting the same number of
expressions as it returns. Refs #30188.
It also makes OuterRef usable in Query without Subquery wrapping. This
should allow Query's internals to more easily perform subquery push downs
during split_exclude(). Refs #21703.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:23>
Django
supplied
-------------------------------------+-------------------------------------
Component: Database layer | Version: 2.1
(models, ORM) |
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"d1e9c25162eecb24dee50fcfe834a2735e53fb0e" d1e9c25]:
{{{
#!CommitTicketReference repository=""
revision="d1e9c25162eecb24dee50fcfe834a2735e53fb0e"
Refs #30188 -- Prevented double annotation of subquery when aggregated
over.
Thanks Can Sarıgöl for the suggested trimming approach.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:25>
Django
supplied
-------------------------------------+-------------------------------------
Reporter: Lukas Klement | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"bdc07f176eb7c099ac3fdc42ebaadd48b7f67409" bdc07f1]:
{{{
#!CommitTicketReference repository=""
revision="bdc07f176eb7c099ac3fdc42ebaadd48b7f67409"
Fixed #30188 -- Fixed a crash when aggregating over a subquery annotation.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:24>
Django
supplied
-------------------------------------+-------------------------------------
Reporter: Lukas Klement | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"3f32154f40a855afa063095e3d091ce6be21f2c5" 3f32154]:
{{{
#!CommitTicketReference repository=""
revision="3f32154f40a855afa063095e3d091ce6be21f2c5"
Refs #30188 -- Avoided GROUP BY when aggregating over non-aggregates.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:26>
Django
supplied
-------------------------------------+-------------------------------------
Reporter: Lukas Klement | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"adfbf653dc1c1d0e0dacc4ed46602d22ba28b004" adfbf653]:
{{{
#!CommitTicketReference repository=""
revision="adfbf653dc1c1d0e0dacc4ed46602d22ba28b004"
Fixed #31568 -- Fixed alias reference when aggregating over multiple
subqueries.
691def10a0197d83d2d108bd9043b0916d0f09b4 made all Subquery() instances
equal to each other which broke aggregation subquery pushdown which
relied on object equality to determine which alias it should select.
Subquery.__eq__() will be fixed in an another commit but
Query.rewrite_cols() should haved used object identity from the start.
Refs #30727, #30188.
Thanks Makina Corpus for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:27>
Django
supplied
-------------------------------------+-------------------------------------
Reporter: Lukas Klement | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"3913acdb29d09109ec82ce789b592e6281aa7be9" 3913acdb]:
{{{
#!CommitTicketReference repository=""
revision="3913acdb29d09109ec82ce789b592e6281aa7be9"
[3.1.x] Fixed #31568 -- Fixed alias reference when aggregating over
multiple subqueries.
691def10a0197d83d2d108bd9043b0916d0f09b4 made all Subquery() instances
equal to each other which broke aggregation subquery pushdown which
relied on object equality to determine which alias it should select.
Subquery.__eq__() will be fixed in an another commit but
Query.rewrite_cols() should haved used object identity from the start.
Refs #30727, #30188.
Thanks Makina Corpus for the report.
Backport of adfbf653dc1c1d0e0dacc4ed46602d22ba28b004 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:28>
Django
supplied
-------------------------------------+-------------------------------------
Reporter: Lukas Klement | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: query, aggregate, | Triage Stage: Accepted
case/when |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"49bbf6570d9f0880b836f741d79e4cdb6e061ea2" 49bbf657]:
{{{
#!CommitTicketReference repository=""
revision="49bbf6570d9f0880b836f741d79e4cdb6e061ea2"
[3.0.x] Fixed #31568 -- Fixed alias reference when aggregating over
multiple subqueries.
691def10a0197d83d2d108bd9043b0916d0f09b4 made all Subquery() instances
equal to each other which broke aggregation subquery pushdown which
relied on object equality to determine which alias it should select.
Subquery.__eq__() will be fixed in an another commit but
Query.rewrite_cols() should haved used object identity from the start.
Refs #30727, #30188.
Thanks Makina Corpus for the report.
Backport of adfbf653dc1c1d0e0dacc4ed46602d22ba28b004 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30188#comment:29>