Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #11293 -- fixed using Q objects to generate ORs with aggregates.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@15173 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 29de7ee9cb78f4216faec7499044fdf65c1b2511 1 parent 1b90cdc
Alex Gaynor authored January 11, 2011
52  django/db/models/sql/query.py
@@ -922,6 +922,19 @@ def remove_inherited_models(self):
922 922
                 self.unref_alias(alias)
923 923
         self.included_inherited_models = {}
924 924
 
  925
+    def need_force_having(self, q_object):
  926
+        """
  927
+        Returns whether or not all elements of this q_object need to be put
  928
+        together in the HAVING clause.
  929
+        """
  930
+        for child in q_object.children:
  931
+            if isinstance(child, Node):
  932
+                if self.need_force_having(child):
  933
+                    return True
  934
+            else:
  935
+                if child[0].split(LOOKUP_SEP)[0] in self.aggregates:
  936
+                    return True
  937
+        return False
925 938
 
926 939
     def add_aggregate(self, aggregate, model, alias, is_summary):
927 940
         """
@@ -972,7 +985,7 @@ def add_aggregate(self, aggregate, model, alias, is_summary):
972 985
         aggregate.add_to_query(self, alias, col=col, source=source, is_summary=is_summary)
973 986
 
974 987
     def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
975  
-            can_reuse=None, process_extras=True):
  988
+            can_reuse=None, process_extras=True, force_having=False):
976 989
         """
977 990
         Add a single filter to the query. The 'filter_expr' is a pair:
978 991
         (filter_string, value). E.g. ('name__contains', 'fred')
@@ -1026,14 +1039,14 @@ def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
1026 1039
             value = SQLEvaluator(value, self)
1027 1040
             having_clause = value.contains_aggregate
1028 1041
 
1029  
-        for alias, aggregate in self.aggregates.items():
1030  
-            if alias == parts[0]:
1031  
-                entry = self.where_class()
1032  
-                entry.add((aggregate, lookup_type, value), AND)
1033  
-                if negate:
1034  
-                    entry.negate()
1035  
-                self.having.add(entry, AND)
1036  
-                return
  1042
+        if parts[0] in self.aggregates:
  1043
+            aggregate = self.aggregates[parts[0]]
  1044
+            entry = self.where_class()
  1045
+            entry.add((aggregate, lookup_type, value), AND)
  1046
+            if negate:
  1047
+                entry.negate()
  1048
+            self.having.add(entry, connector)
  1049
+            return
1037 1050
 
1038 1051
         opts = self.get_meta()
1039 1052
         alias = self.get_initial_alias()
@@ -1082,7 +1095,7 @@ def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
1082 1095
             self.promote_alias_chain(table_it, table_promote)
1083 1096
 
1084 1097
 
1085  
-        if having_clause:
  1098
+        if having_clause or force_having:
1086 1099
             if (alias, col) not in self.group_by:
1087 1100
                 self.group_by.append((alias, col))
1088 1101
             self.having.add((Constraint(alias, col, field), lookup_type, value),
@@ -1123,7 +1136,7 @@ def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
1123 1136
                 self.add_filter(filter, negate=negate, can_reuse=can_reuse,
1124 1137
                         process_extras=False)
1125 1138
 
1126  
-    def add_q(self, q_object, used_aliases=None):
  1139
+    def add_q(self, q_object, used_aliases=None, force_having=False):
1127 1140
         """
1128 1141
         Adds a Q-object to the current filter.
1129 1142
 
@@ -1141,16 +1154,25 @@ def add_q(self, q_object, used_aliases=None):
1141 1154
             else:
1142 1155
                 subtree = False
1143 1156
             connector = AND
  1157
+            if q_object.connector == OR and not force_having:
  1158
+                force_having = self.need_force_having(q_object)
1144 1159
             for child in q_object.children:
1145 1160
                 if connector == OR:
1146 1161
                     refcounts_before = self.alias_refcount.copy()
1147  
-                self.where.start_subtree(connector)
  1162
+                if force_having:
  1163
+                    self.having.start_subtree(connector)
  1164
+                else:
  1165
+                    self.where.start_subtree(connector)
1148 1166
                 if isinstance(child, Node):
1149  
-                    self.add_q(child, used_aliases)
  1167
+                    self.add_q(child, used_aliases, force_having=force_having)
1150 1168
                 else:
1151 1169
                     self.add_filter(child, connector, q_object.negated,
1152  
-                            can_reuse=used_aliases)
1153  
-                self.where.end_subtree()
  1170
+                            can_reuse=used_aliases, force_having=force_having)
  1171
+                if force_having:
  1172
+                    self.having.end_subtree()
  1173
+                else:
  1174
+                    self.where.end_subtree()
  1175
+
1154 1176
                 if connector == OR:
1155 1177
                     # Aliases that were newly added or not used at all need to
1156 1178
                     # be promoted to outer joins if they are nullable relations.
54  tests/regressiontests/aggregation_regress/tests.py
@@ -4,8 +4,8 @@
4 4
 from operator import attrgetter
5 5
 
6 6
 from django.core.exceptions import FieldError
  7
+from django.db.models import Count, Max, Avg, Sum, StdDev, Variance, F, Q
7 8
 from django.test import TestCase, Approximate, skipUnlessDBFeature
8  
-from django.db.models import Count, Max, Avg, Sum, StdDev, Variance, F
9 9
 
10 10
 from models import Author, Book, Publisher, Clues, Entries, HardbackBook
11 11
 
@@ -673,6 +673,58 @@ def test_having_group_by(self):
673 673
             list(qs), list(Book.objects.values_list("name", flat=True))
674 674
         )
675 675
 
  676
+    def test_annotation_disjunction(self):
  677
+        qs = Book.objects.annotate(n_authors=Count("authors")).filter(
  678
+            Q(n_authors=2) | Q(name="Python Web Development with Django")
  679
+        )
  680
+        self.assertQuerysetEqual(
  681
+            qs, [
  682
+                "Artificial Intelligence: A Modern Approach",
  683
+                "Python Web Development with Django",
  684
+                "The Definitive Guide to Django: Web Development Done Right",
  685
+            ],
  686
+            attrgetter("name")
  687
+        )
  688
+
  689
+        qs = Book.objects.annotate(n_authors=Count("authors")).filter(
  690
+            Q(name="The Definitive Guide to Django: Web Development Done Right") | (Q(name="Artificial Intelligence: A Modern Approach") & Q(n_authors=3))
  691
+        )
  692
+        self.assertQuerysetEqual(
  693
+            qs, [
  694
+                "The Definitive Guide to Django: Web Development Done Right",
  695
+            ],
  696
+            attrgetter("name")
  697
+        )
  698
+
  699
+        qs = Publisher.objects.annotate(
  700
+            rating_sum=Sum("book__rating"),
  701
+            book_count=Count("book")
  702
+        ).filter(
  703
+            Q(rating_sum__gt=5.5) | Q(rating_sum__isnull=True)
  704
+        ).order_by('pk')
  705
+        self.assertQuerysetEqual(
  706
+            qs, [
  707
+                "Apress",
  708
+                "Prentice Hall",
  709
+                "Jonno's House of Books",
  710
+            ],
  711
+            attrgetter("name")
  712
+        )
  713
+
  714
+        qs = Publisher.objects.annotate(
  715
+            rating_sum=Sum("book__rating"),
  716
+            book_count=Count("book")
  717
+        ).filter(
  718
+            Q(pk__lt=F("book_count")) | Q(rating_sum=None)
  719
+        ).order_by("pk")
  720
+        self.assertQuerysetEqual(
  721
+            qs, [
  722
+                "Apress",
  723
+                "Jonno's House of Books",
  724
+            ],
  725
+            attrgetter("name")
  726
+        )
  727
+
676 728
     @skipUnlessDBFeature('supports_stddev')
677 729
     def test_stddev(self):
678 730
         self.assertEqual(

0 notes on commit 29de7ee

Please sign in to comment.
Something went wrong with that request. Please try again.