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 Changed 4 years ago by Claude Paroz

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

comment:2 Changed 4 years ago by Baptiste Mispelon

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 Changed 4 years ago by Mariusz Felisiak

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

comment:4 Changed 4 years ago by Simon Charette

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 selected 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.
Last edited 4 years ago by Simon Charette (previous) (diff)

comment:5 Changed 4 years ago by Simon Charette

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

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

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 Changed 4 years ago by Mariusz Felisiak <felisiak.mariusz@…>

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