[Django] #28762: Can't aggregate annotations with array parameters
Django
-------------------------------------+-------------------------------------
Reporter: Daniel | Owner: nobody
Keller |
Type: | Status: new
Uncategorized |
Component: Database | Version: 1.11
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 |
-------------------------------------+-------------------------------------
If I use a database function that takes an array parameter, for example a
histogram:
{{{
import math
from django.db import models
from django.contrib.postgres.fields import ArrayField
class Log2Bucket(models.Func):
function = 'width_bucket'
def __init__(self, expression, a, b, **extra):
max_exp = math.ceil(math.log2(b / a))
buckets = [a * 2**exp for exp in range(max_exp + 1)]
buckets = models.Value(buckets,
output_field=ArrayField(models.FloatField())) ###
super().__init__(expression, buckets,
output_field=models.PositiveSmallIntegerField(), **extra)
}}}
and a simple model
{{{
class Foo(models.Model):
bar = models.FloatField()
}}}
It works fine in queries like
{{{
Foo.objects.annotate(b=Log2Bucket('bar', 1, 100)).values('b')
}}}
but for
{{{
Foo.objects.annotate(b=Log2Bucket('bar', 1,
100)).values('b').annotate(count=models.Count('pk'))
}}}
I get
{{{
...
File "/home/daniel_keller/shared/raumplus_2/env/lib/python3.4/site-
packages/django/db/models/sql/compiler.py", line 128, in get_group_by
if (sql, tuple(params)) not in seen:
TypeError: unhashable type: 'list'
}}}
referring to the list passed to `Value`.
Honestly, I'm not sure if this counts as a bug or a new feature, since
it's clearly unintended behavior, but none of Django's database functions
need it to work.
I can work around it by replacing the marked line with
{{{
buckets = Value("{%s}" % ','.join(map(str, buckets))) ###
}}}
but this is IMO very ugly.
--
Ticket URL: <https://code.djangoproject.com/ticket/28762>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Uncategorized | Status: new
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Josh Smeaton (added)
Comment:
I'm not sure what the proper resolution is either, however, I see the
[https://docs.djangoproject.com/en/dev/ref/models/expressions/#value-
expressions documentation] for `Value` says, "When you need to represent
the value of an integer, boolean, or string within an expression, you can
wrap that value within a `Value()`." You're passing a `list` to `Value`
which contradicts the documentation -- perhaps a more helpful message
could be raised.
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:1>
Django
-------------------------------------+-------------------------------------
| Chachamu
Type: Uncategorized | Status: assigned
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => Tomer Chachamu
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:2>
Django
-------------------------------------+-------------------------------------
| Chachamu
Type: Uncategorized | Status: assigned
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Josh Smeaton):
Specifically, `Value` can represent types that the database driver (such
as psycopg2) has adapters for. But the bug you're hitting is that Value
represents itself as a a SQL parameter, and is then hashed to see if it
already exists.
Can you try ExpressionList
https://github.com/django/django/blob/master/django/db/models/expressions.py#L774
and see if that works for your case? It might be worth documenting
ExpressionList beside Value for these cases if it works.
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:3>
Django
-------------------------------------+-------------------------------------
Type: Uncategorized | Status: new
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Tomer Chachamu => (none)
* status: assigned => new
* has_patch: 0 => 1
* stage: Unreviewed => Accepted
Comment:
It seems like we just need to make the parameters to `cur.execute` always
be hashable. So in this case the list `[1, 2, 4, 8, 16, 32, 64, 128]`
returned from `get_db_prep_value`. I've added a patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:4>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Uncategorized => Bug
* needs_tests: 0 => 1
Comment:
Tests are needed to demonstrate what's fixed.
Most likely the patch doesn't qualify for a backport to 1.11 based on our
[https://docs.djangoproject.com/en/dev/internals/release-process
/#supported-versions supported versions policy] (it would have to be a
regression from an older version of Django) so you can remove the release
note in the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:5>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
#28767 may be related if not a duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:6>
Django
-------------------------------------+-------------------------------------
| Chachamu
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: (none) => Tomer Chachamu
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:7>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => new
* owner: Tomer Chachamu => (none)
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:8>
Django
-------------------------------------+-------------------------------------
| Chachamu
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: (none) => Tomer Chachamu
* status: new => assigned
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:9>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => new
* owner: Tomer Chachamu => (none)
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:10>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:11>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Tomer Chachamu (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:12>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
Comment:
I left comments for improvement on the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:13>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
#29139 is the same exception but a different cause involving aggregating
`JSONField` with `KeyTransform`. The proposed patch doesn't fix that issue
(not that is has to).
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:14>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"ceab25bc6d0d095acfcea46c35d93c17d008fd35" ceab25bc]:
{{{
#!CommitTicketReference repository=""
revision="ceab25bc6d0d095acfcea46c35d93c17d008fd35"
Refs #28762 -- Added test for aggregating over a function with ArrayField
parameters.
Fixed in d87bd29c4f8dfcdf3f4a4eb8340e6770a2416fe3.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:15>
Django
-------------------------------------+-------------------------------------
Component: Database layer | Version: 2.2
(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
-------------------------------------+-------------------------------------
* status: new => closed
* needs_better_patch: 1 => 0
* version: 1.11 => 2.2
* resolution: => fixed
Comment:
Fixed in d87bd29c4f8dfcdf3f4a4eb8340e6770a2416fe3.
--
Ticket URL: <https://code.djangoproject.com/ticket/28762#comment:16>