From 3a1886d1113a68794fe36aafd771a7e5db703e24 Mon Sep 17 00:00:00 2001 From: Josh Smeaton Date: Fri, 20 Mar 2015 15:47:43 +1100 Subject: [PATCH] [1.8.x] Fixed #24508 -- Made annotations commutative Backport of 127b3873d03704f77428b984de022664b268314e from master --- django/db/models/expressions.py | 14 ++++++++++++-- tests/annotations/tests.py | 8 ++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py index 6e4722d4f3..c48204f420 100644 --- a/django/db/models/expressions.py +++ b/django/db/models/expressions.py @@ -238,7 +238,16 @@ class BaseExpression(object): """ Attempts to infer the output type of the expression. If the output fields of all source fields match then we can simply infer the same - type here. + type here. This isn't always correct, but it makes sense most of the + time. + + Consider the difference between `2 + 2` and `2 / 3`. Inferring + the type here is a convenience for the common case. The user should + supply their own output_field with more complex computations. + + If a source does not have an `_output_field` then we exclude it from + this check. If all sources are `None`, then an error will be thrown + higher up the stack in the `output_field` property. """ if self._output_field is None: sources = self.get_source_fields() @@ -246,8 +255,9 @@ class BaseExpression(object): if num_sources == 0: self._output_field = None else: - self._output_field = sources[0] for source in sources: + if self._output_field is None: + self._output_field = source if source is not None and not isinstance(self._output_field, source.__class__): raise FieldError( "Expression contains mixed types. You must set output_field") diff --git a/tests/annotations/tests.py b/tests/annotations/tests.py index 3f50463b48..55ef30b575 100644 --- a/tests/annotations/tests.py +++ b/tests/annotations/tests.py @@ -99,6 +99,14 @@ class NonAggregateAnnotationTestCase(TestCase): sum_rating=Sum('rating') ).filter(sum_rating=F('nope'))) + def test_combined_annotation_commutative(self): + book1 = Book.objects.annotate(adjusted_rating=F('rating') + 2).get(pk=self.b1.pk) + book2 = Book.objects.annotate(adjusted_rating=2 + F('rating')).get(pk=self.b1.pk) + self.assertEqual(book1.adjusted_rating, book2.adjusted_rating) + book1 = Book.objects.annotate(adjusted_rating=F('rating') + None).get(pk=self.b1.pk) + book2 = Book.objects.annotate(adjusted_rating=None + F('rating')).get(pk=self.b1.pk) + self.assertEqual(book1.adjusted_rating, book2.adjusted_rating) + def test_update_with_annotation(self): book_preupdate = Book.objects.get(pk=2) Book.objects.annotate(other_rating=F('rating') - 1).update(rating=F('other_rating'))