[Django] #34285: Index transforms on filtered array aggregates produces incorrect SQL query
Django
--------------------------------------------+------------------------
Reporter: nils-van-zuijlen | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 3.2
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 |
--------------------------------------------+------------------------
For example, with the following models:
{{{
class MembershipKind(models.TextChoices):
MEMBER = "member", _("Member")
DIRECTOR = "director", _("Director")
MANAGER = "manager", _("Manager")
class Project(models.Model):
name = models.CharField(max_length=255)
members = models.ManyToManyField(User, related_name="projects",
blank=True, through="ProjectMember")
class ProjectMember(models.Model):
project = models.ForeignKey(
Project, related_name="memberships", on_delete=models.CASCADE,
verbose_name=_("project")
)
user = models.ForeignKey(User, related_name="project_memberships",
on_delete=models.CASCADE, verbose_name=_("user"))
kind = models.CharField(choices=MembershipKind.choices, max_length=10)
class Meta:
constraints = [models.UniqueConstraint(fields=("project", "user"),
name="project_user_unique_link")]
}}}
The following query has missing parenthesis around the annotated field
first_director_id.
{{{
>>> Project.objects.all().annotate(
... director_ids=ArrayAgg('memberships__user_id',
filter=Q(memberships__kind=MembershipKind.DIRECTOR))
... ).annotate(
... first_director_id=F('director_ids__0')
... ).query.sql_with_params()
(
'''SELECT "imputations_project"."id", "imputations_project"."name",
ARRAY_AGG("imputations_projectmember"."user_id" ) FILTER (WHERE
"imputations_projectmember"."kind" = %s) AS "director_ids",
ARRAY_AGG("imputations_projectmember"."user_id" ) FILTER (WHERE
"imputations_projectmember"."kind" = %s)[%s] AS "first_director_id"
FROM "imputations_project"
LEFT OUTER JOIN "imputations_projectmember" ON ("imputations_project"."id"
= "imputations_projectmember"."project_id")
GROUP BY "imputations_project"."id"''',
(<MembershipKind.DIRECTOR: 'director'>, <MembershipKind.DIRECTOR:
'director'>, 1)
)
}}}
It should be
{{{
(ARRAY_AGG("imputations_projectmember"."user_id" ) FILTER (WHERE
"imputations_projectmember"."kind" = %s))[%s] AS "first_director_id"
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34285>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: nils-van-zuijlen | Owner: nobody
Type: Bug | Status: closed
Component: Uncategorized | Version: 3.2
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Ashad8949):
* status: new => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/34285#comment:1>
Django
----------------------------------+------------------------------------
Type: Bug | Status: new
Component: Uncategorized | Version: 3.2
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
* stage: Unreviewed => Accepted
Comment:
Thanks for the report, it seems the issue lies in `IndexTransform.as_sql`
where me might want to do
{{{#!diff
diff --git a/django/contrib/postgres/fields/array.py
b/django/contrib/postgres/fields/array.py
index 8477dd9fff..6c3b3080d5 100644
--- a/django/contrib/postgres/fields/array.py
+++ b/django/contrib/postgres/fields/array.py
@@ -325,7 +325,7 @@ def __init__(self, index, base_field, *args,
**kwargs):
def as_sql(self, compiler, connection):
lhs, params = compiler.compile(self.lhs)
- return "%s[%%s]" % lhs, params + [self.index]
+ return "(%s)[%%s]" % lhs, params + [self.index]
@property
def output_field(self):
}}}
It seems that slicing is suffering from the same issue (e.g.
`director_ids__0_2`)
Would you be interested
[https://docs.djangoproject.com/en/4.1/intro/contributing/ in submitting a
PR with this patch that includes a regression test]?
Django
Reporter: Nils Van Zuijlen | Owner: nobody
Type: Bug | Status: new
(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 Simon Charette):
* component: Uncategorized => Database layer (models, ORM)
--
Ticket URL: <https://code.djangoproject.com/ticket/34285#comment:2>
Django
-------------------------------------+-------------------------------------
| Zuijlen
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(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
-------------------------------------+-------------------------------------
* owner: nobody => Nils Van Zuijlen
* status: new => assigned
Comment:
I tried to write unit tests on main, but they failed with another error.
The proposed solution makes another test on integer nested indexation
fail, but I don't know how to fix that.
--
Ticket URL: <https://code.djangoproject.com/ticket/34285#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Nils Van Zuijlen | Owner: Nils Van
| Zuijlen
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Nils Van Zuijlen):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/16504 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/34285#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Nils Van Zuijlen | Owner: Nils Van
| Zuijlen
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
Patch LGTM
--
Ticket URL: <https://code.djangoproject.com/ticket/34285#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Nils Van Zuijlen | Owner: Nils Van
| Zuijlen
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(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/34285#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Nils Van Zuijlen | Owner: Nils Van
| Zuijlen
Component: Database layer | Version: 3.2
(models, ORM) |
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
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"ae1fe72e9b1f5fe3b05e5b670bd0c205cd305e71" ae1fe72]:
{{{
#!CommitTicketReference repository=""
revision="ae1fe72e9b1f5fe3b05e5b670bd0c205cd305e71"
Fixed #34285 -- Fixed index/slice lookups on filtered aggregates with
ArrayField.
Thanks Simon Charette for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34285#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Nils Van Zuijlen | Owner: Nils Van
| Zuijlen
Type: Bug | Status: closed
Component: Database layer | Version: 3.2
(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 <felisiak.mariusz@…>):
In [changeset:"e8a39da396b3ed8e469f569e4e865a54ae5dad0b" e8a39da]:
{{{
#!CommitTicketReference repository=""
revision="e8a39da396b3ed8e469f569e4e865a54ae5dad0b"
[4.2.x] Fixed #34285 -- Fixed index/slice lookups on filtered aggregates
with ArrayField.
Thanks Simon Charette for the review.
Backport of ae1fe72e9b1f5fe3b05e5b670bd0c205cd305e71 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34285#comment:8>