Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Mariusz Felisiak ):

 In [changeset:"803caec60bed3b282b9f9961860a467160c0c8f1" 803caec]:
 {{{
 #!CommitTicketReference repository=""
 revision="803caec60bed3b282b9f9961860a467160c0c8f1"
 [4.2.x] Fixed #34798 -- Fixed QuerySet.aggregate() crash when referencing
 expressions containing subqueries.

 Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7,
 complements e5c844d6f2a4ac6ae674d741b5f1fa2a688cedf4.

 Refs #28477, #34551.

 Thanks Haldun Komsuoglu for the report.

 Backport of 3b4a571275d967512866012955eb0b3ae486d63c from main
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018b36b28d53-6f016cf3-4e3c-48f2-926c-270a889caf26-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Mariusz Felisiak ):

 In [changeset:"4ccca9eedc2f453602a20f562399a835a24817c1" 4ccca9ee]:
 {{{
 #!CommitTicketReference repository=""
 revision="4ccca9eedc2f453602a20f562399a835a24817c1"
 [5.0.x] Fixed #34798 -- Fixed QuerySet.aggregate() crash when referencing
 expressions containing subqueries.

 Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7,
 complements e5c844d6f2a4ac6ae674d741b5f1fa2a688cedf4.

 Refs #28477, #34551.

 Thanks Haldun Komsuoglu for the report.

 Backport of 3b4a571275d967512866012955eb0b3ae486d63c from main
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018b36b2313f-43a55868-e019-4bb0-92a9-2a5324294837-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Mariusz Felisiak ):

 In [changeset:"3b4a571275d967512866012955eb0b3ae486d63c" 3b4a5712]:
 {{{
 #!CommitTicketReference repository=""
 revision="3b4a571275d967512866012955eb0b3ae486d63c"
 Fixed #34798 -- Fixed QuerySet.aggregate() crash when referencing
 expressions containing subqueries.

 Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7,
 complements e5c844d6f2a4ac6ae674d741b5f1fa2a688cedf4.

 Refs #28477, #34551.

 Thanks Haldun Komsuoglu for the report.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018b36b1c090-61151cdb-7dff-4fe1-a64e-dda85b16bd19-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Mariusz Felisiak ):

 In [changeset:"7a67b065d7e5653f3af1cbd28882d33d2a088b02" 7a67b065]:
 {{{
 #!CommitTicketReference repository=""
 revision="7a67b065d7e5653f3af1cbd28882d33d2a088b02"
 [4.2.x] Fixed #34717 -- Fixed QuerySet.aggregate() crash when referencing
 window functions.

 Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7.

 Refs #28477.

 Thanks younes-chaoui for the report.

 Backport of 68912e4f6f84f21322f92a2c7b6c77f68f91b9c9 from main
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/010701896cf8e8ba-53beac9b-64be-4a34-a127-7f0b4314127b-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Mariusz Felisiak ):

 In [changeset:"68912e4f6f84f21322f92a2c7b6c77f68f91b9c9" 68912e4]:
 {{{
 #!CommitTicketReference repository=""
 revision="68912e4f6f84f21322f92a2c7b6c77f68f91b9c9"
 Fixed #34717 -- Fixed QuerySet.aggregate() crash when referencing window
 functions.

 Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7.

 Refs #28477.

 Thanks younes-chaoui for the report.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/010701896cf86d0f-91fd83e0-d68e-416e-8049-85f3f9dfeeb4-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette):

 Dustin, [https://forum.djangoproject.com/t/django-4-2-behavior-change-
 when-using-arrayagg-on-unnested-arrayfield-postgresql-specific/21547/2 I
 think this answer is your way out].

 Basically the optimization should be disabled for any set-returning
 function but since Django only has single native one,`Subquery`, so the
 optimization is only disabled when the `subquery = True` attribute is set.

 In order to truly solve this issue I think we should introduce a new
 documented `Expression.set_returning: bool` (better name welcome!) flag
 that defaults to `False` but is set to `True` for `Subquery`.

 The root of this problem is that the ORM simply doesn't support functions
 that return rows in a generic way. Instead it branches out using
 `getattr(expr, "subquery", False)` in all cases that it makes the most
 sense to support them (e.g. `__in` looups).

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/01070189294c5e6f-1f144466-4c38-48cd-8ca3-a9da2e693ca8-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Dustin Lorres):

 Is there a way to opt out of this behavior from application code? After
 upgrading from 3.2 to 4.2 some of our pagination stopped working correctly
 and it was traced back to `.count()` not providing the correct results,
 likely from the updates in this ticket.

 Here is an example of the failure (using PostgreSQL JSON query):

 {{{
 >>> qs = MyModel.objects.annotate(table_element=Func("data",
 
Value("$.MyArray[*]"),function="jsonb_path_query",output_field=JSONField())).filter(pk=1)
 >>> qs.count()
 1
 >>> len(qs)
 2
 }}}

 This assumes a very simple model `MyModel` with a JSON field `data` that
 is has an instance (pk of 1) that has something the data field set to:
 {{{
 {
   "MyArray": [{"id": 1, "name": "test"}, {"id": 2, "name": "test2"}]
 }
 }}}

 The issue is now the `count` is not factoring in the annotation which will
 actually increase the number of rows returned in the queryset (for this
 example due to the jsonb_path_query which returns a set). It is only
 counting the number of rows of `MyModel` which due to the `pk` filter will
 only have one row returned.

 Is there anyway to force the count operation to expand the query and
 include the annotation?

 I tried to filter on the count of the jsonb path query, but it failed:
 {{{
 >>> qs = MyModel.objects.annotate(my_count=Count(Func("data",
 
Value("$.MyArray[*]"),function="jsonb_path_query",output_field=JSONField(.filter(pk=1,
 my_count__gt=0)
 >>> qs.count()
 Exception:
 django.db.utils.NotSupportedError: set-returning functions are not allowed
 in HAVING
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/010701892897cac3-45b216c5-5644-4bf6-9611-48984d78e17e-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Mariusz Felisiak ):

 In [changeset:"c78a4421de0fc3240b91d59e8f9028331777c624" c78a442]:
 {{{
 #!CommitTicketReference repository=""
 revision="c78a4421de0fc3240b91d59e8f9028331777c624"
 [4.2.x] Fixed #34551 -- Fixed QuerySet.aggregate() crash when referencing
 subqueries.

 Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7.

 Refs #28477.

 Thanks Denis Roldán and Mariusz for the test.

 Backport of e5c844d6f2a4ac6ae674d741b5f1fa2a688cedf4 from main
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/01070188471fc0ec-a344647c-50d9-4ebd-90a3-44254226f6c6-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Mariusz Felisiak ):

 In [changeset:"e5c844d6f2a4ac6ae674d741b5f1fa2a688cedf4" e5c844d6]:
 {{{
 #!CommitTicketReference repository=""
 revision="e5c844d6f2a4ac6ae674d741b5f1fa2a688cedf4"
 Fixed #34551 -- Fixed QuerySet.aggregate() crash when referencing
 subqueries.

 Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7.

 Refs #28477.

 Thanks Denis Roldán and Mariusz for the test.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/01070188471ee34b-064c777e-99e1-4750-97a6-e8d26d3f159d-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Mariusz Felisiak ):

 In [changeset:"511dc3db539122577aaba71f5a24d65d5adab092" 511dc3db]:
 {{{
 #!CommitTicketReference repository=""
 revision="511dc3db539122577aaba71f5a24d65d5adab092"
 [4.2.x] Fixed #34464 -- Fixed queryset aggregation over group by
 reference.

 Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7.

 Refs #28477.

 Thanks Ian Cubitt for the report.

 Backport of 9daf8b4109c3e133eb57349bb44d73cc60c5773c from main
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/010701875a14d378-eed83881-1afd-4222-a96a-9e823e7698ac-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Mariusz Felisiak ):

 In [changeset:"9daf8b4109c3e133eb57349bb44d73cc60c5773c" 9daf8b41]:
 {{{
 #!CommitTicketReference repository=""
 revision="9daf8b4109c3e133eb57349bb44d73cc60c5773c"
 Fixed #34464 -- Fixed queryset aggregation over group by reference.

 Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7.

 Refs #28477.

 Thanks Ian Cubitt for the report.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/010701875a147b9b-bf79a6e0-05b1-4311-a95c-6e7800b9f5c9-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by GitHub ):

 In [changeset:"10037130c123cd747d32a14a9ba47e0c5c9a37d1" 1003713]:
 {{{
 #!CommitTicketReference repository=""
 revision="10037130c123cd747d32a14a9ba47e0c5c9a37d1"
 Refs #28477 -- Fixed handling aliased annotations on aggregation.

 Just like when using .annotate(), the .alias() method will generate the
 necessary JOINs to resolve the alias even if not selected.

 Since these JOINs could be multi-valued non-selected aggregates must be
 considered to require subquery wrapping as a GROUP BY is required to
 combine duplicated tuples from the base table.

 Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018474751dd6-8106243f-da5f-4ec8-a09d-f7db7902154a-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Mariusz Felisiak ):

 In [changeset:"a9d2d8d1c36a4338758a792c475965180629a59f" a9d2d8d]:
 {{{
 #!CommitTicketReference repository=""
 revision="a9d2d8d1c36a4338758a792c475965180629a59f"
 Refs #28477 -- Reduced complexity of aggregation over qualify queries.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018465c32838-6b022664-2462-4de2-9ae5-4b14447a0c5a-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak ):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"59bea9efd2768102fc9d3aedda469502c218e9b7" 59bea9ef]:
 {{{
 #!CommitTicketReference repository=""
 revision="59bea9efd2768102fc9d3aedda469502c218e9b7"
 Fixed #28477 -- Stripped unused annotations on aggregation.

 Also avoid an unnecessary pushdown when aggregating over a query that
 doesn't
 have aggregate annotations.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/010701845c739abf-bfef8dc9-d1f0-493d-8eff-ab58036af005-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  assigned
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak):

 * stage:  Accepted => Ready for checkin


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/010701845c582174-6b3a0af8-3268-4ab1-8778-dbb5a2893944-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  assigned
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette):

 * needs_better_patch:  1 => 0


Comment:

 I submitted [https://github.com/django/django/pull/16263 a revised PR]
 that strip unused annotations not only for `.count()` for but any usage of
 `.aggregate`.

 To take example from comment:7 and comment:9 the resulting query is now

 {{{#!python
 Person.objects.annotate(full_name=Concat('first_name', Value(' '),
 'last_name')).count()
 }}}
 {{{#!sql
 SELECT COUNT(*) FROM support_person
 }}}

 The adjusted logic goes along these lines.

 Instead of systematically performing a subquery wrapping when there are
 existing annotations only do so when the pre-existing annotation are
 aggregate or window functions. In both cases, pre-existing
 aggregation/window or not, strip annotations that are not referenced by
 the aggregates.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/010701844c272df4-bc7d30b1-2851-420f-ae77-8dfa6d8b99dc-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  assigned
Component:  Database layer   |  Version:  dev
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette):

 * owner:  Tom Forbes => Simon Charette


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/010701844bc41303-24c55d57-31d8-4906-bc6c-4e4dbc6f8221-00%40eu-central-1.amazonses.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Tom
 Type:   |  Forbes
  Cleanup/optimization   |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by shazad sarwar):

 * Attachment "ProductAndServiceUsage.json.zip" added.


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/061.7b3edce7cecfd5fd44c7006d45a8fb9e%40djangoproject.com.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Tom
 Type:   |  Forbes
  Cleanup/optimization   |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Reupen Shah):

 Just a note to say that the behaviour I was describing was rectified in
 https://github.com/django/django/pull/11062.

 The third case from above now executes the following SQL query:

 {{{
 SELECT COUNT(*) FROM (
 SELECT CONCAT("people_person"."first_name", CONCAT(' ',
 "people_person"."last_name")) AS "full_name" FROM "people_person"
 ) subquery
 }}}

 Although the original ticket was about eliminating unneeded annotations
 completely, the result above is good enough for me, as the group by has
 been eliminated which was the real performance killer.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/061.b2341b4e81a7e803838c69b39ed81c54%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Tom
 Type:   |  Forbes
  Cleanup/optimization   |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Reupen Shah):

 * cc: Reupen Shah (added)


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/061.ad74a45676febf99879681d5b5b8de63%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Tom
 Type:   |  Forbes
  Cleanup/optimization   |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Reupen Shah):

 The behaviour is actually a bit more bizarre than I thought.

 With the same person model, here are four count variations and the
 generated queries:

 1.

 {{{
 Person.objects.count()
 }}}

 {{{
 SELECT COUNT(*) AS "__count" FROM "people_person"
 }}}

 2.

 {{{
 Person.objects.values('pk').count()
 }}}

 {{{
 SELECT COUNT(*) AS "__count" FROM "people_person"
 }}}

 3.

 {{{
 Person.objects.annotate(full_name=Concat('first_name', Value(' '),
 'last_name')).count()
 }}}

 {{{
 SELECT COUNT(*) FROM (SELECT "people_person"."id" AS Col1,
 CONCAT("people_person"."first_name", CONCAT(' ',
 "people_person"."last_name")) AS "full_name" FROM "people_person" GROUP BY
 "people_person"."id", CONCAT("people_person"."first_name", CONCAT(' ',
 "people_person"."last_name"))) subquery
 }}}

 4.

 {{{
 Person.objects.annotate(full_name=Concat('first_name', Value(' '),
 'last_name')).values('pk').count()
 }}}

 {{{
 SELECT COUNT(*) FROM (SELECT "people_person"."id" AS Col1 FROM
 "people_person") subquery
 }}}


 So there's a simple workaround for the case I gave in my last comment in
 calling `.values('pk')` before `.count()`. However, that's not much help
 if Django or other libraries are the ones calling `.count()`.

 On the plus side, I have a better understanding of what the problem is
 from looking at the Django source code.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/061.3a64851e336b74aa6e2351e7d22137f9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Tom
 Type:   |  Forbes
  Cleanup/optimization   |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette):

 This is somewhat related to #30158 where the compiler is not smart enough
 to determine if it can exclude subquery annotations from `GROUP BY` on
 aggregation.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/061.53a6a2b0db6e0d462ad22bfaaabc0126%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Tom
 Type:   |  Forbes
  Cleanup/optimization   |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Reupen Shah):

 I have also run into problems with `QuerySet.count()` being very slow on
 annotated query sets. Django uses a subquery for the count but injects a
 group by into the subquery. This typically causes the database server to
 deduplicate all matched rows using the group by columns which is, in
 general, extremely slow when there are a large number of matched rows.

 For example, consider the following model:

 {{{
 class Person(models.Model):
 """Person model."""

 first_name = models.TextField()
 last_name = models.TextField()
 country = models.TextField(null=True, blank=True)
 }}}

 and query set:

 {{{
 from django.db.models.functions import Concat
 from django.db.models import Value

 queryset = Person.objects.annotate(full_name=Concat('first_name', Value('
 '), 'last_name'))
 }}}

 `queryset.count()` generates the following query under PostgreSQL:

 {{{
 SELECT COUNT(*)
 FROM
   (SELECT "support_person"."id" AS Col1,
   CONCAT("support_person"."first_name", CONCAT(' ',
 "support_person"."last_name")) AS "full_name"
FROM "support_person"
GROUP BY "support_person"."id",
 CONCAT("support_person"."first_name", CONCAT(' ',
 "support_person"."last_name"))) subquery
 }}}

 `list(queryset)` generates:

 {{{
 SELECT "support_person"."id",
"support_person"."first_name",
"support_person"."last_name",
"support_person"."country",
CONCAT("support_person"."first_name", CONCAT(' ',
 "support_person"."last_name")) AS "full_name"
 FROM "support_person"
 }}}


 I am not entirely sure why the subquery for the count needs to be any
 different from the query used when the query set itself is evaluated.
 There are some relevant comments in the source code here:
 
https://github.com/django/django/blob/5deb7a86e8b54d052a3b1dbed1ae7142d362b1c5/django/db/models/sql/query.py#L404-L414

 This has all been tested under Django 2.1.7.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/061.ac61c734d7172f23b1353d1018acb01a%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Tom
 Type:   |  Forbes
  Cleanup/optimization   |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Tim Graham):

 * needs_better_patch:  0 => 1


Comment:

 The PR is still marked [WIP] and there are test failures.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/061.b96311b1c6f07ebd03058a233d53bdec%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Tom
 Type:   |  Forbes
  Cleanup/optimization   |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette):

 * has_patch:  0 => 1


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/061.422351ccdc97b254d2f4ab7a57d727f5%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom Forbes   |Owner:  Tom
 Type:   |  Forbes
  Cleanup/optimization   |   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
-+-

Comment (by Tom Forbes):

 WIP PR: https://github.com/django/django/pull/8928/files

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/061.15e2ce90115bc6a192ff913f0fe3df97%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom  |Owner:  Tom
 Type:   |   Status:  assigned
  Cleanup/optimization   |
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 Tom):

 Same can be done for `exists()`

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/061.0d2049db16521dc628f5265e5f0660fc%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom  |Owner:  Tom
 Type:   |   Status:  assigned
  Cleanup/optimization   |
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 Tom):

 * owner:  nobody => Tom
 * status:  new => assigned


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/061.86bf34199da230627c994d756954efc9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28477: Strip unused annotations from count queries

[Django]

#28477: Strip unused annotations from count queries
-+-
 Reporter:  Tom  |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
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 Tim Graham):

 * stage:  Unreviewed => Accepted


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/061.dd4fe0df603029e694cfe4bf0c916071%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.