Re: [Django] #10060: Multiple table annotation failure
Django
---------------------------------+------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: ORM aggregation | Version: SVN
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Changes (by anonymous):
* ui_ux: => 0
* easy: => 0
Comment:
Another victim. I have a model {{{Foo}}}. I also have models {{{Bar}}} and
{{{Baz}}}, which each have a foreign key pointing to {{{Foo}}} with
related_names {{{'bars'}}}, and {{{'bazs'}}}, respectively:
{{{
for foo in Foo.objects.all().annotate(bar_count=Count('bars'),
baz_count=Count('bazs')):
print foo.bar_count # Was correct
print foo.baz_count # Was completely wrong, but not by any apparent
pattern (ie, the numbers seemed to be almost random)
}}}
I'm unable to display the {{{baz_count}}} in my template now, unless I
want to introduce an N+1 problem (ie, {{{foo.bazs.count()}}} during each
iteration)
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:29>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
---------------------------------+------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: ORM aggregation | Version: SVN
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Comment (by anonymous):
Whoa, thought I was doing something wrong in my code until I searched and
found this. Isn't this kind of a big deal? As in, with this bug, isn't
Django's aggregation basically worthless for anyone who's not doing it on
a single table, or did I miss something?
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:30>
Django
---------------------------------+------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: ORM aggregation | Version: SVN
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Comment (by anonymous):
Is this something that will ever be fixed?
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:31>
Django
---------------------------------+------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: ORM aggregation | Version: SVN
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Comment (by lukeplant):
Replying to [comment:31 anonymous]:
> Is this something that will ever be fixed?
It will only ever get fixed if someone cares enough to write a patch
(complete with tests). I think at this point a patch which threw an
exception for the failure case would be accepted. Obviously ideally this
would be replaced with a proper fix at some point.
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:32>
Django
---------------------------------+------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: ORM aggregation | Version: SVN
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Comment (by anonymous):
Really, it seems there are plenty of ticket with patches (complete with
tests), and with offers to do the work, but the core team doesn't care.
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:33>
Django
---------------------------------+------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: ORM aggregation | Version: SVN
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Comment (by carljm):
Replying to [comment:33 anonymous]:
> Really, it seems there are plenty of ticket with patches (complete with
tests), and with offers to do the work, but the core team doesn't care.
Sorry you're feeling frustrated.
There are currently 15 tickets in Trac marked "Ready for checkin." The
_only_ task in Django's development process that's limited to core
developers is committing those 15 tickets. Fifteen tickets doesn't seem
like such a terrible backlog for an entirely-volunteer crew, frankly.
Any other step in the process (writing a patch, adding tests to an
existing patch, reviewing a patch and marking it "Ready for checkin" if
there are tests, they pass, and it fixes the problem for you) can be done
by anyone in the community. See
https://docs.djangoproject.com/en/dev/internals/contributing/triaging-
tickets/
There is a larger backlog of "design decision needed" tickets (298), but
anyone can contribute usefully to those as well by researching pros and
cons and presenting a balanced summary to the developers mailing list to
start a discussion.
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:34>
Django
---------------------------------+------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: ORM aggregation | Version: SVN
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Comment (by anonymous coward):
It would seem that argumentum ad infinitum is a common theme on here, no?
"A is B", "No, A is A and B is B", "Fine, but C is D..." and so on.
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:35>
Django
---------------------------------+------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: ORM aggregation | Version: SVN
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
* cc: Fak3 (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:36>
Django
---------------------------------+------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Comment (by aaugustin):
#19290 is a duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:37>
Django
---------------------------------+------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: ORM aggregation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Comment (by michael@…):
Hmm, another victim here. Spent 2 hours trying to understand why my
numbers were so odd to finally find this bug. This is the first time in
several django projects I am really feeling left alone :-(
I am no database expert (which is one reason i LOVE django and it's
awesome Queryset) so I don't see any way that I could provide
implementation help or anything, but seeing that this bug is now open for
several years really makes me shake my head. Even if there is no obvious
fix available, PLEASE at least update the documentatin to point out this
issue!
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:38>
Django
---------------------------------+------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: ORM aggregation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
* cc: michaelB (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:39>
Django
---------------------------------+------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: ORM aggregation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Comment (by akaariai):
This is somewhat hard problem. We could do automatic subquery pushdown for
aggregates - it isn't exactly easy but most of the pieces are already
there. I am not sure how well such queries will perform. Although that
isn't perhaps too important. If the results aren't correct speed doesn't
matter...
I think a good start to this would be documentation + detecting the
problematic situations and throwing an error in those cases. Basically if
we have two multijoins in the query (from filtering, aggregation or
something else) and aggregation is used then the query is likely wrong.
However this condition is a little too restrictive - something like:
{{{
qs.filter(translations__lang='fi', translations__name='foo')
}}}
introduces a multijoin to translations, yet the query is safe as
translations.lang is unique. Detecting if we have a unique multijoin is
going to be nasty. So, we need a way to bypass this error checking.
Proposal: aggregates get a new kwarg "inline" - default False. If inline
is False, then multiple multijoins in single query will raise an error. If
it is True, the aggregate is forced to be joined to the query (that is, to
do what aggregates do today). Later on we will likely add another kwarg
"subquery" - defaults again to False. If set to True the aggregate is
forced to subquery (something like in comment:9). We could also add kwarg
"subselect". Later on maybe also "lateral_subquery" and so on.
The proposal allows us to throw an error, yet give users the ability to
force the ORM to generate the queries they are using now. It also allows
for extending the same API if we get subquery aggregate support some day.
I think we need to add the information about multijoins to JoinInfo, and
add the info about used joins to aggregates when they are added to the
query. Then at compiler stage, check for all inline=False aggregates if
there are multijoins in the query not used by the aggregate itself. If so,
throw an error. Changes should be needed at query.setup_joins(),
compiler.get_from_clause() and then a way to annotate the used joins into
the aggregate (this should not be too hard).
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:40>
Django
---------------------------------+------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: ORM aggregation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Comment (by anonymous):
At this point the documentation on annotations can be considered to be
intentionally misleading.
It is hard not to think this given the severity of the issue and the
comments above.
Yes a code fix is hard but at least an obviously tentative documentation
"fix" can be pushed fairly easily.
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:41>
Django
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by anonymous):
Bump
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:43>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by anonymous):
bump!!!!
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:44>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by timo):
Bumping this ticket accomplishes nothing. If you want to see it addressed,
please write a patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:45>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by benjaoming):
Agreed with anonymous 01/28/2013 that there needs to be at least some
documentation on this: https://github.com/django/django/pull/3654
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:46>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"4b23a6c7a9232cc07ec95fe98be17efbd4449822"]:
{{{
#!CommitTicketReference repository=""
revision="4b23a6c7a9232cc07ec95fe98be17efbd4449822"
Documented a current limitation of multiple table annotation; refs #10060.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:47>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"48bd44670b4c58735d5cdf6762d6b61219ecbb12"]:
{{{
#!CommitTicketReference repository=""
revision="48bd44670b4c58735d5cdf6762d6b61219ecbb12"
[1.7.x] Documented a current limitation of multiple table annotation; refs
#10060.
Backport of 4b23a6c7a9232cc07ec95fe98be17efbd4449822 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:48>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"a87ade7e30c80de93ec7ad8335d8714b494a58fb"]:
{{{
#!CommitTicketReference repository=""
revision="a87ade7e30c80de93ec7ad8335d8714b494a58fb"
[1.6.x] Documented a current limitation of multiple table annotation; refs
#10060.
Backport of 4b23a6c7a9232cc07ec95fe98be17efbd4449822 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:49>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by camillobruni):
I recently ran into a similar problem. The deep joining causes an
explosion of values possibly resulting in many duplicate values. I fixed
this in https://github.com/django/django/pull/4388 by implementing the
DISTINCT parameter on the Aggregate functions. Hence you can do
{{{Sum('center__client__loan__payment_schedule__payments__principal',
distinct=True)}}}
which ignores the duplicate entries.
It might be that the issue at hand is slightly different, but this solves
at least one particular case.
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:50>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by camillobruni):
Replying to [comment:50 camillobruni]:
> I recently ran into a similar problem. The deep joining causes an
explosion of values possibly resulting in many duplicate values. I fixed
this in https://github.com/django/django/pull/4388 by implementing the
DISTINCT parameter on the Aggregate functions. Hence you can do
> {{{Sum('center__client__loan__payment_schedule__payments__principal',
distinct=True)}}}
> which ignores the duplicate entries.
>
> It might be that the issue at hand is slightly different, but this
solves at least one particular case.
Sorry for the noise, I was wrong (in hindsight: "obviously")
http://www.sqlteam.com/article/how-to-use-group-by-with-distinct-
aggregates-and-derived-tables describes the problem I thought of dealing
with in more detail.
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:51>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: glic3rinu (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:52>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timgraham):
If I understood Anssi correctly, the existing documentation warning
[https://groups.google.com/d/msg/django-
developers/qiYKd1YZY_E/Tgn0p1wHCQAJ isn't correct], so I submitted
[https://github.com/django/django/pull/5795 a revision].
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:53>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"3d2236773ba88e330841b8c72183b0e978e10909" 3d22367]:
{{{
#!CommitTicketReference repository=""
revision="3d2236773ba88e330841b8c72183b0e978e10909"
Refs #10060 -- Corrected description of multiple annotations bug.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:54>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"ea63cf8d424780aed35e9973bf5e4dbb652c7d8a" ea63cf8d]:
{{{
#!CommitTicketReference repository=""
revision="ea63cf8d424780aed35e9973bf5e4dbb652c7d8a"
[1.9.x] Refs #10060 -- Corrected description of multiple annotations bug.
Backport of 3d2236773ba88e330841b8c72183b0e978e10909 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:55>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: nowak2000@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:56>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by powderflask):
Replying to [comment:9 bendavis78]:
In [comment:16 comment16] bendavis78 recommends this query to solve the
issue:
> {{{
> #!sql
> SELECT
> u.email,
> (SELECT SUM(points) FROM point_earning WHERE user_id=u.id) AS
points_earned,
> (SELECT SUM(points) FROM point_expense WHERE user_id=u.id) AS
points_spent
> FROM
> "user" u
> }}}
For others who might arrive here looking for a reasonable way to work
around this issue, the following worked well for me. I added a custom
Queryset method to the Manager class, and used a
[https://docs.djangoproject.com/en/1.9/ref/models/expressions/#raw-sql-
expressions RawSQL()] expression to create the annotations. This at
least encapsulates the SQL code and allows the annotation to be integrated
with a normal django queryset. Here's a sample for the example given
above:
{{{
#!python
def annotate_sum_for_user(user_related_modelClass, field_name,
annotation_name):
raw_query = """
SELECT SUM({field}) FROM {model} AS model
WHERE model.user_id = user.id
""".format(
field = field_name,
model = user_related_modelClass._meta.db_table,
)
annotation = {annotation_name: RawSQL(raw_query, [])}
return self.annotate(**annotation)
}}}
Usage for above query on presumed User model:
{{{
#!python
users = models.User.objects\
.annotate_sum_for_user(PointEarning, 'points', 'points_earned')\
.annotate_sum_for_user(PointExpense, 'points', 'points_spent')
}}}
Hope someone finds this useful, and a word of HUGE thanks to those who
worked to get a statement of this issue and a link to this thread into the
django documentation -- probably saved me hours.
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:57>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by SonOfLilit):
I just ran into this and submitted a duplicate bug (#26898).
I see a lot of sadness in this thread, joining my own.
My offer to fix if someone knowledgeable about the code can mentor me
still stands. I assume at least detecting it and generating a warning
shouldn't be too hard?
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:58>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by daniloakamine):
The solution proposed by '''powderflask''' is very interesting and worked
well.
This ticket helped me a lot to understand the big problem.
Thank you all, guys.
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:59>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by akermen):
No sure this will work for all different cases, at least it worked well
for me and the below simplified test case:
Model definitions of test case,
{{{
class ModelA(models.Model):
title = models.CharField(max_length=255, blank=False, null=False,
unique=True)
class ModelB(models.Model):
parent = models.ForeignKey(ModelA, on_delete=models.CASCADE)
value = models.IntegerField()
class ModelC(models.Model):
parent = models.ForeignKey(ModelA, on_delete=models.CASCADE)
}}}
and sample data:
{{{
instance_a_1 = ModelA(title="instance1")
instance_a_1.save()
instance_b = ModelB(parent=instance_a_1, value=1)
instance_b.save()
instance_b = ModelB(parent=instance_a_1, value=3)
instance_b.save()
instance_b = ModelB(parent=instance_a_1, value=5)
instance_b.save()
instance_c = ModelC(parent=instance_a_1)
instance_c.save()
instance_c = ModelC(parent=instance_a_1)
instance_c.save()
instance_c = ModelC(parent=instance_a_1)
instance_c.save()
instance_c = ModelC(parent=instance_a_1)
instance_c.save()
instance_c = ModelC(parent=instance_a_1)
instance_c.save()
instance_a_2 = ModelA(title="instance2")
instance_a_2.save()
instance_b = ModelB(parent=instance_a_2, value=7)
instance_b.save()
instance_b = ModelB(parent=instance_a_2, value=11)
instance_b.save()
instance_c = ModelC(parent=instance_a_2)
instance_c.save()
instance_c = ModelC(parent=instance_a_2)
instance_c.save()
instance_c = ModelC(parent=instance_a_2)
instance_c.save()
}}}
Trying to get two independent annotations from two different tables (sum
of values from ModelB and number of ModelC instances per ModelA instance):
{{{
for a in ModelA.objects.all() \
.annotate(sumB=Sum('modelb__value')) \
.annotate(countC=Count('modelc', distinct=True)):
print "%s: sumB: %s countC: %s" % (a.title, a.sumB, a.countC)
}}}
we get these results
{{{
instance1: sumB: 45 countC: 5
instance2: sumB: 54 countC: 3
}}}
instead of these:
{{{
instance1 sumB: 9 countC: 5
instance2 sumB: 18 countC: 3
}}}
no surprise until here.
As you can see the 'sumB' values are repeated (multiplied) by a factor,
lets account that factor inside single query:
{{{
for a in ModelA.objects.all() \
.annotate(countC=Count('modelc', distinct=True)) \
.annotate(countB=Count('modelb')) \
.annotate(countB_distinct=Count('modelb', distinct=True)) \
.annotate(sumB_multiplied=Sum('modelb__value')) \
.annotate(sumB=(F('sumB_multiplied') * F('countB_distinct')) /
F('countB')):
print "%s sumB: %s countC: %s" % (a.title, a.sumB, a.countC)
}}}
and get correct values:
{{{
instance1 sumB: 9 countC: 5
instance2 sumB: 18 countC: 3
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:60>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Artem.Bernatskyy):
Just have spent 5 hours debugging this bug.
+1
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:61>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Maxim (added)
Comment:
I think a note about this "feature" in the docs would be helpful.
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:62>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: th0th (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:63>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Israel Saeta Pérez):
FTR: See a workaround using `SubQuery` for the issue with annotate
described in this ticket, in this SO post:
https://stackoverflow.com/questions/48598245/multiple-annotate-with-sum-
and-display-data-in-admin-django
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:64>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Adam M. Costello (added)
Comment:
Does anyone here understand roughly what is safe to do with annotate()?
If I call annotate() only once on a queryset, with only one argument, is
that always safe, no matter how many relationships I follow in annotate()
and filter()?
Is it safe to pass multiple arguments to annotate() (or call annotate()
multiple times) if none of the annotations follow any relationships? Even
if there are relationships followed inside filter()?
Do the problems with annotate() also apply to aggregate(), or is it safe
to pass multiple arguments to aggregate() ? Is it safe to call annotate()
with one argument and then call aggregate()?
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:65>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Since this ticket has more exposure I wanted to mention that #28296 ought
to fix this by allowing a `subquery` argument to be passed to aggregate
functions.
If it was to be implemented the original report queryset could be made to
return the correct results easily
{{{#!python
Branch.objects.annotate(
total=Sum('center__client__loan__amount'),
repaid=Sum('center__client__loan__payment_schedule__payments__principal',
subquery=True),
)
}}}
Once this lands the ORM should be in a position to detect and warn about
or even deprecate multiple cross joined annotations.
Given the current existing logic with the `Subquery` expression it
shouldn't be too hard for someone with basic expression knowledge about
the ORM to implement a patch if anyone is interested in contributing.
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:66>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by wilhelmhb):
Replying to [comment:57 powderflask]:
A huge thanks to powderflask: for the ones knowledgeable enough with SQL,
this is the way to go =)
I also tried the solution described here:
[https://stackoverflow.com/questions/48598245/multiple-annotate-with-sum-
and-display-data-in-admin-django/48607830#48607830], alas without
success...
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:67>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Antoine (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:68>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Antoine):
People affected by this issue may find this useful:
https://stackoverflow.com/questions/56567841/django-count-and-sum-
annotations-interfere-with-each-other
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:69>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner: (none)
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Adam Sołtysik):
It's very surprising that aggregations aren't done in subqueries. The
problem arises even with basic, one-field aggregations, when there are
duplicates in the original query. So something like this, with a single
reverse foreign key relation:
{{{
User.objects.filter(orders__date=today).distinct().values('first_name').annotate(Count('pk'))
}}}
may produce wrong results (will count orders instead of users), even
though it looks safe with `.distinct()`. From what I've found, the
documentation doesn't warn about such cases.
Funnily enough, when I have rewritten one of my not working queries with
sum aggregation to use a subquery, it actually became faster (on a table
with ~100k rows), but of course YMMV.
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:70>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Matthijs Kooijman):
For anyone else running into this, a reasonable workaround seems to be to
use subqueries for aggregating annotations. This is a bit verbose/hacky in
Django currently, as shown by the multitude of approaches in the
stackoverflow link from [[comment:69|Antoine's comment]]. However, I've
successfully used the [[https://github.com/martsberger/django-sql-utils
|django-sql-utils]] package for this just now. That sounds a bit bulky,
but it just has two utilities, one of which is a `SubqueryAggregate` class
(with derived `SubqueryCount`, `SubquerySum`, etc.) that make converting a
regular joining aggregate into a subquery aggregate easy and concise,
without changing the structure much.
For example taking the example from [[comment:66|comment 66]] and
converting the second annotation to a subquery with django-sql-utils,
you'd get:
{{{
Branch.objects.annotate(
total=Sum('center__client__loan__amount'),
repaid=SubquerySum('center__client__loan__payment_schedule__payments__principal'),
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:71>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: David Kwong (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:72>
Django
-------------------------------------+-------------------------------------
Reporter: svsharma@… | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Natalia Bidart):
#34728 is a (fairly) duplicate of this one.
--
Ticket URL: <https://code.djangoproject.com/ticket/10060#comment:73>