Opened 2 years ago

Closed 23 months ago

Last modified 23 months ago

#34285 closed Bug (fixed)

Index transforms on filtered array aggregates produces incorrect SQL query

Reported by: Nils Van Zuijlen Owned by: Nils Van Zuijlen
Component: Database layer (models, ORM) Version: 3.2
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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"

Change History (8)

comment:1 by Simon Charette, 2 years ago

Triage Stage: UnreviewedAccepted

Thanks for the report, it seems the issue lies in IndexTransform.as_sql where me might want to do

  • django/contrib/postgres/fields/array.py

    diff --git a/django/contrib/postgres/fields/array.py b/django/contrib/postgres/fields/array.py
    index 8477dd9fff..6c3b3080d5 100644
    a b def __init__(self, index, base_field, *args, **kwargs):  
    325325
    326326    def as_sql(self, compiler, connection):
    327327        lhs, params = compiler.compile(self.lhs)
    328         return "%s[%%s]" % lhs, params + [self.index]
     328        return "(%s)[%%s]" % lhs, params + [self.index]
    329329
    330330    @property
    331331    def output_field(self):

It seems that slicing is suffering from the same issue (e.g. director_ids__0_2)

Would you be interested in submitting a PR with this patch that includes a regression test?

comment:2 by Simon Charette, 2 years ago

Component: UncategorizedDatabase layer (models, ORM)

comment:3 by Nils Van Zuijlen, 23 months ago

Owner: changed from nobody to Nils Van Zuijlen
Status: newassigned

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.

comment:4 by Nils Van Zuijlen, 23 months ago

Has patch: set
Patch needs improvement: set
Last edited 23 months ago by Nils Van Zuijlen (previous) (diff)

comment:5 by Simon Charette, 23 months ago

Patch needs improvement: unset

Patch LGTM

comment:6 by Mariusz Felisiak, 23 months ago

Triage Stage: AcceptedReady for checkin

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 23 months ago

Resolution: fixed
Status: assignedclosed

In ae1fe72:

Fixed #34285 -- Fixed index/slice lookups on filtered aggregates with ArrayField.

Thanks Simon Charette for the review.

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 23 months ago

In e8a39da:

[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

Note: See TracTickets for help on using tickets.
Back to Top