[#675] Always split group by expressions into paths when encountering…

… a parameter
Blazebit · Nov 8, 2018 · 274f12d · 274f12d
1 parent 1094faa
commit 274f12d
Show file tree

Hide file tree

Showing 9 changed files with 5 additions and 65 deletions.
diff --git a/core/api/src/main/java/com/blazebit/persistence/spi/DbmsDialect.java b/core/api/src/main/java/com/blazebit/persistence/spi/DbmsDialect.java
@@ -146,14 +146,6 @@ public interface DbmsDialect {
      */
     public boolean supportsReturningColumns();
 
-    /**
-     * Returns true if the dbms supports complex expressions like subqueries or parameters as group by elements, false otherwise.
-     * 
-     * @return Whether complex group by elements are supported by the dbms
-     * @since 1.1.0
-     */
-    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.
      *

diff --git a/...impl/src/main/java/com/blazebit/persistence/impl/FunctionalDependencyAnalyzerVisitor.java b/...impl/src/main/java/com/blazebit/persistence/impl/FunctionalDependencyAnalyzerVisitor.java
@@ -259,8 +259,10 @@ public Boolean visit(PathExpression expr) {
 
                     // If the identifiers are constantified, we don't care if this is a one-to-one
                     orderedAttributes.removeAll(constantifiedAttributes);
+                    orderedAttributes.remove(expr.getField());
                     if (orderedAttributes.isEmpty() || orderedAttributes.size() == 1 && equalsAny(orderedAttributes.iterator().next(), extendedManagedType.getAttribute(expr.getField()).getColumnEquivalentAttributes())) {
                         nonConstantParent = false;
+                        orderedAttributes.clear();
                         managedType = metamodel.getManagedType(ExtendedManagedType.class, JpaMetamodelUtils.resolveFieldClass(baseNode.getJavaType(), attribute));
                     }
                 }

diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/GroupByExpressionGatheringVisitor.java b/core/impl/src/main/java/com/blazebit/persistence/impl/GroupByExpressionGatheringVisitor.java
@@ -219,13 +219,8 @@ public Boolean visit(MapValueExpression expression) {
 
     @Override
     public Boolean visit(ParameterExpression expression) {
-        // Parameters are complex in the sense that they can't be used in the group by unless supported
-        if (dbmsDialect.supportsComplexGroupBy()) {
-            return false;
-        } else {
-            // TODO: reconsider parameters when we have figured out parameter as literal rendering
-            return true;
-        }
+        // TODO: reconsider parameters when we have figured out parameter as literal rendering
+        return true;
     }
 
     @Override

diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/DB2DbmsDialect.java b/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/DB2DbmsDialect.java
@@ -52,11 +52,6 @@ protected static Map<Class<?>, String> getSqlTypes() {
     public String getWithClause(boolean recursive) {
         return "with";
     }
-
-    @Override
-    public boolean supportsComplexGroupBy() {
-        return false;
-    }
 
     @Override
     public boolean supportsComplexJoinOn() {

diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/DefaultDbmsDialect.java b/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/DefaultDbmsDialect.java
@@ -430,11 +430,6 @@ public boolean supportsReturningColumns() {
         return false;
     }
 
-    @Override
-    public boolean supportsComplexGroupBy() {
-        return true;
-    }
-
     @Override
     public boolean supportsGroupByExpressionInHavingMatching() {
         return true;

diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/MSSQLDbmsDialect.java b/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/MSSQLDbmsDialect.java
@@ -55,12 +55,6 @@ public boolean supportsReturningColumns() {
         return true;
     }
 
-    @Override
-    public boolean supportsComplexGroupBy() {
-        // SQL Server bug? https://support.microsoft.com/en-us/kb/2873474
-        return false;
-    }
-
     @Override
     protected String getOperator(SetOperationType type) {
         if (type == null) {

diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/OracleDbmsDialect.java b/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/OracleDbmsDialect.java
@@ -203,12 +203,6 @@ public boolean supportsReturningColumns() {
         return true;
     }
 
-    @Override
-    public boolean supportsComplexGroupBy() {
-        // Oracle does not support subqueries or parameters in the group by clause
-        return false;
-    }
-
     @Override
     public boolean supportsFullRowValueComparison() {
         return false;

diff --git a/core/testsuite/src/main/java/com/blazebit/persistence/testsuite/DelegatingDbmsDialect.java b/core/testsuite/src/main/java/com/blazebit/persistence/testsuite/DelegatingDbmsDialect.java
@@ -109,11 +109,6 @@ public boolean supportsReturningColumns() {
         return delegate.supportsReturningColumns();
     }
 
-    @Override
-    public boolean supportsComplexGroupBy() {
-        return delegate.supportsComplexGroupBy();
-    }
-
     @Override
     public boolean supportsGroupByExpressionInHavingMatching() {
         return delegate.supportsGroupByExpressionInHavingMatching();

diff --git a/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/GroupByTest.java b/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/GroupByTest.java
@@ -84,37 +84,15 @@ public void testGroupByEntitySelect() {
     public void testSizeTransformWithImplicitParameterGroupBy1() {
         CriteriaBuilder<Long> criteria = cbf.create(em, Long.class).from(Document.class, "d")
                 .select("SIZE(d.versions)")
-                .selectCase().when("d.age").lt(2l).thenExpression("'a'").otherwiseExpression("'b'");
+                .selectCase().when("d.age").lt(2L).thenExpression("'a'").otherwiseExpression("'b'");
 
         final String expected = "SELECT " + function("COUNT_TUPLE", "versions_1.id") + ", CASE WHEN d.age < :param_0 THEN 'a' ELSE 'b' END FROM Document d LEFT JOIN d.versions versions_1 GROUP BY d.id, d.age";
         assertEquals(expected, criteria.getQueryString());
         criteria.getResultList();
     }
 
-    @Test
-    @Category({ NoDB2.class, NoMSSQL.class, NoOracle.class })
-    public void testSizeTransformWithImplicitParameterGroupBy2() {
-        CriteriaBuilder<Long> criteria = cbf.create(em, Long.class).from(Document.class, "d")
-                .select("SIZE(d.versions)")
-                .selectCase().when("d.age").lt(2l).thenExpression("'a'").otherwiseExpression("'b'");
-
-        final String expected = "SELECT " + function("COUNT_TUPLE", "versions_1.id") + ", CASE WHEN d.age < :param_0 THEN 'a' ELSE 'b' END FROM Document d LEFT JOIN d.versions versions_1 " +
-                "GROUP BY d.id, " + groupByPathExpressions("CASE WHEN d.age < :param_0 THEN 'a' ELSE 'b' END", "d.age");
-        assertEquals(expected, criteria.getQueryString());
-        criteria.getResultList();
-    }
-
     @Test
     public void testGroupByKeyExpression() {
-        CriteriaBuilderConfiguration config = Criteria.getDefault();
-        config = configure(config);
-        config.registerDialect(dbms, new DelegatingDbmsDialect(cbf.getService(DbmsDialect.class)) {
-            @Override
-            public boolean supportsComplexGroupBy() {
-                return false;
-            }
-        });
-        cbf = config.createCriteriaBuilderFactory(em.getEntityManagerFactory());
         CriteriaBuilder<Long> criteria = cbf.create(em, Long.class);
         criteria.from(Document.class, "d")
                 .select("KEY(contacts)")