diff options
| -rw-r--r-- | django/db/models/sql/expressions.py | 9 | ||||
| -rw-r--r-- | django/db/models/sql/query.py | 2 | ||||
| -rw-r--r-- | docs/releases/1.5.txt | 6 | ||||
| -rw-r--r-- | tests/modeltests/expressions/tests.py | 39 |
4 files changed, 52 insertions, 4 deletions
diff --git a/django/db/models/sql/expressions.py b/django/db/models/sql/expressions.py index 374509914d..c809e25b49 100644 --- a/django/db/models/sql/expressions.py +++ b/django/db/models/sql/expressions.py @@ -1,14 +1,16 @@ from django.core.exceptions import FieldError from django.db.models.constants import LOOKUP_SEP from django.db.models.fields import FieldDoesNotExist +from django.db.models.sql.constants import REUSE_ALL class SQLEvaluator(object): - def __init__(self, expression, query, allow_joins=True): + def __init__(self, expression, query, allow_joins=True, reuse=REUSE_ALL): self.expression = expression self.opts = query.get_meta() self.cols = [] self.contains_aggregate = False + self.reuse = reuse self.expression.prepare(self, query, allow_joins) def prepare(self): @@ -50,9 +52,10 @@ class SQLEvaluator(object): try: field, source, opts, join_list, last, _ = query.setup_joins( field_list, query.get_meta(), - query.get_initial_alias(), False) + query.get_initial_alias(), self.reuse) col, _, join_list = query.trim_joins(source, join_list, last, False) - + if self.reuse is not None and self.reuse != REUSE_ALL: + self.reuse.update(join_list) self.cols.append((node, (join_list[-1], col))) except FieldDoesNotExist: raise FieldError("Cannot resolve keyword %r into field. " diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 4cfb816958..b03465b402 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -1096,7 +1096,7 @@ class Query(object): value = value() elif isinstance(value, ExpressionNode): # If value is a query expression, evaluate it - value = SQLEvaluator(value, self) + value = SQLEvaluator(value, self, reuse=can_reuse) having_clause = value.contains_aggregate for alias, aggregate in self.aggregates.items(): diff --git a/docs/releases/1.5.txt b/docs/releases/1.5.txt index e26b927ae7..ca6b3acfd9 100644 --- a/docs/releases/1.5.txt +++ b/docs/releases/1.5.txt @@ -589,6 +589,12 @@ Miscellaneous :ref:`Q() expressions <complex-lookups-with-q>` and ``QuerySet`` combining where the operators are used as boolean AND and OR operators. +* In a ``filter()`` call, when :ref:`F() expressions <query-expressions>` + contained lookups spanning multi-valued relations, they didn't always reuse + the same relations as other lookups along the same chain. This was changed, + and now F() expressions will always use the same relations as other lookups + within the same ``filter()`` call. + * 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. diff --git a/tests/modeltests/expressions/tests.py b/tests/modeltests/expressions/tests.py index 99eb07e370..ca47521ccd 100644 --- a/tests/modeltests/expressions/tests.py +++ b/tests/modeltests/expressions/tests.py @@ -219,3 +219,42 @@ class ExpressionsTests(TestCase): ) 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) + + def test_ticket_18375_chained_filters(self): + # Test that F() expressions do not reuse joins from previous filter. + qs = Employee.objects.filter( + company_ceo_set__num_employees=F('pk') + ).filter( + company_ceo_set__num_employees=F('company_ceo_set__num_employees') + ) + self.assertEqual(str(qs.query).count('JOIN'), 2) |