queryset-refactor: Fixed the optimization that potentially removes the final

join to handle the case where a to_field attribute is given for the join.


git-svn-id: http://code.djangoproject.com/svn/django/branches/queryset-refactor@6495 bcc190cf-cafb-0310-a4f2-bffc1f526a37

2 files changed, 15 insertions, 17 deletions

diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index 20fbadfe51..ff0da5e6e2 100644
--- a/django/db/models/sql/query.py
+++ b/django/db/models/sql/query.py
@@ -576,6 +576,7 @@ class Query(object):
                 if not null_point and nullable:
                     null_point = len(join_list)
                 if connection == OR and not split:
+                    # FIXME: Document what's going on and why this is needed.
                     if self.alias_map[joins[0]][ALIAS_REFCOUNT] == 1:
                         split = True
                         self.promote_alias(joins[0])
@@ -595,15 +596,16 @@ class Query(object):
 
         col = target_col or target_field.column
 
-        if target_field is opts.pk and join_list:
-            # An optimization: if the final join is against a primary key,
-            # we can go back one step in the join chain and compare against
-            # the lhs of the join instead. The result (potentially) involves
-            # one less table join.
-            self.unref_alias(alias)
+        if join_list:
+            # An optimization: if the final join is against the same column as
+            # we are comparing against, we can go back one step in the join
+            # chain and compare against the lhs of the join instead. The result
+            # (potentially) involves one less table join.
             join = self.alias_map[join_list[-1][-1]][ALIAS_JOIN]
-            alias = join[LHS_ALIAS]
-            col = join[LHS_JOIN_COL]
+            if col == join[RHS_JOIN_COL]:
+                self.unref_alias(alias)
+                alias = join[LHS_ALIAS]
+                col = join[LHS_JOIN_COL]
 
         if (lookup_type == 'isnull' and value is True):
             # If the comparison is against NULL, we need to use a left outer
diff --git a/tests/regressiontests/queries/models.py b/tests/regressiontests/queries/models.py
index 9fa9b1e1e6..36161852a1 100644
--- a/tests/regressiontests/queries/models.py
+++ b/tests/regressiontests/queries/models.py
@@ -128,19 +128,15 @@ Bug #2253
 >>> (q1 & q2).order_by('name')
 [<Item: one>]
 
-Bugs #4088 & #4306
+Bugs #4088, #4306
 >>> Report.objects.filter(creator=1001)
 [<Report: r1>]
 >>> Report.objects.filter(creator__num=1001)
 [<Report: r1>]
-
-# FIXME: The "removing final pk comparison" optimization is biting us here.
-# Need to only remove it if the join was also on the pk value.
-# >>> Report.objects.filter(creator__id=1001)
-# []
-# >>> Report.objects.filter(creator__id=a1.id)
-# [<Report: r1>]
-
+>>> Report.objects.filter(creator__id=1001)
+[]
+>>> Report.objects.filter(creator__id=a1.id)
+[<Report: r1>]
 >>> Report.objects.filter(creator__name='a1')
 [<Report: r1>]