[Django] #28658: Move DISTINCT handling to the Aggregate base class.

Django

unread,

Sep 29, 2017, 9:37:35 PM9/29/17

to django-...@googlegroups.com

#28658: Move DISTINCT handling to the Aggregate base class.
-------------------------------------+-------------------------------------
Reporter: Simon | Owner: Simon Charette
Charette |
Type: | Status: assigned
Cleanup/optimization |
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Django ships with a few `Aggregate` subclasses that expose a `distinct`
kwarg in order to inject a `DISTINCT` clause on compilation.

Since `DISTINCT` is repeatedly used by subclasses and likely to be useful
for custom `Aggregate` subclasses (PostgreSQL supports `DISTINCT` on all
aggregate functions, SQLite on all aggregates with a single argument, ...)
I suggest we handle it in the `Aggregate` base class.

We should be able to rely on `DatebaseOperations.check_expression_support`
to prevent `distinct=True` from being passed to aggregates not supporting
it on a backend basis.

--
Ticket URL: <https://code.djangoproject.com/ticket/28658>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Sep 29, 2017, 9:43:00 PM9/29/17

to django-...@googlegroups.com

#28658: Move DISTINCT handling to the Aggregate base class.
-------------------------------------+-------------------------------------

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* needs_better_patch: 0 => 1
* has_patch: 0 => 1

Comment:

[https://github.com/django/django/pull/9174 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/28658#comment:1>

Django

unread,

Oct 6, 2017, 9:28:31 AM10/6/17

to django-...@googlegroups.com

#28658: Move DISTINCT handling to the Aggregate base class.
-------------------------------------+-------------------------------------

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):

* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/28658#comment:2>

Django

unread,

Nov 24, 2018, 2:14:56 AM11/24/18

to django-...@googlegroups.com

#28658: Move DISTINCT handling to the Aggregate base class.
-------------------------------------+-------------------------------------

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

--
Ticket URL: <https://code.djangoproject.com/ticket/28658#comment:3>

Django

unread,

Nov 27, 2018, 5:31:51 PM11/27/18

to django-...@googlegroups.com

#28658: Move DISTINCT handling to the Aggregate base class.
-------------------------------------+-------------------------------------

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

--
Ticket URL: <https://code.djangoproject.com/ticket/28658#comment:4>

Django

unread,

Dec 14, 2018, 9:00:02 PM12/14/18

to django-...@googlegroups.com

#28658: Move DISTINCT handling to the Aggregate base class.
-------------------------------------+-------------------------------------

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

--
Ticket URL: <https://code.djangoproject.com/ticket/28658#comment:5>

Django

unread,

Jan 9, 2019, 5:53:19 PM1/9/19

to django-...@googlegroups.com

#28658: Move DISTINCT handling to the Aggregate base class.
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: Simon
Type: | Charette

Cleanup/optimization | Status: closed

Component: Database layer | Version: master
(models, ORM) |

Severity: Normal | Resolution: fixed
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 Tim Graham <timograham@…>):

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

Comment:

In [changeset:"bc05547cd8c1dd511c6b6a6c873a1bc63417b111" bc05547]:
{{{
#!CommitTicketReference repository=""
revision="bc05547cd8c1dd511c6b6a6c873a1bc63417b111"
Fixed #28658 -- Added DISTINCT handling to the Aggregate class.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/28658#comment:6>

Django

unread,

Jan 20, 2019, 2:46:39 PM1/20/19

to django-...@googlegroups.com

#28658: Move DISTINCT handling to the Aggregate base class.
-------------------------------------+-------------------------------------

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Claude Paroz):

Regression reported in #30120

--
Ticket URL: <https://code.djangoproject.com/ticket/28658#comment:7>

Django

unread,

Jan 21, 2019, 9:25:29 AM1/21/19

to django-...@googlegroups.com

#28658: Move DISTINCT handling to the Aggregate base class.
-------------------------------------+-------------------------------------

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"6516e49262546238d02f6ca37b74ee0e67dead0a" 6516e492]:
{{{
#!CommitTicketReference repository=""
revision="6516e49262546238d02f6ca37b74ee0e67dead0a"
[2.2.x] Fixed #30120 -- Fixed invalid SQL in distinct aggregate.

Regression in bc05547cd8c1dd511c6b6a6c873a1bc63417b111 (refs #28658).
Backport of 65858119d23e37872505a4476e7141c33981fb50 from master.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/28658#comment:8>

Django

unread,

Jan 21, 2019, 9:25:30 AM1/21/19

to django-...@googlegroups.com

#28658: Move DISTINCT handling to the Aggregate base class.
-------------------------------------+-------------------------------------

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"65858119d23e37872505a4476e7141c33981fb50" 65858119]:
{{{
#!CommitTicketReference repository=""
revision="65858119d23e37872505a4476e7141c33981fb50"