Fixed test problems. Fixed #296

core/api/src/main/java/com/blazebit/persistence/spi/DbmsDialect.java

@@ -153,6 +153,14 @@ public interface DbmsDialect {
      */
     public boolean supportsComplexGroupBy();
 
+    /**
+     * Returns true if the dbms supports matching non-trivial expressions that appear in the group by clause with usages in the having clause.
+     *
+     * @return Whether expressions from the group by clause are matched and reused in the having clause by the dbms
+     * @since 1.2.0
+     */
+    public boolean supportsGroupByExpressionInHavingMatching();
+
     /**
      * Returns true if the dbms supports complex expressions like subqueries in the join on clause, false otherwise.
      *

core/impl/src/main/java/com/blazebit/persistence/impl/GroupByExpressionGatheringVisitor.java

@@ -44,6 +44,7 @@
 import com.blazebit.persistence.impl.expression.TimestampLiteral;
 import com.blazebit.persistence.impl.expression.TreatExpression;
 import com.blazebit.persistence.impl.expression.TrimExpression;
+import com.blazebit.persistence.impl.expression.VisitorAdapter;
 import com.blazebit.persistence.impl.expression.WhenClauseExpression;
 import com.blazebit.persistence.impl.predicate.BetweenPredicate;
 import com.blazebit.persistence.impl.predicate.BinaryExpressionPredicate;
@@ -82,6 +83,39 @@ private boolean setCollect(boolean collect) {
 
     public Set<Expression> extractGroupByExpressions(Expression expression) {
         clear();
+        if (!dbmsDialect.supportsGroupByExpressionInHavingMatching()) {
+            expression.accept(new VisitorAdapter() {
+                @Override
+                public void visit(FunctionExpression expression) {
+                    // Skip aggregate expressions
+                    if (expression instanceof AggregateExpression || (treatSizeAsAggregate && com.blazebit.persistence.impl.util.ExpressionUtils.isSizeFunction(expression))) {
+                        return;
+                    }
+                    super.visit(expression);
+                }
+
+                @Override
+                public void visit(SubqueryExpression expression) {
+                    GroupByExpressionGatheringVisitor.this.visit(expression);
+                }
+
+                @Override
+                public void visit(PathExpression expression) {
+                    expressions.add(expression);
+                }
+
+                @Override
+                public void visit(TreatExpression expression) {
+                    expressions.add(expression);
+                }
+
+                @Override
+                public void visit(PropertyExpression expression) {
+                    expressions.add(expression);
+                }
+            });
+            return expressions;
+        }
         // When having a predicate at the top level, we have to collect
         collect = expression instanceof Predicate;
         if (expression.accept(this)) {
@@ -163,6 +197,26 @@ public Boolean visit(FunctionExpression expression) {
             return true;
         }
 
+        // don't add non-deterministic functions
+        if (collect) {
+            String functionName;
+            if (ExpressionUtils.isFunctionFunctionExpression(expression)) {
+                functionName = ((StringLiteral) expression.getExpressions().get(0)).getValue();
+            } else {
+                functionName = expression.getFunctionName();
+            }
+
+            // Currently we only consider these functions as non-deterministic, but we might want to make this configurable
+            switch (functionName.toUpperCase()) {
+                case "CURRENT_DATE":
+                case "CURRENT_TIME":
+                case "CURRENT_TIMESTAMP":
+                    return true;
+                default:
+                    break;
+            }
+        }
+
         boolean oldCollect = setCollect(false);
         List<Expression> expressions = expression.getExpressions();
         int size = expressions.size();

core/impl/src/main/java/com/blazebit/persistence/impl/dialect/DefaultDbmsDialect.java

@@ -191,8 +191,10 @@ protected String[] appendSetOperands(StringBuilder sqlSb, SetOperationType setTy
         }
 
         for (String operand : operands) {
+            boolean wasFirst = false;
             if (first) {
                 first = false;
+                wasFirst = true;
                 if (emulate) {
                     if (aliases == null) {
                         int selectIndex = SqlUtils.indexOfSelect(operand);
@@ -240,7 +242,11 @@ protected String[] appendSetOperands(StringBuilder sqlSb, SetOperationType setTy
                 if (addWrapper) {
                     sqlSb.append("select * from (");
                 }
-                sqlSb.append(operand);
+                if ((addWrapper || wasFirst) && operand.charAt(0) == '(') {
+                    sqlSb.append(operand, 1, operand.length() - 1);
+                } else {
+                    sqlSb.append(operand);
+                }
                 if (addWrapper) {
                     sqlSb.append(')');
                 }
@@ -375,6 +381,11 @@ public boolean supportsComplexGroupBy() {
         return true;
     }
 
+    @Override
+    public boolean supportsGroupByExpressionInHavingMatching() {
+        return true;
+    }
+
     @Override
     public boolean supportsComplexJoinOn() {
         return true;

core/impl/src/main/java/com/blazebit/persistence/impl/dialect/MySQLDbmsDialect.java

@@ -67,6 +67,12 @@ public boolean supportsExcept(boolean all) {
         return false;
     }
 
+    @Override
+    public boolean supportsGroupByExpressionInHavingMatching() {
+        // MySQL re-evaluates all expressions in the having clause which is why it needs access to all column values
+        return false;
+    }
+
     @Override
     public DbmsLimitHandler createLimitHandler() {
         return new MySQLDbmsLimitHandler();

core/testsuite/src/main/java/com/blazebit/persistence/testsuite/AbstractCoreTest.java

@@ -20,6 +20,7 @@
 
 import com.blazebit.lang.StringUtils;
 import com.blazebit.persistence.spi.CriteriaBuilderConfiguration;
+import com.blazebit.persistence.spi.DbmsDialect;
 import com.blazebit.persistence.spi.EntityManagerFactoryIntegrator;
 import com.blazebit.persistence.spi.JpaProvider;
 import com.blazebit.persistence.spi.JpaProviderFactory;
@@ -157,6 +158,15 @@ protected String groupBy(String... groupBys) {
         distinctGroupBys.addAll(Arrays.asList(groupBys));
         return StringUtils.join(", ", distinctGroupBys);
     }
+
+    protected String groupByPathExpressions(String groupByExpression, String... pathExpressions) {
+        if (cbf.getService(DbmsDialect.class).supportsGroupByExpressionInHavingMatching()) {
+            return groupByExpression;
+        }
+        Set<String> distinctGroupBys = new LinkedHashSet<String>();
+        distinctGroupBys.addAll(Arrays.asList(pathExpressions));
+        return StringUtils.join(", ", distinctGroupBys);
+    }
 
     protected String countStar() {
         if (jpaProvider.supportsCountStar()) {

core/testsuite/src/test/java/com/blazebit/persistence/testsuite/HavingTest.java

@@ -53,8 +53,7 @@ public void testHavingPropertyExpression() {
         criteria.groupBy("d.owner")
             .having("d.age + 1").gt(0L);
 
-        // TODO: Apparently MySQL requires "d.age" to appear in the group by instead of "d.age + 1"
-        assertEquals("SELECT COUNT(d.id) FROM Document d JOIN d.owner owner_1 GROUP BY owner_1, d.age + 1 HAVING d.age + 1 > :param_0", criteria.getQueryString());
+        assertEquals("SELECT COUNT(d.id) FROM Document d JOIN d.owner owner_1 GROUP BY owner_1, " + groupByPathExpressions("d.age + 1", "d.age") + " HAVING d.age + 1 > :param_0", criteria.getQueryString());
         criteria.getResultList();
     }
 
@@ -479,7 +478,9 @@ public void testHavingOrMultipleSubqueryWithSurroundingExpression() {
     public void testHavingCase1() {
         CriteriaBuilder<Long> criteria = cbf.create(em, Long.class).from(Document.class, "d").select("COUNT(versions.id)");
         criteria.groupBy("d.id").havingCase().when("d.id").geExpression("d.age").thenExpression("2").otherwiseExpression("1").eqExpression("d.idx");
-        String expected = "SELECT COUNT(versions_1.id) FROM Document d LEFT JOIN d.versions versions_1 GROUP BY d.id, CASE WHEN d.id >= d.age THEN 2 ELSE 1 END, d.idx HAVING CASE WHEN d.id >= d.age THEN 2 ELSE 1 END = d.idx";
+        String expected = "SELECT COUNT(versions_1.id) FROM Document d LEFT JOIN d.versions versions_1 " +
+                "GROUP BY d.id, " + groupByPathExpressions("CASE WHEN d.id >= d.age THEN 2 ELSE 1 END", "d.age") + ", d.idx " +
+                "HAVING CASE WHEN d.id >= d.age THEN 2 ELSE 1 END = d.idx";
         assertEquals(expected, criteria.getQueryString());
         criteria.getResultList(); 
     }
@@ -515,7 +516,9 @@ public void testHavingSimpleCaseBuilderNotEnded() {
     public void testHavingSimpleCase() {
         CriteriaBuilder<Long> criteria = cbf.create(em, Long.class).from(Document.class, "d").select("COUNT(versions.id)");
         criteria.groupBy("d.id").havingSimpleCase("d.id").when("1", "d.age").otherwise("d.idx").eqExpression("d.idx");
-        String expected = "SELECT COUNT(versions_1.id) FROM Document d LEFT JOIN d.versions versions_1 GROUP BY d.id, CASE d.id WHEN 1 THEN d.age ELSE d.idx END, d.idx HAVING CASE d.id WHEN 1 THEN d.age ELSE d.idx END = d.idx";
+        String expected = "SELECT COUNT(versions_1.id) FROM Document d LEFT JOIN d.versions versions_1 " +
+                "GROUP BY d.id, " + groupByPathExpressions("CASE d.id WHEN 1 THEN d.age ELSE d.idx END", "d.age") + ", d.idx " +
+                "HAVING CASE d.id WHEN 1 THEN d.age ELSE d.idx END = d.idx";
         assertEquals(expected, criteria.getQueryString());
         criteria.getResultList(); 
     }
@@ -526,7 +529,9 @@ public void testHavingAndCase() {
         criteria.groupBy("d.id").havingOr().havingAnd().havingCase()
                 .whenAnd().and("d.id").eqExpression("d.age").and("d.age").ltExpression("4").thenExpression("2")
                 .when("d.id").eqExpression("4").thenExpression("4").otherwiseExpression("3").eqExpression("2").endAnd().endOr();
-        String expected = "SELECT COUNT(versions_1.id) FROM Document d LEFT JOIN d.versions versions_1 GROUP BY d.id, CASE WHEN d.id = d.age AND d.age < 4 THEN 2 WHEN d.id = 4 THEN 4 ELSE 3 END HAVING CASE WHEN d.id = d.age AND d.age < 4 THEN 2 WHEN d.id = 4 THEN 4 ELSE 3 END = 2";
+        String expected = "SELECT COUNT(versions_1.id) FROM Document d LEFT JOIN d.versions versions_1 " +
+                "GROUP BY d.id, " + groupByPathExpressions("CASE WHEN d.id = d.age AND d.age < 4 THEN 2 WHEN d.id = 4 THEN 4 ELSE 3 END", "d.age") + " " +
+                "HAVING CASE WHEN d.id = d.age AND d.age < 4 THEN 2 WHEN d.id = 4 THEN 4 ELSE 3 END = 2";
         assertEquals(expected, criteria.getQueryString());
         criteria.getResultList(); 
     }
@@ -537,7 +542,9 @@ public void testHavingAndSimpleCase() {
         criteria.groupBy("d.id").havingOr().havingAnd().havingSimpleCase("d.id")
                 .when("d.age", "2")
                 .when("4", "4").otherwise("3").eqExpression("2").endAnd().endOr();
-        String expected = "SELECT COUNT(versions_1.id) FROM Document d LEFT JOIN d.versions versions_1 GROUP BY d.id, CASE d.id WHEN d.age THEN 2 WHEN 4 THEN 4 ELSE 3 END HAVING CASE d.id WHEN d.age THEN 2 WHEN 4 THEN 4 ELSE 3 END = 2";
+        String expected = "SELECT COUNT(versions_1.id) FROM Document d LEFT JOIN d.versions versions_1 " +
+                "GROUP BY d.id, " + groupByPathExpressions("CASE d.id WHEN d.age THEN 2 WHEN 4 THEN 4 ELSE 3 END", "d.age") + " " +
+                "HAVING CASE d.id WHEN d.age THEN 2 WHEN 4 THEN 4 ELSE 3 END = 2";
         assertEquals(expected, criteria.getQueryString());
         criteria.getResultList(); 
     }
@@ -548,7 +555,9 @@ public void testHavingOrCase() {
         criteria.groupBy("d.id").havingOr().havingCase()
                 .whenAnd().and("d.id").eqExpression("d.age").and("d.age").ltExpression("4").thenExpression("2")
                 .when("d.id").eqExpression("4").thenExpression("4").otherwiseExpression("3").eqExpression("2").endOr();
-        String expected = "SELECT COUNT(versions_1.id) FROM Document d LEFT JOIN d.versions versions_1 GROUP BY d.id, CASE WHEN d.id = d.age AND d.age < 4 THEN 2 WHEN d.id = 4 THEN 4 ELSE 3 END HAVING CASE WHEN d.id = d.age AND d.age < 4 THEN 2 WHEN d.id = 4 THEN 4 ELSE 3 END = 2";
+        String expected = "SELECT COUNT(versions_1.id) FROM Document d LEFT JOIN d.versions versions_1 " +
+                "GROUP BY d.id, " + groupByPathExpressions("CASE WHEN d.id = d.age AND d.age < 4 THEN 2 WHEN d.id = 4 THEN 4 ELSE 3 END", "d.age") + " " +
+                "HAVING CASE WHEN d.id = d.age AND d.age < 4 THEN 2 WHEN d.id = 4 THEN 4 ELSE 3 END = 2";
         assertEquals(expected, criteria.getQueryString());
         criteria.getResultList(); 
     }
@@ -559,7 +568,9 @@ public void testHavingOrSimpleCase() {
         criteria.groupBy("d.id").havingOr().havingSimpleCase("d.id")
                 .when("d.age", "2")
                 .when("4", "4").otherwise("3").eqExpression("2").endOr();
-        String expected = "SELECT COUNT(versions_1.id) FROM Document d LEFT JOIN d.versions versions_1 GROUP BY d.id, CASE d.id WHEN d.age THEN 2 WHEN 4 THEN 4 ELSE 3 END HAVING CASE d.id WHEN d.age THEN 2 WHEN 4 THEN 4 ELSE 3 END = 2";
+        String expected = "SELECT COUNT(versions_1.id) FROM Document d LEFT JOIN d.versions versions_1 " +
+                "GROUP BY d.id, " + groupByPathExpressions("CASE d.id WHEN d.age THEN 2 WHEN 4 THEN 4 ELSE 3 END", "d.age") + " " +
+                "HAVING CASE d.id WHEN d.age THEN 2 WHEN 4 THEN 4 ELSE 3 END = 2";
         assertEquals(expected, criteria.getQueryString());
         criteria.getResultList(); 
     }

core/testsuite/src/test/java/com/blazebit/persistence/testsuite/SelectTest.java

@@ -436,7 +436,7 @@ public void testSelectNestedAggregate() {
 
         String objectQuery = "SELECT CASE WHEN MIN(d.lastModified) > d.creationDate THEN MIN(d.lastModified) ELSE CURRENT_TIMESTAMP END, owner_1.name "
                 + "FROM Document d JOIN d.owner owner_1 "
-                + "GROUP BY " + groupBy("d.creationDate", "CURRENT_TIMESTAMP", "owner_1.name", renderNullPrecedenceGroupBy("d.id"))
+                + "GROUP BY " + groupBy("d.creationDate", "owner_1.name", renderNullPrecedenceGroupBy("d.id"))
                 + " ORDER BY " + renderNullPrecedence("d.id", "DESC", "LAST");
         assertEquals(objectQuery, cb.getQueryString());
         cb.getResultList();
@@ -453,7 +453,7 @@ public void testSelectNestedAggregatePaginated() {
         String countQuery = "SELECT " + countPaginated("d.id", false) + " FROM Document d";
         String idQuery = "SELECT d.id FROM Document d GROUP BY " + groupBy("d.id", renderNullPrecedenceGroupBy("d.id")) + " ORDER BY " + renderNullPrecedence("d.id", "DESC", "LAST");
         String objectQuery = "SELECT CASE WHEN MIN(d.lastModified) > d.creationDate THEN MIN(d.lastModified) ELSE CURRENT_TIMESTAMP END, owner_1.name FROM Document d JOIN d.owner owner_1 "
-                + "GROUP BY " + groupBy("d.creationDate", "CURRENT_TIMESTAMP", "owner_1.name", renderNullPrecedenceGroupBy("d.id")) + " ORDER BY " + renderNullPrecedence("d.id", "DESC", "LAST");
+                + "GROUP BY " + groupBy("d.creationDate", "owner_1.name", renderNullPrecedenceGroupBy("d.id")) + " ORDER BY " + renderNullPrecedence("d.id", "DESC", "LAST");
 
         assertEquals(countQuery, cb.getPageCountQueryString());
         assertEquals(idQuery, cb.getPageIdQueryString());
@@ -471,7 +471,7 @@ public void testSelectAggregateEntitySelect() {
 
         String objectQuery = "SELECT CASE WHEN MIN(d.lastModified) > d.creationDate THEN MIN(d.lastModified) ELSE CURRENT_TIMESTAMP END, owner_1 FROM Document d "
                 + "JOIN d.owner owner_1 "
-                + "GROUP BY " + groupBy("d.creationDate", "CURRENT_TIMESTAMP", "owner_1.age", "owner_1.id", "owner_1.name", "owner_1.partnerDocument", renderNullPrecedenceGroupBy("d.id"))
+                + "GROUP BY " + groupBy("d.creationDate", "owner_1.age", "owner_1.id", "owner_1.name", "owner_1.partnerDocument", renderNullPrecedenceGroupBy("d.id"))
                 + " ORDER BY " + renderNullPrecedence("d.id", "DESC", "LAST");
 
         assertEquals(objectQuery, cb.getQueryString());
@@ -491,7 +491,7 @@ public void testSelectAggregateEntitySelectPaginated() {
         String idQuery = "SELECT d.id FROM Document d GROUP BY " + groupBy("d.id", renderNullPrecedenceGroupBy("d.id")) + " ORDER BY " + renderNullPrecedence("d.id", "DESC", "LAST");
         String objectQuery = "SELECT CASE WHEN MIN(d.lastModified) > d.creationDate THEN MIN(d.lastModified) ELSE CURRENT_TIMESTAMP END, owner_1 FROM Document d "
                 + "JOIN d.owner owner_1 "
-                + "GROUP BY " + groupBy("d.creationDate", "CURRENT_TIMESTAMP", "owner_1.age", "owner_1.id", "owner_1.name", "owner_1.partnerDocument", renderNullPrecedenceGroupBy("d.id"))
+                + "GROUP BY " + groupBy("d.creationDate", "owner_1.age", "owner_1.id", "owner_1.name", "owner_1.partnerDocument", renderNullPrecedenceGroupBy("d.id"))
                 + " ORDER BY " + renderNullPrecedence("d.id", "DESC", "LAST");
 
         assertEquals(countQuery, cb.getPageCountQueryString());