Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#10478 closed (invalid)

Multiple querysets as rvalues generating bad SQL

Reported by: mtredinnick Owned by: mtredinnick
Component: Database layer (models, ORM) Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Using these models for illustration:

class Person(models.Model):
    name = models.CharField(max_length=50)

class Group(models.Model):
    name = models.CharField(max_length=50)
    people = models.ManyToManyField(Person, related_name="groups")

If people is a queryset for Person objects, then this is a valid construction (in theory):

person_count = people.annotate(num=models.Count("id")).values("num")
groups = Group.objects.filter(people__in=people). \
             annotate(num_people=Count("people")). \
             filter(num_people=person_count)

Problem is, it gives the wrong answer. The SQL for the person_count inner query uses the same alias as the SQL for the people inner query.

SELECT "example_group"."id", "example_group"."name",
       COUNT("example_group_people"."person_id") AS "num"
FROM "example_group"
   LEFT OUTER JOIN "example_group_people"
      ON ("example_group"."id" = "example_group_people"."group_id")
WHERE "example_group_people"."person_id" IN (
   SELECT U0."id"
   FROM "example_person" U0
   WHERE U0."name" IN (%s, %s))
GROUP BY "example_group"."id", "example_group"."name"
HAVING COUNT("example_group_people"."person_id") =  (
   SELECT COUNT(U0."id") AS "num_x"
   FROM "example_person" U0 
   WHERE U0."name" IN (%s, %s) GROUP BY U0."id", U0."name")

This looks potentially fiddly to fix. Certainly need to rearrange the parameters to some calls, by the looks of it. To avoid having to store complex objects (pickling problems), we compute the SQL when the rvalue is inserted into the filter and it has no way of passing back the aliases it uses to the main query. We use get_db_prep_lookup() to do that SQL conversion, which has a bit of a limited interface. Overloading that use may not be the right way.

Should be fixable, but I need to think about it. Mostly creating a bug so that it's noted as a known issue.

Change History (3)

comment:1 Changed 6 years ago by mtredinnick

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 years ago by mtredinnick

  • Resolution set to invalid
  • Status changed from assigned to closed

I wasn't thinking clearly here. The results produced by this query are correct.

comment:3 Changed 4 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

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