Repository: kayak/pypika
Pull request #29 (2016-12-28 20:53:17)

More fixes for is_aggregate, Count and Expr/Case with all None terms
Fixed the Count function to be an AggregateFunction and fixed another bug in is_aggregate for ArithmeticExpression and Case when all contained terms have None for is_aggregate


Patch:
diff --git a/pypika/functions.py b/pypika/functions.py
index d327dca..c119144 100644
--- a/pypika/functions.py
+++ b/pypika/functions.py
@@ -10,7 +10,7 @@ __author__ = "Timothy Heys"
 __email__ = "theys@kayak.com"
 
 
-class Count(Function):
+class Count(AggregateFunction):
     def __init__(self, param, alias=None):
         is_star = isinstance(param, str) and '*' == param
         super(Count, self).__init__('COUNT', Star() if is_star else param, alias=alias)
diff --git a/pypika/terms.py b/pypika/terms.py
index 76fd5be..88d3494 100644
--- a/pypika/terms.py
+++ b/pypika/terms.py
@@ -5,7 +5,7 @@ from datetime import date
 from aenum import Enum
 
 from pypika.enums import Boolean, Equality, Arithmetic, Matching
-from pypika.utils import CaseException, builder
+from pypika.utils import CaseException, builder, resolve_is_aggregate
 
 __author__ = "Timothy Heys"
 __email__ = "theys@kayak.com"
@@ -479,11 +479,8 @@ class ArithmeticExpression(Term):
 
     @property
     def is_aggregate(self):
-        # True if both left and right is aggregate. Ignores None
-        return all(filter(
-            lambda x: x is not None,
-            [self.left.is_aggregate, self.right.is_aggregate]
-        ))
+        # True if both left and right terms are True or None. None if both terms are None. Otherwise, False
+        return resolve_is_aggregate([self.left.is_aggregate, self.right.is_aggregate])
 
     @property
     def tables_(self):
@@ -526,13 +523,9 @@ class Case(Term):
 
     @property
     def is_aggregate(self):
-        cases = [term.is_aggregate
-                 for _, term in self._cases]
-
-        if self._else:
-            cases.append(self._else.is_aggregate)
-
-        return all(filter(lambda x: x is not None, cases))
+        # True if all cases are True or None. None all cases are None. Otherwise, False
+        return resolve_is_aggregate([term.is_aggregate for _, term in self._cases]
+                                    + [self._else.is_aggregate if self._else else None])
 
     @builder
     def when(self, criterion, term):
diff --git a/pypika/utils.py b/pypika/utils.py
index f79beae..4ab525a 100644
--- a/pypika/utils.py
+++ b/pypika/utils.py
@@ -36,3 +36,20 @@ def builder(func):
         return func(self_copy, *args, **kwargs) or self_copy
 
     return _decorator
+
+
+def resolve_is_aggregate(values):
+    """
+    Resolves the is_aggregate flag for an expression that contains multiple terms.  This works like a voter system,
+    each term votes True or False or abstains with None.
+
+    :param values: A list of booleans (or None) for each term in the expression
+    :return: If all values are True or None, True is returned.  If all values are None, None is returned. Otherwise,
+        False is returned.
+    """
+    result = [x
+              for x in values
+              if x is not None]
+    if result:
+        return all(result)
+    return None


Test patch:
diff --git a/pypika/tests/test_aggregate.py b/pypika/tests/test_aggregate.py
index 0efe9c3..b37c7bd 100644
--- a/pypika/tests/test_aggregate.py
+++ b/pypika/tests/test_aggregate.py
@@ -11,10 +11,14 @@ class IsAggregateTests(unittest.TestCase):
         v = Field('foo')
         self.assertFalse(v.is_aggregate)
 
-    def test__valuewrapper_is_neither_aggr_or_not(self):
+    def test__constant_is_neither_aggr_or_not(self):
         v = ValueWrapper(100)
         self.assertIsNone(v.is_aggregate)
 
+    def test__constant_arithmetic_is_neither_aggr_or_not(self):
+        v = ValueWrapper(100) + ValueWrapper(100)
+        self.assertIsNone(v.is_aggregate)
+
     def test__field_arithmetic_is_not_aggregate(self):
         v = Field('foo') + Field('bar')
         self.assertFalse(v.is_aggregate)
@@ -69,3 +73,11 @@ class IsAggregateTests(unittest.TestCase):
             .else_(1)
 
         self.assertTrue(v.is_aggregate)
+
+    def test__case_all_constants_is_neither_aggr_or_not(self):
+        v = Case() \
+            .when(Field('foo') == 1, 1) \
+            .when(Field('foo') == 2, 2) \
+            .else_(3)
+
+        self.assertIsNone(v.is_aggregate)