#1810 - Whan paging over query with @Aggregation ( grouping by) wrong total count is calculated

src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java

@@ -66,7 +66,6 @@ import io.ebeaninternal.server.el.ElPropertyChainBuilder;
 import io.ebeaninternal.server.el.ElPropertyDeploy;
 import io.ebeaninternal.server.el.ElPropertyValue;
 import io.ebeaninternal.server.persist.DeleteMode;
-import io.ebeaninternal.server.persist.DmlUtil;
 import io.ebeaninternal.server.query.CQueryPlan;
 import io.ebeaninternal.server.query.ExtraJoin;
 import io.ebeaninternal.server.query.STreeProperty;
@@ -77,6 +76,7 @@ import io.ebeaninternal.server.query.STreeType;
 import io.ebeaninternal.server.query.SqlBeanLoad;
 import io.ebeaninternal.server.querydefn.DefaultOrmQuery;
 import io.ebeaninternal.server.querydefn.OrmQueryDetail;
+import io.ebeaninternal.server.querydefn.OrmQueryProperties;
 import io.ebeaninternal.server.rawsql.SpiRawSql;
 import io.ebeaninternal.server.type.DataBind;
 import io.ebeaninternal.server.type.ScalarType;
@@ -333,6 +333,7 @@ public class BeanDescriptor<T> implements BeanType<T>, STreeType {
    * list of properties that are Lists/Sets/Maps (Derived).
    */
   private final BeanProperty[] propertiesNonMany;
+  private final BeanProperty[] propertiesAggregate;
   private final BeanPropertyAssocMany<?>[] propertiesMany;
   private final BeanPropertyAssocMany<?>[] propertiesManySave;
   private final BeanPropertyAssocMany<?>[] propertiesManyDelete;
@@ -515,6 +516,7 @@ public class BeanDescriptor<T> implements BeanType<T>, STreeType {
 
     this.propertiesMany = listHelper.getMany();
     this.propertiesNonMany = listHelper.getNonMany();
+    this.propertiesAggregate = listHelper.getAggregates();
     this.propertiesManySave = listHelper.getManySave();
     this.propertiesManyDelete = listHelper.getManyDelete();
     this.propertiesManyToMany = listHelper.getManyToMany();
@@ -3169,6 +3171,25 @@ public class BeanDescriptor<T> implements BeanType<T>, STreeType {
     return propertiesEmbedded;
   }
 
+  /**
+   * Return true if the query detail includes an aggregation property.
+   */
+  public boolean includesAggregation(OrmQueryDetail detail) {
+    return detail != null && propertiesAggregate.length > 0 && includesAggregation(detail.getChunk(null, false));
+  }
+
+  private boolean includesAggregation(OrmQueryProperties rootProps) {
+    if (rootProps != null) {
+      final Set<String> included = rootProps.getIncluded();
+      for (BeanProperty property : propertiesAggregate) {
+        if (included.contains(property.getName())) {
+          return true;
+        }
+      }
+    }
+    return false;
+  }
+
   /**
    * Set all properties to be loaded (recurse to embedded beans).
    */

src/main/java/io/ebeaninternal/server/deploy/meta/DeployBeanPropertyLists.java

@@ -10,7 +10,6 @@ import io.ebeaninternal.server.deploy.BeanPropertyIdClass;
 import io.ebeaninternal.server.deploy.BeanPropertyOrderColumn;
 import io.ebeaninternal.server.deploy.BeanPropertySimpleCollection;
 import io.ebeaninternal.server.deploy.InheritInfo;
-import io.ebeaninternal.server.deploy.TableJoin;
 import io.ebeaninternal.server.deploy.generatedproperty.GeneratedProperty;
 import io.ebeaninternal.server.properties.BeanPropertySetter;
 import io.ebeaninternal.server.type.ScalarTypeString;
@@ -54,6 +53,8 @@ public class DeployBeanPropertyLists {
 
   private final List<BeanProperty> nonManys = new ArrayList<>();
 
+  private final List<BeanProperty> aggs = new ArrayList<>();
+
   private final List<BeanPropertyAssocOne<?>> ones = new ArrayList<>();
 
   private final List<BeanPropertyAssocOne<?>> onesImported = new ArrayList<>();
@@ -232,6 +233,9 @@ public class DeployBeanPropertyLists {
 
     } else {
       nonManys.add(prop);
+      if (prop.isAggregation()) {
+        aggs.add(prop);
+      }
       if (prop.isTenantId()) {
         tenant = prop;
       }
@@ -331,6 +335,10 @@ public class DeployBeanPropertyLists {
     return nonManys.toArray(new BeanProperty[0]);
   }
 
+  public BeanProperty[] getAggregates() {
+    return aggs.toArray(new BeanProperty[0]);
+  }
+
   public BeanPropertyAssocMany<?>[] getMany() {
     return manys.toArray(new BeanPropertyAssocMany[0]);
   }

src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java

@@ -14,7 +14,6 @@ import io.ebean.event.readaudit.ReadAuditQueryPlan;
 import io.ebean.text.PathProperties;
 import io.ebean.util.SplitName;
 import io.ebean.util.StringHelper;
-import io.ebeaninternal.api.ManyWhereJoins;
 import io.ebeaninternal.api.SpiQuery;
 import io.ebeaninternal.server.core.OrmQueryRequest;
 import io.ebeaninternal.server.deploy.BeanDescriptor;
@@ -44,9 +43,6 @@ import java.util.List;
  */
 class CQueryBuilder {
 
-  private static final String DELETE = "Delete";
-  private static final String UPDATE = "Update";
-
   final String tableAliasPlaceHolder;
   final String columnAliasPrefix;
 
@@ -104,7 +100,6 @@ class CQueryBuilder {
    */
   <T> CQueryUpdate buildUpdateQuery(boolean deleteRequest, OrmQueryRequest<T> request) {
 
-    String type = (deleteRequest) ? DELETE : UPDATE;
     SpiQuery<T> query = request.getQuery();
     String rootTableAlias = query.getAlias();
     query.setDelete();
@@ -263,12 +258,14 @@ class CQueryBuilder {
     query.setFirstRow(0);
     query.setMaxRows(0);
 
-    ManyWhereJoins manyWhereJoins = query.getManyWhereJoins();
-
     boolean countDistinct = query.isDistinct();
+    boolean withAgg = false;
     if (!countDistinct) {
-      // minimise select clause for standard count
-      query.setSelectId();
+      withAgg = includesAggregation(request, query);
+      if (!withAgg) {
+        // minimise select clause for standard count
+        query.setSelectId();
+      }
     }
 
     CQueryPredicates predicates = new CQueryPredicates(binder, request);
@@ -286,14 +283,14 @@ class CQueryBuilder {
       sqlTree.addSoftDeletePredicate(query);
     }
 
-    boolean hasMany = sqlTree.hasMany();
+    boolean wrap = sqlTree.hasMany() || withAgg;
 
     String sqlSelect = null;
     if (countDistinct) {
       if (sqlTree.isSingleProperty()) {
         request.setInlineCountDistinct();
       }
-    } else if (!hasMany) {
+    } else if (!wrap) {
       sqlSelect = "select count(*)";
     }
 
@@ -304,7 +301,7 @@ class CQueryBuilder {
       if (countDistinct) {
         sql = wrapSelectCount(sql);
 
-      } else if (hasMany || query.isRawSql()) {
+      } else if (wrap || query.isRawSql()) {
         // remove order by - mssql does not accept order by in subqueries
         int pos = sql.lastIndexOf(" order by ");
         if (pos != -1) {
@@ -321,6 +318,13 @@ class CQueryBuilder {
     return new CQueryRowCount(queryPlan, request, predicates);
   }
 
+  /**
+   * Return true if the query includes an aggregation property.
+   */
+  private <T> boolean includesAggregation(OrmQueryRequest<T> request, SpiQuery<T> query) {
+    return request.getBeanDescriptor().includesAggregation(query.getDetail());
+  }
+
   private String wrapSelectCount(String sql) {
     sql = "select count(*) from ( " + sql + ")";
     if (selectCountWithAlias) {

src/test/java/org/tests/iud/TestCarWheelIud.java

@@ -1,15 +1,21 @@
 package org.tests.iud;
 
 import io.ebean.BaseTestCase;
+import io.ebean.DB;
 import io.ebean.Ebean;
+import io.ebean.PagedList;
+import org.ebeantest.LoggedSqlCollector;
+import org.junit.Test;
 import org.tests.model.carwheel.Car;
 import org.tests.model.carwheel.Tire;
 import org.tests.model.carwheel.Wheel;
-import org.junit.Test;
 
 import java.util.ArrayList;
 import java.util.List;
 
+import static java.util.Arrays.asList;
+import static org.assertj.core.api.Assertions.assertThat;
+
 public class TestCarWheelIud extends BaseTestCase {
 
   @Test
@@ -53,4 +59,44 @@ public class TestCarWheelIud extends BaseTestCase {
     Ebean.delete(car2);
 
   }
+
+  @Test
+  public void aggregatePaging() {
+
+    DB.find(Car.class).delete();
+
+    final Car car0 = createCar("Ford", 40);
+    final Car car1 = createCar("Ford", 50);
+    final Car car2 = createCar("Mazda", 12);
+
+    DB.saveAll(asList(car0, car1, car2));
+
+    LoggedSqlCollector.start();
+
+    final PagedList<Car> pagedList = DB.find(Car.class)
+      .select("brand, totalSold")
+      .setMaxRows(10)
+      .findPagedList();
+
+    final List<Car> list = pagedList.getList();
+    final int count = pagedList.getTotalCount();
+
+    assertThat(list).hasSize(2);
+    assertThat(count).isEqualTo(2);
+
+    final List<String> sql = LoggedSqlCollector.stop();
+    assertThat(sql).hasSize(2);
+    if (isH2() || isPostgres()) {
+      assertThat(sql.get(0)).contains("select t0.brand, sum(t0.sold) from sa_car t0 group by t0.brand limit 10");
+      assertThat(sql.get(1)).contains("select count(*) from ( select t0.brand, sum(t0.sold) from sa_car t0 group by t0.brand)");
+    }
+
+  }
+
+  private Car createCar(String brand, int sold) {
+    Car car0 = new Car();
+    car0.setBrand(brand);
+    car0.setSold(sold);
+    return car0;
+  }
 }

src/test/java/org/tests/model/aggregation/TestAggregationTopLevel.java

@@ -33,6 +33,28 @@ public class TestAggregationTopLevel extends BaseTestCase {
     assertThat(result).isNotEmpty();
   }
 
+  @Test
+  public void query_count() {
+
+    Query<DMachineStatsAgg> query = DB.find(DMachineStatsAgg.class)
+      .select("date, totalKms")
+      .setMaxRows(10)
+      .having().gt("totalKms", 1)
+      .query();
+
+    LoggedSqlCollector.start();
+    query.findCount();
+    query.findList();
+
+    List<String> sql = LoggedSqlCollector.stop();
+    assertThat(sql).hasSize(2);
+
+    if (isH2() || isPostgres()) {
+      assertThat(sql.get(0)).contains("select count(*) from ( select t0.date, sum(t0.total_kms) from d_machine_stats t0 group by t0.date having sum(t0.total_kms) > ?)");
+      assertThat(sql.get(1)).contains("select t0.date, sum(t0.total_kms) from d_machine_stats t0 group by t0.date having sum(t0.total_kms) > ? limit 10");
+    }
+  }
+
   @Test
   public void query_machineTotalKms_withHaving() {
 

src/test/java/org/tests/model/carwheel/Car.java

@@ -1,5 +1,7 @@
 package org.tests.model.carwheel;
 
+import io.ebean.annotation.Aggregation;
+
 import javax.persistence.CascadeType;
 import javax.persistence.Entity;
 import javax.persistence.GeneratedValue;
@@ -21,6 +23,13 @@ public class Car {
   @Version
   private int version;
 
+  private String brand;
+
+  private int sold;
+
+  @Aggregation("sum(sold)")
+  private int totalSold;
+
   @OneToMany(mappedBy = "car", cascade = CascadeType.ALL)
   private List<Wheel> wheels;
 
@@ -40,6 +49,30 @@ public class Car {
     this.version = version;
   }
 
+  public String getBrand() {
+    return brand;
+  }
+
+  public void setBrand(String brand) {
+    this.brand = brand;
+  }
+
+  public int getSold() {
+    return sold;
+  }
+
+  public void setSold(int sold) {
+    this.sold = sold;
+  }
+
+  public int getTotalSold() {
+    return totalSold;
+  }
+
+  public void setTotalSold(int totalSold) {
+    this.totalSold = totalSold;
+  }
+
   public List<Wheel> getWheels() {
     return wheels;
   }