#99 - ENH (419) : Simplify aggregation queries - part 2

src/main/java/com/avaje/ebeaninternal/api/ManyWhereJoins.java

@@ -155,4 +155,11 @@ public class ManyWhereJoins implements Serializable {
     aggregation = true;
   }
 
+  /**
+   * Ensure we have the join required to support the aggregation properties.
+   */
+  public void addAggregationJoin(String property) {
+    this.aggregation = true;
+    joins.put(property, new PropertyJoin(property, SqlJoinType.INNER));
+  }
 }

src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeBuilder.java

@@ -236,11 +236,17 @@ public class SqlTreeBuilder {
       }
     }
 
+    OrmQueryProperties queryProps = queryDetail.getChunk(prefix, false);
+    SqlTreeProperties props = getBaseSelect(desc, queryProps);
+
     if (prefix == null && !rawSql) {
+      if (props.requireSqlDistinct(manyWhereJoins)) {
+        query.setSqlDistinct(true);
+      }
       addManyWhereJoins(myJoinList);
     }
 
-    SqlTreeNode selectNode = buildNode(prefix, prop, desc, myJoinList);
+    SqlTreeNode selectNode = buildNode(prefix, prop, desc, myJoinList, props);
     if (joinList != null) {
       joinList.add(selectNode);
     }
@@ -264,11 +270,7 @@ public class SqlTreeBuilder {
     }
   }
 
-  private SqlTreeNode buildNode(String prefix, BeanPropertyAssoc<?> prop, BeanDescriptor<?> desc, List<SqlTreeNode> myList) {
-
-    OrmQueryProperties queryProps = queryDetail.getChunk(prefix, false);
-
-    SqlTreeProperties props = getBaseSelect(desc, queryProps);
+  private SqlTreeNode buildNode(String prefix, BeanPropertyAssoc<?> prop, BeanDescriptor<?> desc, List<SqlTreeNode> myList, SqlTreeProperties props) {
 
     if (prefix == null) {
       buildExtraJoins(desc, myList);

src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeNode.java

@@ -10,7 +10,7 @@ import com.avaje.ebeaninternal.server.deploy.DbSqlContext;
 import java.sql.SQLException;
 import java.util.List;
 
-public interface SqlTreeNode {
+interface SqlTreeNode {
 
   String COMMA = ", ";
 

src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeNodeBean.java

@@ -27,7 +27,7 @@ import java.util.Map;
 /**
  * Normal bean included in the query.
  */
-public class SqlTreeNodeBean implements SqlTreeNode {
+class SqlTreeNodeBean implements SqlTreeNode {
 
   private static final SqlTreeNode[] NO_CHILDREN = new SqlTreeNode[0];
 
@@ -83,7 +83,7 @@ public class SqlTreeNodeBean implements SqlTreeNode {
    */
   private boolean intersectionAsOfTableAlias;
 
-  private boolean aggregation;
+  private final boolean aggregation;
 
   /**
    * Construct for leaf node.
@@ -127,6 +127,7 @@ public class SqlTreeNodeBean implements SqlTreeNode {
 
     this.partialObject = props.isPartialObject();
     this.properties = props.getProps();
+    this.aggregation = props.isAggregation();
     this.children = myChildren == null ? NO_CHILDREN : myChildren.toArray(new SqlTreeNode[myChildren.size()]);
 
     pathMap = createPathMap(prefix, desc);
@@ -447,9 +448,6 @@ public class SqlTreeNodeBean implements SqlTreeNode {
 
     for (int i = 0; i < props.length; i++) {
       props[i].appendSelect(ctx, subQuery);
-      if (props[i].isAggregation()) {
-        aggregation = true;
-      }
     }
   }
 

src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeNodeExtraJoin.java

@@ -22,7 +22,7 @@ import java.util.List;
  * etc in this case we must add an extra join.
  * </p>
  */
-public class SqlTreeNodeExtraJoin implements SqlTreeNode {
+class SqlTreeNodeExtraJoin implements SqlTreeNode {
 
   private final BeanPropertyAssoc<?> assocBeanProperty;
 
@@ -34,7 +34,7 @@ public class SqlTreeNodeExtraJoin implements SqlTreeNode {
 
   private List<SqlTreeNodeExtraJoin> children;
 
-  public SqlTreeNodeExtraJoin(String prefix, BeanPropertyAssoc<?> assocBeanProperty, boolean pathContainsMany) {
+  SqlTreeNodeExtraJoin(String prefix, BeanPropertyAssoc<?> assocBeanProperty, boolean pathContainsMany) {
     this.prefix = prefix;
     this.assocBeanProperty = assocBeanProperty;
     this.pathContainsMany = pathContainsMany;

src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeNodeManyRoot.java

@@ -8,11 +8,11 @@ import com.avaje.ebeaninternal.server.deploy.DbSqlContext;
 import java.sql.SQLException;
 import java.util.List;
 
-public final class SqlTreeNodeManyRoot extends SqlTreeNodeBean {
+final class SqlTreeNodeManyRoot extends SqlTreeNodeBean {
 
   private final BeanPropertyAssocMany<?> manyProp;
 
-  public SqlTreeNodeManyRoot(String prefix, BeanPropertyAssocMany<?> prop, SqlTreeProperties props, List<SqlTreeNode> myList, boolean disableLazyLoad) {
+  SqlTreeNodeManyRoot(String prefix, BeanPropertyAssocMany<?> prop, SqlTreeProperties props, List<SqlTreeNode> myList, boolean disableLazyLoad) {
     super(prefix, prop, props, myList, disableLazyLoad);
     this.manyProp = prop;
   }

src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeNodeManyWhereJoin.java

@@ -17,7 +17,7 @@ import java.util.List;
 /**
  * Join to Many (or child of a many) to support where clause predicates on many properties.
  */
-public class SqlTreeNodeManyWhereJoin implements SqlTreeNode {
+class SqlTreeNodeManyWhereJoin implements SqlTreeNode {
 
   private final String parentPrefix;
 

src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeNodeRoot.java

@@ -11,7 +11,7 @@ import java.util.List;
 /**
  * Represents the root node of the Sql Tree.
  */
-public final class SqlTreeNodeRoot extends SqlTreeNodeBean {
+final class SqlTreeNodeRoot extends SqlTreeNodeBean {
 
   private final TableJoin includeJoin;
 

src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeProperties.java

@@ -4,6 +4,7 @@ import java.util.ArrayList;
 import java.util.LinkedHashSet;
 import java.util.List;
 
+import com.avaje.ebeaninternal.api.ManyWhereJoins;
 import com.avaje.ebeaninternal.server.deploy.BeanProperty;
 import com.avaje.ebeaninternal.server.deploy.TableJoin;
 
@@ -33,6 +34,8 @@ public class SqlTreeProperties {
 
   private boolean allProperties;
 
+  private boolean aggregation;
+
   public SqlTreeProperties() {
   }
 
@@ -80,4 +83,41 @@ public class SqlTreeProperties {
     this.allProperties = true;
   }
 
+  /**
+   * Check for an aggregation property and set manyWhereJoin as needed.
+   * <p>
+   * Return true if a Sql distinct is required.
+   * </p>
+   */
+  public boolean requireSqlDistinct(ManyWhereJoins manyWhereJoins) {
+    String joinProperty = aggregationJoin();
+    if (joinProperty != null) {
+      aggregation = true;
+      manyWhereJoins.addAggregationJoin(joinProperty);
+      return false;
+    } else{
+      return manyWhereJoins.requireSqlDistinct();
+    }
+  }
+
+  /**
+   * Return true if this contains an aggregation property.
+   */
+  public boolean isAggregation() {
+    return aggregation;
+  }
+
+  /**
+   * Return the property to join for aggregation.
+   */
+  private String aggregationJoin() {
+    if (!allProperties) {
+      for (BeanProperty beanProperty : propsList) {
+        if (beanProperty.isAggregation()) {
+          return beanProperty.getElPrefix();
+        }
+      }
+    }
+    return null;
+  }
 }

src/main/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQuery.java

@@ -373,9 +373,6 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
     if (havingExpressions != null) {
       havingExpressions.containsMany(beanDescriptor, manyWhereJoins);
     }
-    if (manyWhereJoins.requireSqlDistinct()) {
-      setSqlDistinct(true);
-    }
   }
 
   /**

src/test/java/com/avaje/tests/query/aggregation/TestAggregationCount.java

@@ -14,7 +14,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 public class TestAggregationCount extends BaseTestCase {
 
   @Test
-  public void test() {
+  public void testFull() {
 
     TEventOne one = new TEventOne("first");
     one.getLogs().add(new TEventMany("all", 1, 10));
@@ -28,8 +28,7 @@ public class TestAggregationCount extends BaseTestCase {
     two.getLogs().add(new TEventMany("alf", 30, 13));
     Ebean.save(two);
 
-
-    Query<TEventOne> query = Ebean.find(TEventOne.class)
+    Query<TEventOne> query2 = Ebean.find(TEventOne.class)
         .select("name, count, totalUnits, totalAmount")
         .where()
           .startsWith("logs.description", "a")
@@ -37,14 +36,14 @@ public class TestAggregationCount extends BaseTestCase {
           .ge("count", 1)
         .orderBy().asc("name");
 
-    List<TEventOne> list = query.findList();
+    List<TEventOne> list = query2.findList();
     for (TEventOne eventOne : list) {
       System.out.println(eventOne.getId() + " " + eventOne.getName() + " count:" + eventOne.getCount() + " units:" + eventOne.getTotalUnits() + " amount:" + eventOne.getTotalAmount());
     }
 
     assertThat(list).isNotEmpty();
 
-    String sql = query.getGeneratedSql();
+    String sql = query2.getGeneratedSql();
     assertThat(sql).contains("select t0.id c0, t0.name c1, count(u1.*) c2, sum(u1.units) c3, sum(u1.units * u1.amount) c4 from tevent_one t0");
     assertThat(sql).contains("from tevent_one t0 join tevent_many u1 on u1.event_id = t0.id ");
     assertThat(sql).contains("where u1.description like ? ");
@@ -52,4 +51,39 @@ public class TestAggregationCount extends BaseTestCase {
 
   }
 
+  @Test
+  public void testSelectOnly() {
+
+    Query<TEventOne> query0 = Ebean.find(TEventOne.class)
+        .select("name, count, totalUnits, totalAmount");
+
+    query0.findList();
+    assertThat(query0.getGeneratedSql()).contains("select t0.id c0, t0.name c1, count(u1.*) c2, sum(u1.units) c3, sum(u1.units * u1.amount) c4 from tevent_one t0");
+    assertThat(query0.getGeneratedSql()).contains("group by t0.id, t0.name");
+  }
+
+  @Test
+  public void testSelectWhere() {
+
+    Query<TEventOne> query0 = Ebean.find(TEventOne.class)
+        .select("name, count, totalUnits, totalAmount")
+        .where().gt("logs.description", "a").query();
+
+    query0.findList();
+    assertThat(query0.getGeneratedSql()).contains("select t0.id c0, t0.name c1, count(u1.*) c2, sum(u1.units) c3, sum(u1.units * u1.amount) c4 from tevent_one t0");
+    assertThat(query0.getGeneratedSql()).contains("group by t0.id, t0.name");
+  }
+
+  @Test
+  public void testSelectHavingOrderBy() {
+
+    Query<TEventOne> query1 = Ebean.find(TEventOne.class)
+        .select("name, count, totalUnits, totalAmount")
+        .having().ge("count", 1)
+        .orderBy().asc("name");
+
+    query1.findList();
+    assertThat(query1.getGeneratedSql()).contains("having count(u1.*) >= ?  order by t0.name");
+  }
+
 }