[#513] fixes #513, prevent literal group bys, fixed test logging

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

@@ -262,7 +262,7 @@ protected AbstractCommonQueryBuilder(MainQuery mainQuery, boolean isMainQuery, D
         this.transformerGroups = Arrays.<ExpressionTransformerGroup<?>>asList(
                 new SimpleTransformerGroup(new OuterFunctionVisitor(joinManager)),
                 new SimpleTransformerGroup(new SubqueryRecursiveExpressionVisitor()),
-                new SizeTransformerGroup(sizeTransformationVisitor, orderByManager, selectManager, joinManager, groupByManager));
+                new SizeTransformerGroup(sizeTransformationVisitor, orderByManager, selectManager, joinManager));
         this.resultType = resultClazz;
 
         this.finalSetOperationBuilder = finalSetOperationBuilder;

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

@@ -24,6 +24,7 @@
 import com.blazebit.persistence.impl.dialect.MySQLDbmsDialect;
 import com.blazebit.persistence.impl.dialect.OracleDbmsDialect;
 import com.blazebit.persistence.impl.dialect.PostgreSQLDbmsDialect;
+import com.blazebit.persistence.impl.function.subquery.SubqueryFunction;
 import com.blazebit.persistence.parser.expression.ConcurrentHashMapExpressionCache;
 import com.blazebit.persistence.impl.function.cast.CastFunction;
 import com.blazebit.persistence.impl.function.count.AbstractCountFunction;
@@ -490,6 +491,12 @@ private void loadFunctions() {
         jpqlFunctionGroup.add("oracle", new LpadRepeatFunction());
         jpqlFunctionGroup.add("microsoft", new ReplicateRepeatFunction());
         registerFunction(jpqlFunctionGroup);
+
+        // subquery
+
+        jpqlFunctionGroup = new JpqlFunctionGroup(SubqueryFunction.FUNCTION_NAME, false);
+        jpqlFunctionGroup.add(null, new SubqueryFunction());
+        registerFunction(jpqlFunctionGroup);
     }
 
     private void loadDbmsDialects() {

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

@@ -139,6 +139,12 @@ private static Set<String> resolveAggregateFunctions(Map<String, JpqlFunctionGro
                 aggregateFunctions.add(entry.getKey().toLowerCase());
             }
         }
+        // add standard JPQL aggregate functions
+        aggregateFunctions.add("sum");
+        aggregateFunctions.add("min");
+        aggregateFunctions.add("max");
+        aggregateFunctions.add("avg");
+        aggregateFunctions.add("count");
         return aggregateFunctions;
     }
 

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

@@ -80,7 +80,11 @@ public Boolean visit(SubqueryExpression expression) {
 
         @Override
         public Boolean visit(FunctionExpression expression) {
-            return com.blazebit.persistence.parser.util.ExpressionUtils.isSizeFunction(expression);
+            if (com.blazebit.persistence.parser.util.ExpressionUtils.isSizeFunction(expression)) {
+                return true;
+            } else {
+                return super.visit(expression);
+            }
         }
     };
 

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

@@ -33,6 +33,7 @@
 import com.blazebit.persistence.parser.expression.FunctionExpression;
 import com.blazebit.persistence.parser.expression.GeneralCaseExpression;
 import com.blazebit.persistence.parser.expression.ListIndexExpression;
+import com.blazebit.persistence.parser.expression.LiteralExpression;
 import com.blazebit.persistence.parser.expression.MapEntryExpression;
 import com.blazebit.persistence.parser.expression.MapKeyExpression;
 import com.blazebit.persistence.parser.expression.MapValueExpression;
@@ -61,7 +62,7 @@
 import com.blazebit.persistence.spi.DbmsDialect;
 
 /**
- * Returns false if expression is usable in groupBy, true otherwise
+ * Returns false if expression is required in groupBy, true otherwise
  * @author Christian Beikov
  * @since 1.0.0
  */
@@ -91,6 +92,10 @@ private boolean setCollect(boolean collect) {
     }
 
     public Set<Expression> extractGroupByExpressions(Expression expression) {
+        // grouping by a literal does not make sense and even causes errors in some DBs such as PostgreSQL
+        if (expression instanceof LiteralExpression) {
+            return Collections.emptySet();
+        }
         clear();
         if (!dbmsDialect.supportsGroupByExpressionInHavingMatching()) {
             expression.accept(new VisitorAdapter() {

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

@@ -105,4 +105,8 @@ public QueryConfiguration getMutableQueryConfiguration() {
     public QueryConfiguration getQueryConfiguration() {
         return queryConfiguration;
     }
+
+    public CriteriaBuilderFactoryImpl getCbf() {
+        return cbf;
+    }
 }

core/impl/src/main/java/com/blazebit/persistence/impl/function/subquery/SubqueryFunction.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright 2014 - 2018 Blazebit.
+ *
+ * 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.blazebit.persistence.impl.function.subquery;
+
+import com.blazebit.persistence.spi.FunctionRenderContext;
+import com.blazebit.persistence.spi.JpqlFunction;
+
+/**
+ * This is an internal FUNCTION that is used to bypass the Hibernate parser for rendering subqueries as
+ * aggregate function arguments.
+ *
+ * @author Moritz Becker
+ * @since 1.2.0
+ */
+public class SubqueryFunction implements JpqlFunction {
+
+    public static final String FUNCTION_NAME = "subquery";
+
+    @Override
+    public boolean hasArguments() {
+        return true;
+    }
+
+    @Override
+    public boolean hasParenthesesIfNoArguments() {
+        return true;
+    }
+
+    @Override
+    public Class<?> getReturnType(Class<?> firstArgumentType) {
+        return firstArgumentType;
+    }
+
+    @Override
+    public void render(FunctionRenderContext context) {
+        context.addArgument(0);
+    }
+}

core/impl/src/main/java/com/blazebit/persistence/impl/transform/SizeTransformationVisitor.java

@@ -18,6 +18,7 @@
 
 import com.blazebit.persistence.impl.AttributeHolder;
 import com.blazebit.persistence.impl.ClauseType;
+import com.blazebit.persistence.impl.function.subquery.SubqueryFunction;
 import com.blazebit.persistence.parser.EntityMetamodel;
 import com.blazebit.persistence.impl.JoinManager;
 import com.blazebit.persistence.impl.JoinNode;
@@ -80,6 +81,7 @@ public class SizeTransformationVisitor extends ExpressionModifierCollectingResul
     private JoinNode currentJoinNode;
     // size expressions with arguments having a blacklisted base node will become subqueries
     private Set<JoinNode> joinNodeBlacklist = new HashSet<>();
+    private boolean aggregateFunctionContext;
 
     public SizeTransformationVisitor(MainQuery mainQuery, SubqueryInitiatorFactory subqueryInitFactory, JoinManager joinManager, JpaProvider jpaProvider) {
         this.mainQuery = mainQuery;
@@ -141,9 +143,17 @@ public Boolean visit(FunctionExpression expression) {
         if (clause != ClauseType.WHERE && ExpressionUtils.isSizeFunction(expression)) {
             return true;
         }
-        return super.visit(expression);
+        if (!aggregateFunctionContext && mainQuery.getCbf().getAggregateFunctions().contains(expression.getFunctionName().toLowerCase())) {
+            aggregateFunctionContext = true;
+            Boolean result = super.visit(expression);
+            aggregateFunctionContext = false;
+            return result;
+        } else {
+            return super.visit(expression);
+        }
     }
 
+    @Override
     protected void onModifier(ExpressionModifier parentModifier) {
         PathExpression sizeArg = (PathExpression) ((FunctionExpression) parentModifier.get()).getExpressions().get(0);
         parentModifier.set(getSizeExpression(parentModifier, sizeArg));
@@ -206,30 +216,32 @@ private Expression getSizeExpression(ExpressionModifier parentModifier, PathExpr
                 // a subquery is required for bags when other collections are joined as well because we cannot rely on distinctness for bags
                 // for now, we always generate a subquery when a bag is encountered
                 jpaProvider.isBag((EntityType<?>) targetAttribute.getDeclaringType(), targetAttribute.getName()) ||
-                requiresBlacklistedNode(sizeArg);
+                requiresBlacklistedNode(sizeArg) ||
+                aggregateFunctionContext;
 
         if (subqueryRequired) {
-            return generateSubquery(sizeArg);
+            return wrapSubqueryConditionally(generateSubquery(sizeArg), aggregateFunctionContext);
         } else {
             if (currentJoinNode != null &&
                     (!currentJoinNode.equals(sizeArgJoin))) {
                 int currentJoinDepth = currentJoinNode.getJoinDepth();
                 int sizeArgJoinDepth = sizeArgJoin.getJoinDepth();
                 if (currentJoinDepth > sizeArgJoinDepth) {
-                    return generateSubquery(sizeArg);
+                    return wrapSubqueryConditionally(generateSubquery(sizeArg), aggregateFunctionContext);
                 } else {
                     // we have to change all transformed expressions to subqueries
                     for (TransformedExpressionEntry transformedExpressionEntry : transformedExpressions) {
                         PathExpression originalSizeArg = transformedExpressionEntry.getOriginalSizeArg();
-                        transformedExpressionEntry.getParentModifier().set(generateSubquery(originalSizeArg));
+                        Expression subquery = wrapSubqueryConditionally(generateSubquery(originalSizeArg), transformedExpressionEntry.isAggregateFunctionContext());
+                        transformedExpressionEntry.getParentModifier().set(subquery);
                     }
                     transformedExpressions.clear();
                     requiredGroupBys.clear();
                     lateJoins.clear();
                     distinctRequired = false;
 
                     if (currentJoinDepth == sizeArgJoinDepth) {
-                        return generateSubquery(sizeArg);
+                        return wrapSubqueryConditionally(generateSubquery(sizeArg), aggregateFunctionContext);
                     }
                 }
             }
@@ -265,7 +277,7 @@ private Expression getSizeExpression(ExpressionModifier parentModifier, PathExpr
             countArguments.add(keyExpression);
 
             AggregateExpression countExpr = createCountFunction(distinctRequired, countArguments);
-            transformedExpressions.add(new TransformedExpressionEntry(countExpr, originalSizeArg, parentModifier));
+            transformedExpressions.add(new TransformedExpressionEntry(countExpr, originalSizeArg, parentModifier, aggregateFunctionContext));
 
             String joinLookupKey = getJoinLookupKey(sizeArg);
             LateJoinEntry lateJoin = lateJoins.get(joinLookupKey);
@@ -342,6 +354,19 @@ private SubqueryExpression generateSubquery(PathExpression sizeArg) {
         return new SubqueryExpression(countSubquery);
     }
 
+    private Expression wrapSubqueryConditionally(SubqueryExpression subquery, boolean wrap) {
+        if (wrap) {
+            // we need to wrap subqueries in aggregate functions in COALESCE to trick the Hibernate parser
+            // see https://hibernate.atlassian.net/browse/HHH-9331
+            List<Expression> subqueryFunctionArguments = new ArrayList<>(1);
+            subqueryFunctionArguments.add(new StringLiteral(SubqueryFunction.FUNCTION_NAME));
+            subqueryFunctionArguments.add(subquery);
+            return new FunctionExpression("FUNCTION", subqueryFunctionArguments);
+        } else {
+            return subquery;
+        }
+    }
+
     /**
      * @author Christian Beikov
      * @since 1.2.0
@@ -350,11 +375,13 @@ private static class TransformedExpressionEntry {
         private final AggregateExpression transformedExpression;
         private final PathExpression originalSizeArg;
         private final ExpressionModifier parentModifier;
+        private final boolean aggregateFunctionContext;
 
-        public TransformedExpressionEntry(AggregateExpression transformedExpression, PathExpression originalSizeArg, ExpressionModifier parentModifier) {
+        public TransformedExpressionEntry(AggregateExpression transformedExpression, PathExpression originalSizeArg, ExpressionModifier parentModifier, boolean aggregateFunctionContext) {
             this.transformedExpression = transformedExpression;
             this.originalSizeArg = originalSizeArg;
             this.parentModifier = parentModifier;
+            this.aggregateFunctionContext = aggregateFunctionContext;
         }
 
         public AggregateExpression getTransformedExpression() {
@@ -368,6 +395,10 @@ public PathExpression getOriginalSizeArg() {
         public ExpressionModifier getParentModifier() {
             return parentModifier;
         }
+
+        public boolean isAggregateFunctionContext() {
+            return aggregateFunctionContext;
+        }
     }
 
     /**

core/impl/src/main/java/com/blazebit/persistence/impl/transform/SizeTransformerGroup.java

@@ -18,7 +18,6 @@
 
 import com.blazebit.persistence.impl.AbstractManager;
 import com.blazebit.persistence.impl.ClauseType;
-import com.blazebit.persistence.impl.GroupByManager;
 import com.blazebit.persistence.impl.JoinManager;
 import com.blazebit.persistence.impl.JoinNode;
 import com.blazebit.persistence.impl.OrderByManager;
@@ -42,15 +41,13 @@ public class SizeTransformerGroup implements ExpressionTransformerGroup<Expressi
     private final SizeTransformationVisitor sizeTransformationVisitor;
     private final SelectManager<?> selectManager;
     private final JoinManager joinManager;
-    private final GroupByManager groupByManager;
     private final SizeExpressionTransformer sizeExpressionTransformer;
     private final SizeSelectInfoTransformer sizeSelectExpressionTransformer;
 
-    public SizeTransformerGroup(SizeTransformationVisitor sizeTransformationVisitor, OrderByManager orderByManager, SelectManager<?> selectManager, JoinManager joinManager, GroupByManager groupByManager) {
+    public SizeTransformerGroup(SizeTransformationVisitor sizeTransformationVisitor, OrderByManager orderByManager, SelectManager<?> selectManager, JoinManager joinManager) {
         this.sizeTransformationVisitor = sizeTransformationVisitor;
         this.selectManager = selectManager;
         this.joinManager = joinManager;
-        this.groupByManager = groupByManager;
         this.sizeExpressionTransformer = new SizeExpressionTransformer(sizeTransformationVisitor);
         this.sizeSelectExpressionTransformer = new SizeSelectInfoTransformer(sizeTransformationVisitor, orderByManager);
     }