Code

Opened 4 years ago

Closed 8 months ago

#13017 closed Bug (needsinfo)

generic relation generates needless join

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

Description

I have an generic relation, and tries to do some database queries with this relation.
When looking at the generated sql query i saw, that double joins are created. This happens
because generic relations are based on m2m table with a missing database table.

An example for the model:

from django.db import models
from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes import generic

class GenericTest(models.Model):
    contentType = models.ForeignKey(ContentType)
    objectId = models.PositiveIntegerField()
    contentObject = generic.GenericForeignKey('contentType', 'objectId')

class RefTest(models.Model):
    generic = generic.GenericRelation(GenericTest, content_type_field='contentType', object_id_field='objectId')
    testdata = models.PositiveIntegerField()

the query:

GenericTest.objects.filter(reftest__testdata=0)

as sql:

('SELECT U0.`id` FROM `test_generictest` U0 INNER JOIN `test_generictest` U1 ON (U0.`id` = U1.`id`) INNER JOIN `test_reftest` U2 ON (U1.`objectId` = U2.`id`) WHERE U2.`testdata` = %s ', (0,))

With the applied patch, which made the same test, which is done in the direct case, the sql query looks like:

('SELECT U0.`id` FROM `test_generictest` U0 INNER JOIN `test_reftest` U1 ON (U0.`id` = U1.`id`) WHERE U1.`testdata` = %s ', (0,))

For simple databases and simple sql queries, this extra join is no problem, but with complex ones, its a huge performance lack.

Attachments (1)

sql_query.patch (1.4 KB) - added by MS 4 years ago.
Patch für Double Joins

Download all attachments as: .zip

Change History (15)

comment:1 Changed 4 years ago by russellm

  • milestone set to 1.2
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

This is possibly a regression caused by the recent m2m changes; needs further investigation.

comment:2 Changed 4 years ago by MS

This bug is also in Django 1.1 so i think it's not a regression.

comment:3 Changed 4 years ago by MS

The new patch has removed the double line, which was not nessecary in the patch before.

comment:4 Changed 4 years ago by russellm

  • milestone changed from 1.2 to 1.3

Not critical for 1.2

Changed 4 years ago by MS

Patch für Double Joins

comment:5 Changed 4 years ago by MS

There was another mistake in the patch, using the wrong database field with generic relations.

I am sad, that this patch won't be in 1.2, because it saves us a lot of time and ram used the database. Without it we would need a bigger server for database requests.

comment:6 Changed 3 years ago by lukeplant

  • Type set to Cleanup/optimization

comment:7 Changed 3 years ago by lukeplant

  • Severity set to Normal

comment:8 Changed 3 years ago by blacklwhite

  • Triage Stage changed from Accepted to Ready for checkin
Last edited 3 years ago by blacklwhite (previous) (diff)

comment:9 Changed 3 years ago by blacklwhite

  • Triage Stage changed from Ready for checkin to Accepted

Sorry, wrong ticket number.

Last edited 3 years ago by blacklwhite (previous) (diff)

comment:10 Changed 3 years ago by julien

  • Needs tests set
  • Type changed from Cleanup/optimization to Bug

comment:11 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

comment:11 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:12 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:13 Changed 8 months ago by akaariai

  • Resolution set to needsinfo
  • Status changed from new to closed

I am closing this as needsinfo. The query GenericTest.objects.filter(reftest__testdata=0) isn't valid Django query.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.