Re: [Django] #15049: Using annotation before and after filter gives wrong results
Django
-------------------------------------+-------------------------------------
Reporter: Alex | Owner: anonymous
Type: Bug | Status: assigned
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 |
-------------------------------------+-------------------------------------
Changes (by mbmohitbagga88@…):
* owner: => anonymous
* cc: mbmohitbagga88@… (added)
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/15049#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Alex | Owner: anonymous
Type: Bug | Status: assigned
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 akaariai):
Just a warning - this isn't at all easy to tackle. Basically if the query
has more than one "multijoin" then aggregates must be done either as
subselects or as subqueries.
An alternate is to just throw an error in such cases. Even this would be
better than producing wrong results silently.
--
Ticket URL: <https://code.djangoproject.com/ticket/15049#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Alex | Owner: anonymous
Type: Bug | Status: assigned
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 terpsquared):
Just ran into this. At the very least, the documentation should be updated
to reflect this issue.
https://docs.djangoproject.com/en/dev/topics/db/aggregation/#order-of-
annotate-and-filter-clauses
--
Ticket URL: <https://code.djangoproject.com/ticket/15049#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Alex | Owner: anonymous
Type: Bug | Status: assigned
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 timgraham):
Confirmed it's still an issue as of
dad8434d6ff5da10959672726dc9b397296d380b. Attaching a rebased test.
In the interim, documentation patches would of course be welcomed.
--
Ticket URL: <https://code.djangoproject.com/ticket/15049#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Alex | Owner: anonymous
Type: Bug | Status: assigned
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
-------------------------------------+-------------------------------------
* Attachment "15049-test.diff" added.
--
Ticket URL: <https://code.djangoproject.com/ticket/15049>
Django
-------------------------------------+-------------------------------------
Reporter: Alex | Owner:
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
-------------------------------------+-------------------------------------
Changes (by timgraham):
* owner: anonymous =>
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/15049#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Alex | 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 akaariai):
What is happening here is that:
- first annotate uses the first join it finds for authors, or generates
a new one
- the second filter call introduces another join on authors
- the second annotate uses again the first join it finds for authors
It would likely be somewhat easy to make the second annotate use the last
introduced join instead of the first join for authors. But, this has a
couple of problems. It could break existing code, and it is likely that
the results would be incorrect in any case (due to Django not handling
multiple multivalued joins + annotate correctly).
I say we don't fix this issue. Instead we could add an explicit way to
introduce joins. Something like:
{{{
qs = Book.objects.values("name").annotate(
n_authors=Count("authors")
).add_relation(
authors2='authors'
).filter(
authors2__name__startswith="Adrian"
).annotate(
n_authors2=Count("authors2")
)
}}}
Now it is explicit what happens. It is notable that the results are still
incorrect. To get correct results something like this might work:
{{{
qs = Book.objects.values("name").annotate(
n_authors=Count("authors")
).annotate(
n_authors2=Count("authors", subquery=True,
filter=Q(authors__name__startswith="Adrian"))
)
}}}
Of course, we have a bit more work to do before this is possible. The
point is I don't think we can be smart enough to automatically generate
the correct query for the report's case.
--
Ticket URL: <https://code.djangoproject.com/ticket/15049#comment:11>
Django
-------------------------------------+-------------------------------------
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 Simon Charette):
> I say we don't fix this issue. Instead we could add an explicit way to
introduce joins.
fwiw this `add_relation` somewhat exists today though
`QuerySet.alias(alias=FilteredRelation("relation"))`
--
Ticket URL: <https://code.djangoproject.com/ticket/15049#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: Alex Gaynor | 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 Abhinav Yadav):
[https://github.com/django/django/pull/16273 PR]
PR documenting this irregular behavior from the duplicate ticket #33403
--
Ticket URL: <https://code.djangoproject.com/ticket/15049#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: Alex Gaynor | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/15049#comment:14>
Django
-------------------------------------+-------------------------------------
| Yadav
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: (none) => Abhinav Yadav
* needs_better_patch: 0 => 1
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/15049#comment:15>