Author: mtredinnick
Date: 2011-08-22 20:38:42 -0700 (Mon, 22 Aug 2011)
New Revision: 16656
Modified:
django/trunk/AUTHORS
django/trunk/django/db/models/sql/query.py
django/trunk/tests/regressiontests/queries/models.py
django/trunk/tests/regressiontests/queries/tests.py
Log:
Fixed an isnull=False filtering edge-case. Fixes #15316.
The bulk of this patch is due to some fine analysis from Aleksandra
Sendecka.
Modified: django/trunk/AUTHORS
===================================================================
--- django/trunk/AUTHORS 2011-08-23 03:38:28 UTC (rev 16655)
+++ django/trunk/AUTHORS 2011-08-23 03:38:42 UTC (rev 16656)
@@ -448,6 +448,7 @@
[email protected]
[email protected]
Ilya Semenov <[email protected]>
+ Aleksandra Sendecka <[email protected]>
[email protected]
John Shaffer <[email protected]>
Pete Shinners <[email protected]>
Modified: django/trunk/django/db/models/sql/query.py
===================================================================
--- django/trunk/django/db/models/sql/query.py 2011-08-23 03:38:28 UTC (rev
16655)
+++ django/trunk/django/db/models/sql/query.py 2011-08-23 03:38:42 UTC (rev
16656)
@@ -1105,7 +1105,10 @@
# Process the join list to see if we can remove any inner joins from
# the far end (fewer tables in a query is better).
- col, alias, join_list = self.trim_joins(target, join_list, last, trim)
+ nonnull_comparison = (lookup_type == 'isnull' and value is False)
+ col, alias, join_list = self.trim_joins(target, join_list, last, trim,
+ nonnull_comparison)
+
if connector == OR:
# Some joins may need to be promoted when adding a new filter to a
# disjunction. We walk the list of new joins and where it diverges
@@ -1442,7 +1445,7 @@
return field, target, opts, joins, last, extra_filters
- def trim_joins(self, target, join_list, last, trim):
+ def trim_joins(self, target, join_list, last, trim, nonnull_check=False):
"""
Sometimes joins at the end of a multi-table sequence can be trimmed. If
the final join is against the same column as we are comparing against,
@@ -1463,6 +1466,11 @@
trimmed before anything. See the documentation of add_filter() for
details about this.
+ The 'nonnull_check' parameter is True when we are using inner joins
+ between tables explicitly to exclude NULL entries. In that case, the
+ tables shouldn't be trimmed, because the very action of joining to them
+ alters the result set.
+
Returns the final active column and table alias and the new active
join_list.
"""
@@ -1470,7 +1478,7 @@
penultimate = last.pop()
if penultimate == final:
penultimate = last.pop()
- if trim and len(join_list) > 1:
+ if trim and final > 1:
extra = join_list[penultimate:]
join_list = join_list[:penultimate]
final = penultimate
@@ -1483,12 +1491,13 @@
alias = join_list[-1]
while final > 1:
join = self.alias_map[alias]
- if col != join[RHS_JOIN_COL] or join[JOIN_TYPE] != self.INNER:
+ if (col != join[RHS_JOIN_COL] or join[JOIN_TYPE] != self.INNER or
+ nonnull_check):
break
self.unref_alias(alias)
alias = join[LHS_ALIAS]
col = join[LHS_JOIN_COL]
- join_list = join_list[:-1]
+ join_list.pop()
final -= 1
if final == penultimate:
penultimate = last.pop()
Modified: django/trunk/tests/regressiontests/queries/models.py
===================================================================
--- django/trunk/tests/regressiontests/queries/models.py 2011-08-23
03:38:28 UTC (rev 16655)
+++ django/trunk/tests/regressiontests/queries/models.py 2011-08-23
03:38:42 UTC (rev 16656)
@@ -317,3 +317,29 @@
def __unicode__(self):
return self.name
+
+class SimpleCategory(models.Model):
+ name = models.CharField(max_length=10)
+
+ def __unicode__(self):
+ return self.name
+
+class SpecialCategory(SimpleCategory):
+ special_name = models.CharField(max_length=10)
+
+ def __unicode__(self):
+ return self.name + " " + self.special_name
+
+class CategoryItem(models.Model):
+ category = models.ForeignKey(SimpleCategory)
+
+ def __unicode__(self):
+ return "category item: " + str(self.category)
+
+class OneToOneCategory(models.Model):
+ new_name = models.CharField(max_length=10)
+ category = models.OneToOneField(SimpleCategory)
+
+ def __unicode__(self):
+ return "one2one " + self.new_name
+
\ No newline at end of file
Modified: django/trunk/tests/regressiontests/queries/tests.py
===================================================================
--- django/trunk/tests/regressiontests/queries/tests.py 2011-08-23 03:38:28 UTC
(rev 16655)
+++ django/trunk/tests/regressiontests/queries/tests.py 2011-08-23 03:38:42 UTC
(rev 16656)
@@ -15,7 +15,7 @@
DumbCategory, ExtraInfo, Fan, Item, LeafA, LoopX, LoopZ, ManagedModel,
Member, NamedCategory, Note, Number, Plaything, PointerA, Ranking, Related,
Report, ReservedName, Tag, TvChef, Valid, X, Food, Eaten, Node, ObjectA,
ObjectB,
- ObjectC)
+ ObjectC, CategoryItem, SimpleCategory, SpecialCategory, OneToOneCategory)
class BaseQuerysetTest(TestCase):
@@ -1043,11 +1043,135 @@
[]
)
+ def test_ticket15316_filter_false(self):
+ c1 = SimpleCategory.objects.create(name="category1")
+ c2 = SpecialCategory.objects.create(name="named category1",
+ special_name="special1")
+ c3 = SpecialCategory.objects.create(name="named category2",
+ special_name="special2")
+ ci1 = CategoryItem.objects.create(category=c1)
+ ci2 = CategoryItem.objects.create(category=c2)
+ ci3 = CategoryItem.objects.create(category=c3)
+
+ qs =
CategoryItem.objects.filter(category__specialcategory__isnull=False)
+ self.assertEqual(qs.count(), 2)
+ self.assertQuerysetEqual(qs, [ci2.pk, ci3.pk], lambda x: x.pk, False)
+
+ def test_ticket15316_exclude_false(self):
+ c1 = SimpleCategory.objects.create(name="category1")
+ c2 = SpecialCategory.objects.create(name="named category1",
+ special_name="special1")
+ c3 = SpecialCategory.objects.create(name="named category2",
+ special_name="special2")
+
+ ci1 = CategoryItem.objects.create(category=c1)
+ ci2 = CategoryItem.objects.create(category=c2)
+ ci3 = CategoryItem.objects.create(category=c3)
+
+ qs =
CategoryItem.objects.exclude(category__specialcategory__isnull=False)
+ self.assertEqual(qs.count(), 1)
+ self.assertQuerysetEqual(qs, [ci1.pk], lambda x: x.pk)
+
+ def test_ticket15316_filter_true(self):
+ c1 = SimpleCategory.objects.create(name="category1")
+ c2 = SpecialCategory.objects.create(name="named category1",
+ special_name="special1")
+ c3 = SpecialCategory.objects.create(name="named category2",
+ special_name="special2")
+
+ ci1 = CategoryItem.objects.create(category=c1)
+ ci2 = CategoryItem.objects.create(category=c2)
+ ci3 = CategoryItem.objects.create(category=c3)
+
+ qs =
CategoryItem.objects.filter(category__specialcategory__isnull=True)
+ self.assertEqual(qs.count(), 1)
+ self.assertQuerysetEqual(qs, [ci1.pk], lambda x: x.pk)
+
+ def test_ticket15316_exclude_true(self):
+ c1 = SimpleCategory.objects.create(name="category1")
+ c2 = SpecialCategory.objects.create(name="named category1",
+ special_name="special1")
+ c3 = SpecialCategory.objects.create(name="named category2",
+ special_name="special2")
+
+ ci1 = CategoryItem.objects.create(category=c1)
+ ci2 = CategoryItem.objects.create(category=c2)
+ ci3 = CategoryItem.objects.create(category=c3)
+
+ qs =
CategoryItem.objects.exclude(category__specialcategory__isnull=True)
+ self.assertEqual(qs.count(), 2)
+ self.assertQuerysetEqual(qs, [ci2.pk, ci3.pk], lambda x: x.pk, False)
+
+ def test_ticket15316_one2one_filter_false(self):
+ c = SimpleCategory.objects.create(name="cat")
+ c0 = SimpleCategory.objects.create(name="cat0")
+ c1 = SimpleCategory.objects.create(name="category1")
+
+ c2 = OneToOneCategory.objects.create(category = c1, new_name="new1")
+ c3 = OneToOneCategory.objects.create(category = c0, new_name="new2")
+
+ ci1 = CategoryItem.objects.create(category=c)
+ ci2 = CategoryItem.objects.create(category=c0)
+ ci3 = CategoryItem.objects.create(category=c1)
+
+ qs =
CategoryItem.objects.filter(category__onetoonecategory__isnull=False)
+ self.assertEqual(qs.count(), 2)
+ self.assertQuerysetEqual(qs, [ci2.pk, ci3.pk], lambda x: x.pk, False)
+
+ def test_ticket15316_one2one_exclude_false(self):
+ c = SimpleCategory.objects.create(name="cat")
+ c0 = SimpleCategory.objects.create(name="cat0")
+ c1 = SimpleCategory.objects.create(name="category1")
+
+ c2 = OneToOneCategory.objects.create(category = c1, new_name="new1")
+ c3 = OneToOneCategory.objects.create(category = c0, new_name="new2")
+
+ ci1 = CategoryItem.objects.create(category=c)
+ ci2 = CategoryItem.objects.create(category=c0)
+ ci3 = CategoryItem.objects.create(category=c1)
+
+ qs =
CategoryItem.objects.exclude(category__onetoonecategory__isnull=False)
+ self.assertEqual(qs.count(), 1)
+ self.assertQuerysetEqual(qs, [ci1.pk], lambda x: x.pk)
+
+ def test_ticket15316_one2one_filter_true(self):
+ c = SimpleCategory.objects.create(name="cat")
+ c0 = SimpleCategory.objects.create(name="cat0")
+ c1 = SimpleCategory.objects.create(name="category1")
+
+ c2 = OneToOneCategory.objects.create(category = c1, new_name="new1")
+ c3 = OneToOneCategory.objects.create(category = c0, new_name="new2")
+
+ ci1 = CategoryItem.objects.create(category=c)
+ ci2 = CategoryItem.objects.create(category=c0)
+ ci3 = CategoryItem.objects.create(category=c1)
+
+ qs =
CategoryItem.objects.filter(category__onetoonecategory__isnull=True)
+ self.assertEqual(qs.count(), 1)
+ self.assertQuerysetEqual(qs, [ci1.pk], lambda x: x.pk)
+
+ def test_ticket15316_one2one_exclude_true(self):
+ c = SimpleCategory.objects.create(name="cat")
+ c0 = SimpleCategory.objects.create(name="cat0")
+ c1 = SimpleCategory.objects.create(name="category1")
+
+ c2 = OneToOneCategory.objects.create(category = c1, new_name="new1")
+ c3 = OneToOneCategory.objects.create(category = c0, new_name="new2")
+
+ ci1 = CategoryItem.objects.create(category=c)
+ ci2 = CategoryItem.objects.create(category=c0)
+ ci3 = CategoryItem.objects.create(category=c1)
+
+ qs =
CategoryItem.objects.exclude(category__onetoonecategory__isnull=True)
+ self.assertEqual(qs.count(), 2)
+ self.assertQuerysetEqual(qs, [ci2.pk, ci3.pk], lambda x: x.pk, False)
+
+
class Queries5Tests(TestCase):
def setUp(self):
- # Ordering by 'rank' gives us rank2, rank1, rank3. Ordering by the
Meta.ordering
- # will be rank3, rank2, rank1.
+ # Ordering by 'rank' gives us rank2, rank1, rank3. Ordering by the
+ # Meta.ordering will be rank3, rank2, rank1.
n1 = Note.objects.create(note='n1', misc='foo', id=1)
n2 = Note.objects.create(note='n2', misc='bar', id=2)
e1 = ExtraInfo.objects.create(info='e1', note=n1)
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
