Ticket #18375: ticket_18375_15.diff

django/db/models/sql/expressions.py

@@ -9 +9 @@
         self.cols = []
 
         self.contains_aggregate = False
+        self.used_joins = []
         self.expression.prepare(self, query, allow_joins)
 
     def prepare(self):
@@ -51 +52 @@
                 field, source, opts, join_list, last, _ = query.setup_joins(
                     field_list, query.get_meta(),
                     query.get_initial_alias(), False)
+                # Note that even if we trim some joins away, the joins are
+                # still usable by this node.
+                self.used_joins.extend(join_list)
                 col, _, join_list = query.trim_joins(source, join_list, last, False)
-
                 self.cols.append((node, (join_list[-1], col)))
             except FieldDoesNotExist:
                 raise FieldError("Cannot resolve keyword %r into field. "

django/db/models/sql/query.py

@@ -1100 +1100 @@
         elif isinstance(value, ExpressionNode):
             # If value is a query expression, evaluate it
             value = SQLEvaluator(value, self)
+            if value.used_joins and can_reuse is not None:
+                can_reuse.update(value.used_joins)
+
             having_clause = value.contains_aggregate
 
         for alias, aggregate in self.aggregates.items():

docs/releases/1.5.txt

@@ -563 +563 @@
   :ref:`Q() expressions <complex-lookups-with-q>` and ``QuerySet`` combining where
   the operators are used as boolean AND and OR operators.
 
+* When :ref:`F() expressions <query-expressions>` were used in lookups spanning
+  multi-valued relations it was possible that the F() and other lookups in a
+  single filter() call didn't target the same relation. This was change and now
+  F() expressions will reuse the same relation.
+
 * The :ttag:`csrf_token` template tag is no longer enclosed in a div. If you need
   HTML validation against pre-HTML5 Strict DTDs, you should add a div around it
   in your pages.

tests/modeltests/expressions/tests.py

@@ -219 +219 @@
         )
         acme.num_employees = F("num_employees") + 16
         self.assertRaises(TypeError, acme.save)
+
+    def test_ticket_18375_join_reuse(self):
+        # Test that reverse multijoin F() references and the lookup target
+        # the same join. Pre #18375 the F() join was generated first, and the
+        # lookup couldn't reuse that join.
+        qs = Employee.objects.filter(
+            company_ceo_set__num_chairs=F('company_ceo_set__num_employees'))
+        self.assertEqual(str(qs.query).count('JOIN'), 1)
+
+    def test_ticket_18375_kwarg_ordering(self):
+        # The next query was dict-randomization dependent - if the "gte=1"
+        # was seen first, then the F() will reuse the join generated by the
+        # gte lookup, if F() was seen first, then it generated a join the
+        # other lookups could not reuse.
+        qs = Employee.objects.filter(
+            company_ceo_set__num_chairs=F('company_ceo_set__num_employees'),
+            company_ceo_set__num_chairs__gte=1)
+        self.assertEqual(str(qs.query).count('JOIN'), 1)
+
+    def test_ticket_18375_kwarg_ordering_2(self):
+        # Another similar case for F() than above. Now we have the same join
+        # in two filter kwargs, one in the lhs lookup, one in F. Here pre
+        # #18375 the amount of joins generated was random if dict
+        # randomization was enabled, that is the generated query dependend
+        # on which clause was seen first.
+        qs = Employee.objects.filter(
+            company_ceo_set__num_employees=F('pk'),
+            pk=F('company_ceo_set__num_employees')
+        )
+        self.assertEqual(str(qs.query).count('JOIN'), 1)