Improve count distinct serialization #587

querydsl · Dec 9, 2013 · 84e78e4 · 84e78e4
1 parent cc87245
commit 84e78e4
Show file tree

Hide file tree

Showing 8 changed files with 228 additions and 9 deletions.
diff --git a/querydsl-sql/src/main/java/com/mysema/query/sql/H2Templates.java b/querydsl-sql/src/main/java/com/mysema/query/sql/H2Templates.java
@@ -44,6 +44,7 @@ public H2Templates(char escape, boolean quote) {
         super("\"", escape, quote);
         setNativeMerge(true);
         setLimitRequired(true);
+        setCountDistinctMultipleColumns(true);
         add(Ops.MathOps.ROUND, "round({0},0)");
         add(Ops.TRIM, "trim(both from {0})");
 

diff --git a/querydsl-sql/src/main/java/com/mysema/query/sql/PostgresTemplates.java b/querydsl-sql/src/main/java/com/mysema/query/sql/PostgresTemplates.java
@@ -45,6 +45,7 @@ public PostgresTemplates(boolean quote) {
     public PostgresTemplates(char escape, boolean quote) {
         super("\"", escape, quote);
         setDummyTable(null);
+        setCountDistinctMultipleColumns(true);
         // type mappings
         addClass2TypeMappings("numeric(3,0)", Byte.class);
         addClass2TypeMappings("double precision", Double.class);

diff --git a/querydsl-sql/src/main/java/com/mysema/query/sql/SQLSerializer.java b/querydsl-sql/src/main/java/com/mysema/query/sql/SQLSerializer.java
@@ -166,6 +166,7 @@ void serializeForQuery(QueryMetadata metadata, boolean forCountRow) {
         final List<OrderSpecifier<?>> orderBy = metadata.getOrderBy();
         final Set<QueryFlag> flags = metadata.getFlags();
         final boolean hasFlags = !flags.isEmpty();
+        String suffix = null;
 
         List<Expression<?>> sqlSelect;
         if (select.size() == 1) {
@@ -231,13 +232,29 @@ void serializeForQuery(QueryMetadata metadata, boolean forCountRow) {
             if (!metadata.isDistinct()) {
                 append(templates.getCountStar());
             } else {
-                append(templates.getDistinctCountStart());
+                List<? extends Expression<?>> columns;
                 if (sqlSelect.isEmpty()) {
-                    handle(COMMA, getIdentifierColumns(joins));
+                    columns = getIdentifierColumns(joins);
                 } else {
-                    handle(COMMA, sqlSelect);
+                    columns = sqlSelect;
+                }
+                if (columns.size() == 1) {
+                    append(templates.getDistinctCountStart());
+                    handle(columns.get(0));
+                    append(templates.getDistinctCountEnd());
+                } else if (templates.isCountDistinctMultipleColumns()) {
+                    append(templates.getDistinctCountStart());
+                    append("(").handle(COMMA, columns).append(")");
+                    append(templates.getDistinctCountEnd());
+                } else {
+                    // select count(*) from (select distinct ...)
+                    append(templates.getCountStar());
+                    append(templates.getFrom());
+                    append("(");
+                    append(templates.getSelectDistinct());
+                    handle(COMMA, columns);
+                    suffix = ") internal";
                 }
-                append(templates.getDistinctCountEnd());
             }
 
         } else if (!sqlSelect.isEmpty()) {
@@ -351,6 +368,10 @@ void serializeForQuery(QueryMetadata metadata, boolean forCountRow) {
             templates.serializeModifiers(metadata, this);
         }
 
+        if (suffix != null) {
+            append(suffix);
+        }
+
         // reset stage
         stage = oldStage;
 

diff --git a/querydsl-sql/src/main/java/com/mysema/query/sql/SQLTemplates.java b/querydsl-sql/src/main/java/com/mysema/query/sql/SQLTemplates.java
@@ -183,6 +183,8 @@ public SQLTemplates build() {
 
     private boolean limitRequired = false;
 
+    private boolean countDistinctMultipleColumns = false;
+
     protected SQLTemplates(String quoteStr, char escape, boolean useQuotes) {
         super(escape);
         this.quoteStr = quoteStr;
@@ -479,10 +481,6 @@ public final String getCreateTable() {
         return createTable;
     }
 
-    public final boolean isPrintSchema() {
-        return printSchema;
-    }
-
     public final String getWith() {
         return with;
     }
@@ -491,6 +489,14 @@ public final String getWithRecursive() {
         return withRecursive;
     }
 
+    public final boolean isCountDistinctMultipleColumns() {
+        return countDistinctMultipleColumns;
+    }
+
+    public final boolean isPrintSchema() {
+        return printSchema;
+    }
+
     public final boolean isParameterMetadataAvailable() {
         return parameterMetadataAvailable;
     }
@@ -773,6 +779,8 @@ protected void setLimitRequired(boolean limitRequired) {
         this.limitRequired = limitRequired;
     }
 
-
+    protected void setCountDistinctMultipleColumns(boolean countDistinctMultipleColumns) {
+        this.countDistinctMultipleColumns = countDistinctMultipleColumns;
+    }
 
 }
diff --git a/querydsl-sql/src/test/java/com/mysema/query/SelectBase.java b/querydsl-sql/src/test/java/com/mysema/query/SelectBase.java
@@ -62,6 +62,7 @@
 import com.mysema.query.sql.domain.Employee;
 import com.mysema.query.sql.domain.IdName;
 import com.mysema.query.sql.domain.QEmployee;
+import com.mysema.query.sql.domain.QEmployeeNoPK;
 import com.mysema.query.sql.domain.QIdName;
 import com.mysema.query.support.Expressions;
 import com.mysema.query.types.ArrayConstructorExpression;
@@ -371,6 +372,22 @@ private double coth(double x) {
         return Math.cosh(x) / Math.sinh(x);
     }
 
+    @Test
+    public void Count_With_PK() {
+        query().from(employee).count();
+    }
+
+    @Test
+    public void Count_Without_PK() {
+        query().from(QEmployeeNoPK.employee).count();
+    }
+
+    @Test
+    @Ignore // FIXME
+    public void Count2() {
+        query().from(employee).singleResult(employee.count());
+    }
+
     @Test
     @SkipForQuoted
     @ExcludeIn(ORACLE)
@@ -389,6 +406,22 @@ public void Count_All_Oracle() {
         query().from(employee).uniqueResult(Wildcard.count.as(rowCount));
     }
 
+    @Test
+    public void Count_Distinct_With_PK() {
+        query().from(employee).distinct().count();
+    }
+
+    @Test
+    public void Count_Distinct_Without_PK() {
+        query().from(QEmployeeNoPK.employee).distinct().count();
+    }
+
+    @Test
+    @Ignore // FIXME
+    public void Count_Distinct2() {
+        query().from(employee).singleResult(employee.countDistinct());
+    }
+
     @Test
     public void Custom_Projection() {
         List<Projection> tuples = query().from(employee).list(

diff --git a/querydsl-sql/src/test/java/com/mysema/query/sql/SQLSerializerTest.java b/querydsl-sql/src/test/java/com/mysema/query/sql/SQLSerializerTest.java
@@ -26,6 +26,7 @@
 import com.mysema.query.Survey;
 import com.mysema.query.Tuple;
 import com.mysema.query.sql.domain.QEmployee;
+import com.mysema.query.sql.domain.QEmployeeNoPK;
 import com.mysema.query.sql.domain.QSurvey;
 import com.mysema.query.support.Expressions;
 import com.mysema.query.types.Expression;
@@ -46,6 +47,33 @@ public void Count() {
         assertEquals("count(EMPLOYEE.ID) + count(distinct EMPLOYEE.ID)", serializer.toString());
     }
 
+    @Test
+    public void CountDistinct() {
+        SQLSerializer serializer = new SQLSerializer(Configuration.DEFAULT);
+        SQLSubQuery query = new SQLSubQuery();
+        query.from(QEmployeeNoPK.employee);
+        query.distinct();
+        serializer.serializeForQuery(query.queryMixin.getMetadata(), true);
+        assertEquals("select count(*)\n" +
+        "from (select distinct EMPLOYEE.ID, EMPLOYEE.FIRSTNAME, EMPLOYEE.LASTNAME, EMPLOYEE.SALARY, " +
+            "EMPLOYEE.DATEFIELD, EMPLOYEE.TIMEFIELD, EMPLOYEE.SUPERIOR_ID\n" +
+         "from EMPLOYEE EMPLOYEE) internal", serializer.toString());
+    }
+
+    @Test
+    public void CountDistinct_PostgreSQL() {
+        Configuration postgres = new Configuration(new PostgresTemplates());
+        SQLSerializer serializer = new SQLSerializer(postgres);
+        SQLSubQuery query = new SQLSubQuery();
+        query.from(QEmployeeNoPK.employee);
+        query.distinct();
+        serializer.serializeForQuery(query.queryMixin.getMetadata(), true);
+        assertEquals("select count(" +
+            "distinct (EMPLOYEE.ID, EMPLOYEE.FIRSTNAME, EMPLOYEE.LASTNAME, EMPLOYEE.SALARY, " +
+            "EMPLOYEE.DATEFIELD, EMPLOYEE.TIMEFIELD, EMPLOYEE.SUPERIOR_ID))\n" +
+            "from EMPLOYEE EMPLOYEE", serializer.toString());
+    }
+
     @Test
     public void Some() {
         //select some((e.FIRSTNAME is not null)) from EMPLOYEE

diff --git a/querydsl-sql/src/test/java/com/mysema/query/sql/domain/QEmployeeNoPK.java b/querydsl-sql/src/test/java/com/mysema/query/sql/domain/QEmployeeNoPK.java
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2011, Mysema Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.mysema.query.sql.domain;
+
+import java.math.BigDecimal;
+
+import com.mysema.query.sql.ColumnMetadata;
+import com.mysema.query.sql.ForeignKey;
+import com.mysema.query.sql.RelationalPathBase;
+import com.mysema.query.types.PathMetadata;
+import com.mysema.query.types.PathMetadataFactory;
+import com.mysema.query.types.path.DatePath;
+import com.mysema.query.types.path.NumberPath;
+import com.mysema.query.types.path.StringPath;
+import com.mysema.query.types.path.TimePath;
+
+//@Schema("PUBLIC")
+//@Table("EMPLOYEE")
+public class QEmployeeNoPK extends RelationalPathBase<Employee> {
+
+    private static final long serialVersionUID = 1394463749655231079L;
+
+    public static final QEmployeeNoPK employee = new QEmployeeNoPK("EMPLOYEE");
+
+    public final NumberPath<Integer> id = createNumber("id", Integer.class);
+
+    public final StringPath firstname = createString("firstname");
+
+    public final StringPath lastname = createString("lastname");
+
+    public final NumberPath<BigDecimal> salary = createNumber("salary", BigDecimal.class);
+
+    public final DatePath<java.sql.Date> datefield = createDate("datefield", java.sql.Date.class);
+
+    public final TimePath<java.sql.Time> timefield = createTime("timefield", java.sql.Time.class);
+
+    public final NumberPath<Integer> superiorId = createNumber("superiorId", Integer.class);
+
+    public final ForeignKey<Employee> superiorIdKey = createForeignKey(superiorId, "ID");
+
+    public final ForeignKey<Employee> _superiorIdKey = createInvForeignKey(id, "SUPERIOR_ID");
+
+    public QEmployeeNoPK(String path) {
+        super(Employee.class, PathMetadataFactory.forVariable(path), "PUBLIC", "EMPLOYEE");
+        addMetadata();
+    }
+
+    public QEmployeeNoPK(PathMetadata<?> metadata) {
+        super(Employee.class, metadata, "PUBLIC", "EMPLOYEE");
+        addMetadata();
+    }
+
+    protected void addMetadata() {
+        addMetadata(id, ColumnMetadata.named("ID"));
+        addMetadata(firstname, ColumnMetadata.named("FIRSTNAME"));
+        addMetadata(lastname, ColumnMetadata.named("LASTNAME"));
+        addMetadata(salary, ColumnMetadata.named("SALARY"));
+        addMetadata(datefield, ColumnMetadata.named("DATEFIELD"));
+        addMetadata(timefield, ColumnMetadata.named("TIMEFIELD"));
+        addMetadata(superiorId, ColumnMetadata.named("SUPERIOR_ID"));
+    }
+
+}
diff --git a/querydsl-sql/src/test/java/com/mysema/query/sql/domain/QSurveyNoPK.java b/querydsl-sql/src/test/java/com/mysema/query/sql/domain/QSurveyNoPK.java
@@ -0,0 +1,53 @@
+/*
+ * Copyright 2011, Mysema Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.mysema.query.sql.domain;
+
+import com.mysema.query.sql.ColumnMetadata;
+import com.mysema.query.sql.RelationalPathBase;
+import com.mysema.query.types.PathMetadata;
+import com.mysema.query.types.PathMetadataFactory;
+import com.mysema.query.types.path.NumberPath;
+import com.mysema.query.types.path.StringPath;
+
+//@Schema("PUBLIC")
+//@Table("SURVEY")
+public class QSurveyNoPK extends RelationalPathBase<QSurveyNoPK>{
+
+    private static final long serialVersionUID = -7427577079709192842L;
+
+    public static final QSurveyNoPK survey = new QSurveyNoPK("SURVEY");
+
+    public final StringPath name = createString("name");
+
+    public final StringPath name2 = createString("name2");
+
+    public final NumberPath<Integer> id = createNumber("id", Integer.class);
+
+    public QSurveyNoPK(String path) {
+        super(QSurveyNoPK.class, PathMetadataFactory.forVariable(path), "PUBLIC", "SURVEY");
+        addMetadata();
+    }
+
+    public QSurveyNoPK(PathMetadata<?> metadata) {
+        super(QSurveyNoPK.class, metadata, "PUBLIC", "SURVEY");
+        addMetadata();
+    }
+
+    protected void addMetadata() {
+        addMetadata(name, ColumnMetadata.named("NAME"));
+        addMetadata(name2, ColumnMetadata.named("NAME2"));
+        addMetadata(id, ColumnMetadata.named("ID"));
+    }
+
+}