[Django] #31228: SQLite does support DISTINCT on aggregate functions

[Django]

#31228: SQLite does support DISTINCT on aggregate functions
-------------------------------------+-------------------------------------
Reporter: André | Owner: nobody
Terra |
Type: Bug | Status: new
Component: Database | Version: 3.0
layer (models, ORM) |
Severity: Normal | Keywords: sqlite
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Contrary to what is suggested in
[https://github.com/django/django/blob/6f9ecc23f676e7a6f25d1a6cf830fe638a1eb589/django/db/backends/sqlite3/operations.py#L60
lines 60-64] of `django.db.backends.sqlite3.operations.py`, SQLite does
support DISTINCT on aggregate functions.

One such example is GROUP_CONCAT, which is quite similar to PostgreSQL's
STRING_AGG.

I can't find any canonical links which provide a useful explanation of
GROUP_CONCAT, but this should be good enough:
https://www.w3resource.com/sqlite/aggregate-functions-and-grouping-
group_concat.php

I came across this issue when trying to create my own `GroupConcat`
function subclassing `Aggregate` (similar to the
[https://github.com/django/django/blob/6f9ecc23f676e7a6f25d1a6cf830fe638a1eb589/django/contrib/postgres/aggregates/general.py#L53
StringAgg implementation] from postgres) and noticed that skipping the
check in `django.db.backends.sqlite3.operations.py` would allow my queries
to run as advertised.

My code for `GroupConcat` is:

{{{
from django.db.models import Value
from django.db.models.aggregates import Aggregate

class GroupConcat(Aggregate):
function = 'GROUP_CONCAT'
template = '%(function)s(%(distinct)s %(expressions)s)'
allow_distinct = True

def __init__(self, expression, delimiter=None, **extra):
if delimiter is not None:
self.allow_distinct = False
delimiter_expr = Value(str(delimiter))
super().__init__(expression, delimiter_expr, **extra)
else:
super().__init__(expression, **extra)

def as_sqlite(self, compiler, connection, **extra_context):
return super().as_sql(
compiler, connection,
function=self.function,
template=self.template,
**extra_context
)
}}}

For the record, as the code above suggests, a separate issue is that
GROUP_CONCAT only allows DISTINCT when a delimiter isn't specified.

After some discussion on #django, an argument was raised in favor of
changing the message to say "Django doesn't support...", but I would
imagine that skipping the check entirely would simply result in an
`OperationalError` for malformed queries while still allowing users to
extend the ORM as needed.

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

[Django]

#31228: DISTINCT with GROUP_CONCAT() and multiple expressions raises
NotSupportedError on SQLite.
-------------------------------------+-------------------------------------
Reporter: Andy Terra | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

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

Changes (by felixxm):

* cc: Simon Charette (added)
* version: 3.0 => master
* stage: Unreviewed => Accepted

Comment:

Thanks for this ticket. I agree that it shouldn't raise an error in some
cases, but removing this check is not an option, IMO.

Regression in bc05547cd8c1dd511c6b6a6c873a1bc63417b111.

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

[Django]

#31228: DISTINCT with GROUP_CONCAT() and multiple expressions raises
NotSupportedError on SQLite.
-------------------------------------+-------------------------------------
Reporter: Andy Terra | Owner: nobody

Type: Bug | Status: new

Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

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

Changes (by Hongtao Ma):

* cc: Hongtao Ma (added)

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

[Django]

#31228: DISTINCT with GROUP_CONCAT() and multiple expressions raises
NotSupportedError on SQLite.
-------------------------------------+-------------------------------------

Reporter: Andy Terra | Owner: Hongtao
| Ma
Type: Bug | Status: assigned

Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

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

Changes (by Hongtao Ma):

* owner: nobody => Hongtao Ma
* status: new => assigned

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

[Django]

#31228: DISTINCT with GROUP_CONCAT() and multiple expressions raises
NotSupportedError on SQLite.
-------------------------------------+-------------------------------------
Reporter: Andy Terra | Owner: Hongtao
| Ma
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

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

Comment (by Hongtao Ma):

There seems to be problem for '''concatenation style''' aggregation in
general, here is what i experimented on postgresql:
{{{
In [18]: Blog.objects.annotate(StringAgg('title', delimiter='x'))
---------------------------------------------------------------------------
TypeError Traceback (most recent call
last)
~/django/django/db/models/query.py in annotate(self, *args, **kwargs)
1071 try:
-> 1072 if arg.default_alias in kwargs:
1073 raise ValueError("The named annotation '%s'
conflicts with the "

~/django/django/db/models/aggregates.py in default_alias(self)
64 return '%s__%s' % (expressions[0].name,
self.name.lower())
---> 65 raise TypeError("Complex expressions require an alias")
66

TypeError: Complex expressions require an alias

During handling of the above exception, another exception occurred:

TypeError Traceback (most recent call
last)
<ipython-input-18-b924dae7712a> in <module>
----> 1 Blog.objects.annotate(StringAgg('title', delimiter='x'))

~/django/django/db/models/manager.py in manager_method(self, *args,
**kwargs)
80 def create_method(name, method):
81 def manager_method(self, *args, **kwargs):
---> 82 return getattr(self.get_queryset(), name)(*args,
**kwargs)
83 manager_method.__name__ = method.__name__
84 manager_method.__doc__ = method.__doc__

~/django/django/db/models/query.py in annotate(self, *args, **kwargs)
1075 % arg.default_alias)
1076 except TypeError:
-> 1077 raise TypeError("Complex annotations require an
alias")
1078 annotations[arg.default_alias] = arg
1079 annotations.update(kwargs)

TypeError: Complex annotations require an alias

In [19]:
}}}

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

[Django]

#31228: DISTINCT with GROUP_CONCAT() and multiple expressions raises
NotSupportedError on SQLite.
-------------------------------------+-------------------------------------
Reporter: Andy Terra | Owner: Hongtao
| Ma
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

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

Comment (by felixxm):

An error is quite descriptive IMO, ''"TypeError: Complex expressions
require an alias"''.

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

[Django]

#31228: DISTINCT with GROUP_CONCAT() and multiple expressions raises
NotSupportedError on SQLite.
-------------------------------------+-------------------------------------
Reporter: Andy Terra | Owner: Hongtao
| Ma
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

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

Comment (by Hongtao Ma):

Replying to [comment:5 felixxm]:

> An error is quite descriptive IMO, ''"TypeError: Complex expressions
require an alias"''.

Not quite sure about what kind of 'expressions' deserve being called
complex. But I assume '''StringAgg('title', delimiter='x') ''' shouldn't
be one of them. The problem here is that the delimiter part of StringAgg
is included in the expressions, which probably makes it complex.

From the syntax defined by Postgresql, the expression is explicitly
separated from the delimiter:
{{{string_agg(expression, delimiter)}}}
[https://www.postgresql.org/docs/11/functions-aggregate.html string_agg]

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

[Django]

#31228: DISTINCT with GROUP_CONCAT() and multiple expressions raises
NotSupportedError on SQLite.
-------------------------------------+-------------------------------------
Reporter: Andy Terra | Owner: Hongtao
| Ma
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

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

Comment (by felixxm):

Yes, because `delimiter` is treated as an expression in `StringAgg`.
Please don't add unrelated comments/questions to tickets.

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

[Django]

#31228: DISTINCT with GROUP_CONCAT() and multiple expressions raises
NotSupportedError on SQLite.
-------------------------------------+-------------------------------------
Reporter: Andy Terra | Owner: Hongtao
| Ma
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

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

Changes (by Hongtao Ma):

* cc: Hongtao Ma (removed)
* has_patch: 0 => 1
* needs_tests: 0 => 1

Comment:

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

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

[Django]

#31228: DISTINCT with GROUP_CONCAT() and multiple expressions raises
NotSupportedError on SQLite.
-------------------------------------+-------------------------------------
Reporter: Andy Terra | Owner: Hongtao
| Ma
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:

Keywords: sqlite | 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 felixxm):

* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/31228#comment:9>

[Django]

#31228: DISTINCT with GROUP_CONCAT() and multiple expressions raises
NotSupportedError on SQLite.
-------------------------------------+-------------------------------------
Reporter: Andy Terra | Owner: Hongtao
| Ma

Type: Bug | Status: closed

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

Severity: Normal | Resolution: fixed

Keywords: sqlite | 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 <felisiak.mariusz@…>):

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

Comment:

In [changeset:"cbb6531e5bef7ffe0c46d6c44d598d7bcdf9029e" cbb6531]:
{{{
#!CommitTicketReference repository=""
revision="cbb6531e5bef7ffe0c46d6c44d598d7bcdf9029e"
Fixed #31228 -- Reallowed aggregates to be used with multiple expressions
and no DISTINCT on SQLite.

Regression in bc05547cd8c1dd511c6b6a6c873a1bc63417b111.

Thanks Andy Terra for the report.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31228#comment:10>