[Django] #27849: Support Postgres FILTER WHERE conditions in ArrayAgg
Django
--------------------------------------------+------------------------
Reporter: Tom | Owner: (none)
Type: New feature | Status: new
Component: contrib.postgres | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------------+------------------------
In some circumstances being able to filter results included in an ArrayAgg
is needed. PostgreSQL supports this through the FILTER WHERE clause:
{{{
SELECT ARRAY_AGG(id) FILTER (WHERE id < 10) as foo FROM table
}}}
Adding support for Q expressions in the ArrayAgg class could provide this
functionality, i.e:
{{{
SomeModel.objects.annotate(foo=ArrayAgg('some_relation__id',
where=Q(some_relation__bar=10)))
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27849>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Tom | Owner: (none)
Type: New feature | Status: new
Component: contrib.postgres | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tom):
I have added a completely basic, first-attempt at this here:
https://github.com/django/django/pull/8073
This currently works as described in the ticket above, but the code is not
optimal and is copied from our internal implementation. As this feature is
needed for the application I am working on I can continue to develop this,
but I have some questions:
Should this be an extension to `ArrayAgg` or a separate aggregate? If it
should be an extension, how can I change the `template` to add the `FILTER
(WHERE)` clause if required?
There doesn't seem to be an easy way to subclass `ArrayAgg` and add
elements to `data` or `params`, which is needed in this case. How could
this be achieved without duplicating the entire `as_sql` code as it is
currently? (it seems like the `as_sql` method should be broken up a bit, I
think?)
It would be quite nice to be able to pass a whole, full-fat `QuerySet`
into the aggregate, with the predicate that it is has been
`values_list`'ed and is a relation of the model being queried (i.e
`SomeModel.objects.annotate(foo=ArrayAgg('some_relation__id',
where=SomeRelation.objects.filter(parent=F('id')).values_list('xyz'))` or
somesuch. Is this even possible, or would it be chewing off too much?
--
Ticket URL: <https://code.djangoproject.com/ticket/27849#comment:1>
Django
Reporter: Tom | Owner: (none)
Type: New feature | Status: new
Component: contrib.postgres | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mads Jensen):
For what it's worth, I posted a snippet that solves a more generic version
of this a
https://djangosnippets.org/snippets/10603/ The syntax is not supported on
Oracle, MySQL nor SQLite, so I suppose new feature-flags could be
introduced etc.
--
Ticket URL: <https://code.djangoproject.com/ticket/27849#comment:2>
Django
Reporter: Tom | Owner: (none)
Type: New feature | Status: new
Component: contrib.postgres | Version: master
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
Comment:
You can ask on the DevelopersMailingList to get feedback about the design
decisions such as the syntax.
--
Ticket URL: <https://code.djangoproject.com/ticket/27849#comment:3>
Django
Reporter: Tom | Owner: (none)
Type: New feature | Status: new
Component: contrib.postgres | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tom):
I've added a new merge request:
[https://github.com/django/django/pull/8306]. I'm at the Djangocon sprints
for the next couple of days if anyone wishes to talk to me in person about
this, or has any comments.
--
Ticket URL: <https://code.djangoproject.com/ticket/27849#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Tom | Owner: nobody
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* owner: (none) => nobody
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* component: contrib.postgres => Database layer (models, ORM)
Comment:
[https://github.com/django/django/pull/8352 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/27849#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Tom | Owner: nobody
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
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/27849#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Tom | Owner: nobody
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27849#comment:7>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tom):
* owner: nobody => Tom
* needs_better_patch: 1 => 0
* status: new => assigned
Comment:
I've made the changes requested in the github ticket, any reviews would be
appreciated.
--
Ticket URL: <https://code.djangoproject.com/ticket/27849#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Tom | Owner: Tom
Type: New feature | Status: assigned
Component: Database layer | Version: master
(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
Changes (by felixxm):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/27849#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Tom | Owner: Tom
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
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:"b78d100fa62cd4fbbc70f2bae77c192cb36c1ccd" b78d100f]:
{{{
#!CommitTicketReference repository=""
revision="b78d100fa62cd4fbbc70f2bae77c192cb36c1ccd"
Fixed #27849 -- Added filtering support to aggregates.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27849#comment:10>
Django
-------------------------------------+-------------------------------------
| Forbes
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"b43acf22dfa59815a1f4db0558acd98816325f66" b43acf22]:
{{{
#!CommitTicketReference repository=""
revision="b43acf22dfa59815a1f4db0558acd98816325f66"
Refs #27849 -- Removed empty Q() hack in filtered Aggregate.as_sql().
This required allowing WhereNode to be provided as When(condition).
This was made possible by cf12257db23fa248c89a3da3f718aa01a50ca659.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27849#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Tom Forbes | Owner: Tom
| Forbes
Type: New feature | Status: closed
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"77cf70ea9699c3c4e74663955998753d70f65166" 77cf70ea]:
{{{
#!CommitTicketReference repository=""
revision="77cf70ea9699c3c4e74663955998753d70f65166"
Refs #27849 -- Added test for filtered aggregates with empty conditions.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27849#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: Tom Forbes | Owner: Tom
| Forbes
Type: New feature | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"967f8750ab63f9ca74ce63ada580ccc5b10b3f3b" 967f875]:
{{{
#!CommitTicketReference repository=""
revision="967f8750ab63f9ca74ce63ada580ccc5b10b3f3b"
Refs #27849 -- Fixed filtered aggregates crash on filters that match
everything.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27849#comment:13>