Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#31109 closed Bug (fixed)

Multiple annotation with Subquery generates wrong SQL.

Reported by: Thierry Bastian Owned by: Simon Charette
Component: Database layer (models, ORM) Version: 3.0
Severity: Release blocker Keywords:
Cc: Simon Charette Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have 2 models Item and ItemInfo.

class Item(models.Model):
    name = TextField()
    ...

class ItemInfo(models.Model):
    id = models.OneToOneField(Item, models.CASCADE, db_column='id', primary_key=True)
    size = models.BigIntegerField(blank=True, null=True)
    ...

If I do a QuerySet like this:

        Item.objects \
            .annotate(
            size=Subquery(
                ItemInfo.objects.filter(id=OuterRef('id')).values_list(
                    'size', flat=True
                )[:1]
            ),
            depends_on=StringAgg('name', ', ' )
        )

If I simply execute it, it works. Please don't look at the QuerySet, I really tried to make it the simplest example ever
But if I do a .exists() I'm getting

django.db.utils.ProgrammingError: column "size" does not exist
LINE 1...

Obviously that used to work with any 2.x version.

Change History (7)

comment:1 by Claude Paroz, 4 years ago

Could this be a duplicate of #31094? Maybe you could try with that patch applied?

comment:2 by Baptiste Mispelon, 4 years ago

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

I reproduced the issue in the current master (abeb4599af5ea7a384e18ec6201a95c9e419b44a).

This regression seems to have been caused by fb3f034f1c63160c0ff13c609acd01c18be12f80 (found using git bisect).

I'll bump the severity to "release blocker" because of the regression.

Thanks!

comment:3 by Mariusz Felisiak, 4 years ago

Cc: Simon Charette added
Summary: multiple annotation with SubQuery generates wrong SQLMultiple annotation with Subquery generates wrong SQL.

comment:4 by Simon Charette, 4 years ago

So here's what happens here.

  1. Since StringAgg is an aggregate it sets queryset.query.group_by = True on annotation.
  2. QuerySet.exists() relies on Query.has_results
  3. Query.has_results does the following when distinct=False and group_by=True.
    1. It adds all the the concrete fields of the model to address #24835
    2. Calls set_group_by which happens to add a Ref referring to the selected Subquery annotation to group_by since fb3f034f1c63160c0ff13c609acd01c18be12f80 .
    3. It then calls clear_select_clause which orphans the Ref since it now points to column that isn't referenced to anymore.

The solutions is either to:

  1. Add a parameter to Query.set_group_by to prevent GROUP BY by aliases; effectively passing alias=None to all its annotation.get_group_by_cols calls.
  2. Make clear_select_clause remove all Ref to annotation_select entries from group_by before calling set_annotation_mask(()).
  1. is likely less invasive and seems more correct given the sole caller requiring this behaviour does so because it knows it'll call clear_select_clause right after.
Version 0, edited 4 years ago by Simon Charette (next)

comment:5 by Simon Charette, 4 years ago

Has patch: set
Owner: changed from nobody to Simon Charette
Status: newassigned

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In 720de4d:

Fixed #31109 -- Disabled grouping by aliases on QuerySet.exists().

Clearing the SELECT clause in Query.has_results was orphaning GROUP BY
references to it.

Thanks Thierry Bastian for the report and Baptiste Mispelon for the
bisect.

Regression in fb3f034f1c63160c0ff13c609acd01c18be12f80.

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In 7b065c41:

[3.0.x] Fixed #31109 -- Disabled grouping by aliases on QuerySet.exists().

Clearing the SELECT clause in Query.has_results was orphaning GROUP BY
references to it.

Thanks Thierry Bastian for the report and Baptiste Mispelon for the
bisect.

Regression in fb3f034f1c63160c0ff13c609acd01c18be12f80.

Backport of 720de4d0441fcfdb543051389c70efbe66ed962a from master

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