ISPN-5732 Aggregation functions should ignore NULL values

infinispan · Sep 10, 2015 · f8a1014 · f8a1014
1 parent 2132479
commit f8a1014
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 8 deletions.
diff --git a/...edded-query-it/src/test/java/org/infinispan/all/embeddedquery/QueryDslConditionsTest.java b/...edded-query-it/src/test/java/org/infinispan/all/embeddedquery/QueryDslConditionsTest.java
@@ -2052,6 +2052,30 @@ public void testOrderBySum() {
       assertEquals(22, list.get(0)[0]);
    }
 
+   @Test
+   public void testCountNull() {
+      QueryFactory qf = getQueryFactory();
+      Query q = qf.from(getModelFactory().getUserImplClass())
+            .select(Expression.count("age"))
+            .build();
+      List<Object[]> list = q.list();
+      assertEquals(1, list.size());
+      assertEquals(1, list.get(0).length);
+      assertEquals(1L, list.get(0)[0]);  // only non-null "age"s were counted
+   }
+
+   @Test
+   public void testAvgNull() {
+      QueryFactory qf = getQueryFactory();
+      Query q = qf.from(getModelFactory().getUserImplClass())
+            .select(Expression.avg("age"))
+            .build();
+      List<Object[]> list = q.list();
+      assertEquals(1, list.size());
+      assertEquals(1, list.get(0).length);
+      assertEquals(22.0, list.get(0)[0]);  // only non-null "age"s were counted
+   }
+
    @Test
    public void testParam() throws Exception {
       QueryFactory qf = getQueryFactory();

diff --git a/object-filter/src/main/java/org/infinispan/objectfilter/impl/aggregation/AvgAccumulator.java b/object-filter/src/main/java/org/infinispan/objectfilter/impl/aggregation/AvgAccumulator.java
@@ -10,9 +10,9 @@ public final class AvgAccumulator extends FieldAccumulator {
 
    private static class Avg {
       double sum;
-      int count;
+      long count;
 
-      Avg(double sum, int count) {
+      Avg(double sum, long count) {
          this.sum = sum;
          this.count = count;
       }

diff --git a/...t-filter/src/main/java/org/infinispan/objectfilter/impl/aggregation/CountAccumulator.java b/...t-filter/src/main/java/org/infinispan/objectfilter/impl/aggregation/CountAccumulator.java
@@ -1,6 +1,8 @@
 package org.infinispan.objectfilter.impl.aggregation;
 
 /**
+ * COUNT returns a Long greater or equal than 0. Null values are not counted.
+ *
  * @author anistor@redhat.com
  * @since 8.0
  */
@@ -12,11 +14,13 @@ public CountAccumulator(int pos) {
 
    @Override
    public void init(Object[] row) {
-      row[pos] = 1;
+      row[pos] = row[pos] != null ? 1L : 0L;
    }
 
    @Override
    public void update(Object[] srcRow, Object[] destRow) {
-      destRow[pos] = (Integer) destRow[pos] + 1;
+      if (srcRow[pos] != null) {
+         destRow[pos] = (Long) destRow[pos] + 1;
+      }
    }
 }
diff --git a/query/src/test/java/org/infinispan/query/dsl/embedded/QueryDslConditionsTest.java b/query/src/test/java/org/infinispan/query/dsl/embedded/QueryDslConditionsTest.java
@@ -1932,6 +1932,28 @@ public void testOrderBySum() {
       assertEquals(22, list.get(0)[0]);
    }
 
+   public void testCountNull() {
+      QueryFactory qf = getQueryFactory();
+      Query q = qf.from(getModelFactory().getUserImplClass())
+            .select(Expression.count("age"))
+            .build();
+      List<Object[]> list = q.list();
+      assertEquals(1, list.size());
+      assertEquals(1, list.get(0).length);
+      assertEquals(1L, list.get(0)[0]);  // only non-null "age"s were counted
+   }
+
+   public void testAvgNull() {
+      QueryFactory qf = getQueryFactory();
+      Query q = qf.from(getModelFactory().getUserImplClass())
+            .select(Expression.avg("age"))
+            .build();
+      List<Object[]> list = q.list();
+      assertEquals(1, list.size());
+      assertEquals(1, list.get(0).length);
+      assertEquals(22.0, list.get(0)[0]);  // only non-null "age"s were counted
+   }
+
    public void testParam() throws Exception {
       QueryFactory qf = getQueryFactory();
 

diff --git a/query/src/test/java/org/infinispan/query/dsl/embedded/impl/QueryEngineTest.java b/query/src/test/java/org/infinispan/query/dsl/embedded/impl/QueryEngineTest.java
@@ -291,8 +291,8 @@ public void testGlobalCount() {
       List<Object[]> list = q.list();
       assertEquals(1, list.size());
       assertEquals(2, list.get(0).length);
-      assertEquals(3, list.get(0)[0]);
-      assertEquals(3, list.get(0)[1]);
+      assertEquals(3L, list.get(0)[0]);
+      assertEquals(2L, list.get(0)[1]);
    }
 
    public void testGlobalAvg() {
@@ -332,9 +332,9 @@ public void testAggregateGroupingField() {
       List<Object[]> list = q.list();
       assertEquals(2, list.size());
       assertEquals(1, list.get(0).length);
-      assertEquals(1, list.get(0)[0]);
+      assertEquals(1L, list.get(0)[0]);
       assertEquals(1, list.get(1).length);
-      assertEquals(2, list.get(1)[0]);
+      assertEquals(2L, list.get(1)[0]);
    }
 
    public void testAggregateEmbedded1() {