Extra-Join Regression from ticket #15316
|Reported by:||famousactress@…||Owned by:||nobody|
|Component:||Database layer (models, ORM)||Version:||1.4|
|Has patch:||no||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
The change to prevent over-trimming in trim_joins() for https://code.djangoproject.com/ticket/15316 appears to have introduced new, unneeded joins (LEFT OUTERs) as a side effect. The result is that in certain cases Django 1.4+ creates less efficient SQL than Django 1.3.
The following test passes in 1.3.x and fails w/ #15316's change. The failure is due to a new LEFT OUTER JOIN appearing against Document's delete_log.
class AuditLog(models.Model): info = models.CharField(max_length=50) class Person(models.Model): name = models.CharField(max_length=50) delete_log = models.ForeignKey(AuditLog) class Document(models.Model): status = models.CharField(max_length=50) delete_log = models.ForeignKey(AuditLog) person = models.ForeignKey(Person)
class OneFourRegressionProof(unittest.TestCase): """ Should evolve this into a better test case that proves something positive, but for now this is attempting to provide evidence that for some queries we got less efficient in Django 1.4 """ def setUp(self): pass def test_it(self): """ Ideally, this would only create on INNER join on person. """ qs = Document.objects.exclude(delete_log__isnull=False).filter(status='something', person__delete_log__isnull=True) self.assertEquals(1, str(qs.query).count('INNER JOIN')) self.assertEquals(0, str(qs.query).count('OUTER JOIN'))
git bisect results:
(django-base)~/projects/elation/django-fork ((2e56066...)|BISECTING) philltornroth → git bisect bad 2e56066a5b93b528fbce37285bac591b44bc6ed7 is the first bad commit commit 2e56066a5b93b528fbce37285bac591b44bc6ed7 Author: Malcolm Tredinnick <email@example.com> Date: Tue Aug 23 03:38:42 2011 +0000 Fixed an isnull=False filtering edge-case. Fixes #15316. The bulk of this patch is due to some fine analysis from Aleksandra Sendecka. git-svn-id: http://code.djangoproject.com/svn/django/trunk@16656 bcc190cf-cafb-0310-a4f2-bffc1f526a37 :100644 100644 5635f2141d5ed22bfcc03df7fd5456a48f7f051e 38c7601579c5b366a5027358a3806b24baeaddfb M AUTHORS :040000 040000 33a384a63b8e8d7e3b325640339f13a14babc315 fe57cf0e7bc932f15d181d3a186bffe453d6bb2a M django :040000 040000 afb2570e431195108c3d8b0b634e4b30eac42168 e1213b550270d389ffdca827116642b036855a6b M tests
It's worth noting that this is effectively an exacerbation of https://code.djangoproject.com/ticket/10790 . I began the discussion on that ticket and a patch is starting to take shape, but it looks too drastic to be a 1.4 merge candidate. I wanted to stand up this related ticket so that a more surgical fix to 1.4 could be considered, if reasonable.
Anecdotally, I'm seeing queries in our system where this change added 3-6 new joins, making an upgrade to 1.4 a complete non-starter for us unless there's a workaround that I haven't turned up yet.
Change History (9)
comment:1 Changed 4 years ago by akaariai
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
- Triage Stage changed from Unreviewed to Accepted
comment:8 Changed 3 years ago by Anssi Kääriäinen <akaariai@…>
- Resolution set to fixed
- Status changed from new to closed