Opened 21 months ago

Last modified 21 months ago

#20535 new Bug

Unnecessary join created for intermediate table between two M2M tables

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

Description

In this example:

from django.db import models

class Person(models.Model):
    name = models.CharField(max_length=128)
    friends = models.ManyToManyField('self', symmetrical=False, through='Relationship')

    def __unicode__(self):
        return self.name

class Relationship(models.Model):
    from_person = models.ForeignKey(Person, related_name='idols')
    to_person = models.ForeignKey(Person, related_name='followers')

Example data for a test:

ringo, _ = Person.objects.get_or_create(name="Ringo Starr")
paul, _ = Person.objects.get_or_create(name="Paul McCartney")
george, _ = Person.objects.get_or_create(name="George Harrison")
john, _ = Person.objects.get_or_create(name="John Lennon")
yoko, _ = Person.objects.get_or_create(name="Yoko Ono")

Relationship.objects.get_or_create(from_person=ringo, to_person=paul)
Relationship.objects.get_or_create(from_person=george, to_person=john)
Relationship.objects.get_or_create(from_person=paul, to_person=ringo)
Relationship.objects.get_or_create(from_person=paul, to_person=george)
Relationship.objects.get_or_create(from_person=paul, to_person=john)
Relationship.objects.get_or_create(from_person=john, to_person=paul)
Relationship.objects.get_or_create(from_person=john, to_person=yoko)
Relationship.objects.get_or_create(from_person=yoko, to_person=john)

Then doing (get idols of the idols of paul):

    print Relationship.objects.filter(from_person__followers__from_person=paul).query

This produces a join between Relationship and Person, then to Relationship, then to Person. The final join (to Person) gets trimmed by trim_joins() after the setup_joins() in Query.add_filter(). However, the unneeded join with Person (between the two joined Relationship) is not removed. This produces something like:

SELECT ... FROM relationship INNER JOIN person ON (relationship.from_person_id = person.id) INNER JOIN relationship T3 ON (person.id = T3.to_person_id) WHERE T3.from_person_id = 2;

When in fact all that would be needed is:

SELECT ... FROM relationship INNER JOIN relationship T3 ON (relationship.from_person_id = T3.to_person_id) WHERE T3.from_person_id = 2

Attachments (4)

#20535-tests.diff (1.4 KB) - added by Kronuz 21 months ago.
#20535-tests_master.diff (572 bytes) - added by Kronuz 21 months ago.
Tests for master
#20353-Unnecessary_join created for intermediate_table_between_two_M2M_tables.diff (3.5 KB) - added by Kronuz 21 months ago.
Patch with tests
#20535-Unnecessary_join created for intermediate_table_between_two_M2M_tables.diff (3.4 KB) - added by Kronuz 21 months ago.

Download all attachments as: .zip

Change History (7)

Changed 21 months ago by Kronuz

Changed 21 months ago by Kronuz

Tests for master

comment:1 Changed 21 months ago by Kronuz

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 21 months ago by Kronuz

  • Version changed from 1.5 to master

comment:3 Changed 21 months ago by akaariai

  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Yes, skipping non-necessary intermediate joins would be good. Unfortunately the added patch seems to introduce a lot of errors in the test suite.

Maybe it would be better to do the trimming at compiler.get_from_clause() time? A join can be skipped if there are as many tables joining to it as its alias_refcount is, and every joined table can be joined directly to the "parent" table instead.

Trimming joins before joins promotion is done is a bad idea. I am not sure if this applies to internal join trimming though.

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