Bug correction for the group by clause

Author: nt

src/main/java/org/mybatis/dynamic/sql/select/GroupByModel.java

@@ -1,5 +1,5 @@
 /**
- *    Copyright 2016-2017 the original author or authors.
+ *    Copyright 2016-2018 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -22,6 +22,7 @@
 import java.util.stream.Stream;
 
 import org.mybatis.dynamic.sql.BasicColumn;
+import org.mybatis.dynamic.sql.SqlColumn;
 
 public class GroupByModel {
     private List<BasicColumn> columns = new ArrayList<>();

src/main/java/org/mybatis/dynamic/sql/select/QueryExpressionDSL.java

@@ -1,5 +1,5 @@
 /**
- *    Copyright 2016-2017 the original author or authors.
+ *    Copyright 2016-2018 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -25,6 +25,7 @@
 import org.mybatis.dynamic.sql.BasicColumn;
 import org.mybatis.dynamic.sql.BindableColumn;
 import org.mybatis.dynamic.sql.SortSpecification;
+import org.mybatis.dynamic.sql.SqlColumn;
 import org.mybatis.dynamic.sql.SqlCriterion;
 import org.mybatis.dynamic.sql.SqlTable;
 import org.mybatis.dynamic.sql.VisitableCondition;
@@ -217,8 +218,9 @@ public SelectDSL<R> orderBy(SortSpecification...columns) {
         }
 
         public GroupByFinisher groupBy(BasicColumn...columns) {
-            groupByModel = GroupByModel.of(columns);
+            whereModel = buildWhereModel();
             selectDSL.addQueryExpression(buildModel());
+            selectDSL.setGroupByModel(GroupByModel.of(columns));
             return new GroupByFinisher();
         }
 

src/main/java/org/mybatis/dynamic/sql/select/SelectDSL.java

@@ -1,5 +1,5 @@
 /**
- *    Copyright 2016-2017 the original author or authors.
+ *    Copyright 2016-2018 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -37,6 +37,7 @@ public class SelectDSL<R> {
     private Function<SelectModel, R> adapterFunction;
     private List<QueryExpressionModel> queryExpressions = new ArrayList<>();    
     private OrderByModel orderByModel;
+    private GroupByModel groupByModel;
 
     private SelectDSL(Function<SelectModel, R> adapterFunction) {
         this.adapterFunction = Objects.requireNonNull(adapterFunction);
@@ -96,9 +97,14 @@ void setOrderByModel(OrderByModel orderByModel) {
         this.orderByModel = orderByModel;
     }
 
+    void setGroupByModel(GroupByModel groupByModel) {
+        this.groupByModel = groupByModel;
+    }
+    
     public R build() {
         SelectModel selectModel = SelectModel.withQueryExpressions(queryExpressions)
                 .withOrderByModel(orderByModel)
+                .withGroupByModel(groupByModel)
                 .build();
         return adapterFunction.apply(selectModel);
     }

src/main/java/org/mybatis/dynamic/sql/select/SelectModel.java

@@ -29,10 +29,12 @@
 public class SelectModel {
     private List<QueryExpressionModel> queryExpressions;
     private OrderByModel orderByModel;
+    private GroupByModel groupByModel;
 
     private SelectModel(Builder builder) {
         queryExpressions = Objects.requireNonNull(builder.queryExpressions);
         orderByModel = builder.orderByModel;
+        groupByModel = builder.groupByModel;
     }
 
     public <R> Stream<R> mapQueryExpressions(Function<QueryExpressionModel, R> mapper) {
@@ -43,6 +45,10 @@ public Optional<OrderByModel> orderByModel() {
         return Optional.ofNullable(orderByModel);
     }
 
+    public Optional<GroupByModel> groupByModel() {
+        return Optional.ofNullable(groupByModel);
+    }
+    
     public SelectStatementProvider render(RenderingStrategy renderingStrategy) {
         return SelectRenderer.withSelectModel(this)
                 .withRenderingStrategy(renderingStrategy)
@@ -57,6 +63,7 @@ public static Builder withQueryExpressions(List<QueryExpressionModel> queryExpre
     public static class Builder {
         private List<QueryExpressionModel> queryExpressions = new ArrayList<>();
         private OrderByModel orderByModel;
+        private GroupByModel groupByModel;
 
         public Builder withQueryExpressions(List<QueryExpressionModel> queryExpressions) {
             this.queryExpressions.addAll(queryExpressions);
@@ -68,6 +75,11 @@ public Builder withOrderByModel(OrderByModel orderByModel) {
             return this;
         }
 
+        public Builder withGroupByModel(GroupByModel groupByModel) {
+            this.groupByModel = groupByModel;
+            return this;
+        }
+        
         public SelectModel build() {
             return new SelectModel(this);
         }

src/main/java/org/mybatis/dynamic/sql/select/render/SelectRenderer.java

@@ -1,5 +1,5 @@
 /**
- *    Copyright 2016-2017 the original author or authors.
+ *    Copyright 2016-2018 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -19,8 +19,12 @@
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import org.mybatis.dynamic.sql.BasicColumn;
+import org.mybatis.dynamic.sql.BindableColumn;
 import org.mybatis.dynamic.sql.SortSpecification;
+import org.mybatis.dynamic.sql.SqlColumn;
 import org.mybatis.dynamic.sql.render.RenderingStrategy;
+import org.mybatis.dynamic.sql.select.GroupByModel;
 import org.mybatis.dynamic.sql.select.OrderByModel;
 import org.mybatis.dynamic.sql.select.QueryExpressionModel;
 import org.mybatis.dynamic.sql.select.SelectModel;
@@ -45,6 +49,7 @@ public SelectStatementProvider render() {
         return SelectStatementProvider.withQueryExpression(collector.queryExpression())
                 .withParameters(collector.parameters())
                 .withOrderByClause(selectModel.orderByModel().map(this::renderOrderBy))
+                .withGroupByClause(selectModel.groupByModel().map(this::renderGroupBy))
                 .build();
     }
 
@@ -61,6 +66,10 @@ private String renderOrderBy(OrderByModel orderByModel) {
                 .collect(CustomCollectors.joining(", ", "order by ", "")); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
     }
 
+    private String renderGroupBy(GroupByModel groupByModel) {
+        return groupByModel.mapColumns(this::groupByPhrase).collect(CustomCollectors.joining(", ", "group by ", "")); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+    }
+    
     private String orderByPhrase(SortSpecification column) {
         String phrase = column.aliasOrName();
         if (column.isDescending()) {
@@ -69,6 +78,10 @@ private String orderByPhrase(SortSpecification column) {
         return phrase;
     }
 
+    private String groupByPhrase(BasicColumn column) {
+        return ((SqlColumn) column).aliasOrName();
+    }
+    
     public static Builder withSelectModel(SelectModel selectModel) {
         return new Builder().withSelectModel(selectModel);
     }

src/main/java/org/mybatis/dynamic/sql/select/render/SelectStatementProvider.java

@@ -1,5 +1,5 @@
 /**
- *    Copyright 2016-2017 the original author or authors.
+ *    Copyright 2016-2018 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -28,10 +28,12 @@ public class SelectStatementProvider {
     private String queryExpression;
     private Map<String, Object> parameters;
     private Optional<String> orderByClause;
+    private Optional<String> groupByClause;
 
     private SelectStatementProvider(Builder builder) {
         queryExpression = Objects.requireNonNull(builder.queryExpression);
         orderByClause = Objects.requireNonNull(builder.orderByClause);
+        groupByClause = Objects.requireNonNull(builder.groupByClause);
         parameters = Collections.unmodifiableMap(Objects.requireNonNull(builder.parameters));
     }
 
@@ -40,7 +42,7 @@ public Map<String, Object> getParameters() {
     }
 
     public String getSelectStatement() {
-        return queryExpression + StringUtilities.spaceBefore(orderByClause);
+        return queryExpression + StringUtilities.spaceBefore(orderByClause) + StringUtilities.spaceBefore(groupByClause);
     }
 
     public static Builder withQueryExpression(String queryExpression) {
@@ -50,6 +52,7 @@ public static Builder withQueryExpression(String queryExpression) {
     public static class Builder {
         private String queryExpression;
         private Optional<String> orderByClause = Optional.empty();
+        private Optional<String> groupByClause = Optional.empty();
         private Map<String, Object> parameters = new HashMap<>();
 
         public Builder withQueryExpression(String queryExpression) {
@@ -62,6 +65,11 @@ public Builder withOrderByClause(Optional<String> orderByClause) {
             return this;
         }
 
+        public Builder withGroupByClause(Optional<String> groupByClause) {
+            this.groupByClause = groupByClause;
+            return this;
+        }
+        
         public Builder withParameters(Map<String, Object> parameters) {
             this.parameters.putAll(parameters);
             return this;

src/test/java/org/mybatis/dynamic/sql/select/SelectStatementTest.java

@@ -1,5 +1,5 @@
 /**
- *    Copyright 2016-2017 the original author or authors.
+ *    Copyright 2016-2018 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -252,7 +252,7 @@ public void testGroupBySingleColumn() {
             String expectedFullStatement = "select a.column1 as A_COLUMN1, a.column2 "
                     + "from foo a "
                     + "where a.column1 = #{parameters.p1,jdbcType=DATE} "
-                    + "group by column2 ";
+                    + "group by column2";
 
             softly.assertThat(selectStatement.getSelectStatement()).isEqualTo(expectedFullStatement);