diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractFullQueryBuilder.java b/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractFullQueryBuilder.java index 04d7c12649..422c3968ab 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractFullQueryBuilder.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractFullQueryBuilder.java @@ -443,7 +443,7 @@ private PaginatedCriteriaBuilder page(int firstRow, int pageSize, ResolvedExp throw new IllegalStateException("Cannot paginate a DISTINCT query"); } if (groupByManager.hasGroupBys() && identifierExpressions != null) { - throw new IllegalStateException("Cannot paginate a GROUP BY query by the expressions [" + expressionString(identifierExpressions) + "] as it is implicitly paginated by "); + throw new IllegalStateException("Cannot paginate a GROUP BY query by the expressions [" + expressionString(identifierExpressions) + "] as it is implicitly paginated by it's group by clause!"); } if (!havingManager.isEmpty()) { throw new IllegalStateException("Cannot paginate a HAVING query"); diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedCriteriaBuilderImpl.java b/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedCriteriaBuilderImpl.java index b82c63de77..db96dfb09f 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedCriteriaBuilderImpl.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedCriteriaBuilderImpl.java @@ -414,7 +414,10 @@ protected void prepareAndCheck() { hasCollections = joinManager.hasCollections(); if (hasGroupBy && identifierExpressions != null) { - throw new IllegalStateException("Cannot build the query because of implicit GROUP BY clauses that would be added although the query is paginating by the expressions [" + expressionString(identifierExpressions) + "]"); + ResolvedExpression[] uniqueIdentifierExpressions = getUniqueIdentifierExpressions(); + if (!isEquivalent(uniqueIdentifierExpressions, identifierExpressions)) { + throw new IllegalStateException("Cannot build the query because of implicit GROUP BY clauses that would be added although the query is paginating by the expressions [" + expressionString(identifierExpressions) + "]"); + } } // Paginated criteria builders always need the last order by expression to be unique @@ -436,6 +439,25 @@ protected void prepareAndCheck() { needsCheck = false; } + private boolean isEquivalent(ResolvedExpression[] uniqueIdentifierExpressions, ResolvedExpression[] identifierExpressions) { + if (uniqueIdentifierExpressions == null || uniqueIdentifierExpressions.length != identifierExpressions.length) { + return false; + } + + int identifiersSize = identifierExpressions.length; + int uniqueIdentifiersSize = uniqueIdentifierExpressions.length; + OUTER: for (int i = 0; i < identifiersSize; i++) { + ResolvedExpression identifierExpression = identifierExpressions[i]; + for (int j = 0; j < uniqueIdentifiersSize; j++) { + if (identifierExpression.equals(uniqueIdentifierExpressions[j])) { + continue OUTER; + } + } + return false; + } + return true; + } + @SuppressWarnings("unchecked") private Map.Entry, KeysetExtractionObjectBuilder> getObjectQuery(boolean normalQueryMode, Set keyRestrictedLeftJoins) { String queryString = getBaseQueryString(); diff --git a/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/PaginationTest.java b/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/PaginationTest.java index b40e97990d..ecb223092e 100644 --- a/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/PaginationTest.java +++ b/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/PaginationTest.java @@ -427,6 +427,67 @@ public void testPaginatedWithGroupBy7() { pcb.getResultList(); } + @Test + @Category(NoEclipselink.class) + // Eclipselink does not render the table alias necessary for the path expression in the count function... + public void testPaginatedWithGroupBy8() { + String expectedCountQuery = "SELECT " + countPaginated("d.id", false) + " FROM Document d"; + String expectedIdQuery = "SELECT d.id FROM Document d GROUP BY " + groupBy("d.id", "d.name", "d.age") + " ORDER BY d.name ASC, d.age ASC, d.id ASC"; + String expectedObjectQuery = "SELECT d.id, d.name, COUNT(" + joinAliasValue("contacts_1", "id") + ") FROM Document d LEFT JOIN d.contacts contacts_1" + + " WHERE d.id IN :ids" + + " GROUP BY " + groupBy("d.id", "d.name", "d.age") + + " ORDER BY d.name ASC, d.age ASC, d.id ASC"; + CriteriaBuilder cb = cbf.create(em, Tuple.class).from(Document.class, "d") + .select("d.id") + .select("d.name") + .select("COUNT(contacts.id)"); + cb.page(0, 1); + cb.orderByAsc("d.name").orderByAsc("d.age").orderByAsc("d.id"); + + PaginatedCriteriaBuilder pcb = cb.page(0, 1, "d.id"); + assertEquals(expectedCountQuery, pcb.getPageCountQueryString()); + assertEquals(expectedIdQuery, pcb.getPageIdQueryString()); + assertEquals(expectedObjectQuery, pcb.getQueryString()); + pcb.getResultList(); + } + + @Test + @Category(NoEclipselink.class) + // Eclipselink does not render the table alias necessary for the path expression in the count function... + public void testPaginatedWithGroupBy9() { + String expectedCountQuery = "SELECT " + countPaginated("d.id", false) + " FROM Document d"; + String expectedIdQuery = "SELECT d.id FROM Document d GROUP BY " + groupBy("d.id") + " ORDER BY d.id ASC"; + String expectedObjectQuery = "SELECT d.id, d.name, COUNT(" + joinAliasValue("contacts_1", "id") + ") FROM Document d LEFT JOIN d.contacts contacts_1" + + " WHERE d.id IN :ids" + + " GROUP BY " + groupBy("d.id", "d.name") + + " ORDER BY d.id ASC"; + CriteriaBuilder cb = cbf.create(em, Tuple.class).from(Document.class, "d") + .select("d.id") + .select("d.name") + .select("COUNT(contacts.id)"); + cb.page(0, 1); + cb.orderByAsc("d.id"); + + PaginatedCriteriaBuilder pcb = cb.page(0, 1, "d.id"); + assertEquals(expectedCountQuery, pcb.getPageCountQueryString()); + assertEquals(expectedIdQuery, pcb.getPageIdQueryString()); + assertEquals(expectedObjectQuery, pcb.getQueryString()); + pcb.getResultList(); + } + + @Test + public void testPaginatedWithGroupBy10() { + CriteriaBuilder cb = cbf.create(em, Tuple.class).from(Document.class, "d") + .select("d.id") + .select("d.name") + .select("COUNT(contacts.id)"); + cb.page(0, 1); + cb.groupBy("d.name").orderByAsc("d.name").orderByAsc("d.age"); + + // No unique ordering + verifyException(cb, IllegalStateException.class).page(0, 1, "d.id"); + } + @Test public void testPaginateExplicitWithGroupBy() { CriteriaBuilder cb = cbf.create(em, Tuple.class).from(Document.class, "d") diff --git a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/proxy/ProxyFactory.java b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/proxy/ProxyFactory.java index 4a2d09e748..12516b6469 100644 --- a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/proxy/ProxyFactory.java +++ b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/proxy/ProxyFactory.java @@ -311,7 +311,7 @@ private Class createProxyClass(EntityViewManager entityViewMana parentField = new CtField(pool.get(DirtyTracker.class.getName()), "$$_parent", cc); parentField.setModifiers(getModifiers(true)); cc.addField(parentField); - parentIndexField = new CtField(pool.get(int.class.getName()), "$$_parentIndex", cc); + parentIndexField = new CtField(CtClass.intType, "$$_parentIndex", cc); parentIndexField.setModifiers(getModifiers(true)); cc.addField(parentIndexField); @@ -392,7 +392,7 @@ private Class createProxyClass(EntityViewManager entityViewMana if (mutableAttributeCount > 64) { throw new IllegalArgumentException("Support for more than 64 mutable attributes per view is not yet implemented! " + viewType.getJavaType().getName() + " has " + mutableAttributeCount); } else { - dirtyField = new CtField(pool.get(long.class.getName()), "$$_dirty", cc); + dirtyField = new CtField(CtClass.longType, "$$_dirty", cc); dirtyField.setModifiers(getModifiers(true)); cc.addField(dirtyField); @@ -1641,7 +1641,7 @@ private CtMethod createEquals(Class viewClass, CtClass cc, boolean idBased, C for (CtField field : fields) { if (field.getType().isPrimitive()) { - if ("boolean".equals(field.getType().getName())) { + if (CtClass.booleanType == field.getType()) { sb.append("\tif ($0.").append(field.getName()).append(" != other.is"); StringUtils.addFirstToUpper(sb, field.getName()).append("()"); sb.append(") {\n"); @@ -1687,33 +1687,28 @@ private CtMethod createHashCode(CtClass cc, CtField... fields) throws NotFoundEx for (CtField field : fields) { if (field.getType().isPrimitive()) { - Class type; - try { - type = ReflectionUtils.getClass(Descriptor.toClassName(field.getFieldInfo().getDescriptor())); - } catch (ClassNotFoundException ex) { - throw new IllegalArgumentException("Unsupported primitive type: " + Descriptor.toClassName(field.getFieldInfo().getDescriptor()), ex); - } - if (double.class == type) { + CtClass type = field.getType(); + if (CtClass.doubleType == type) { sb.append("long bits = java.lang.Double.doubleToLongBits($0.").append(field.getName()).append(");"); } sb.append("\thash = 83 * hash + "); - if (boolean.class == type) { - sb.append("$0.").append(field.getName()).append(" ? 1231 : 1237").append(";\n"); - } else if (byte.class == type || short.class == type || char.class == type) { + if (CtClass.booleanType == type) { + sb.append("($0.").append(field.getName()).append(" ? 1231 : 1237").append(");\n"); + } else if (CtClass.byteType == type || CtClass.shortType == type || CtClass.charType == type) { sb.append("(int) $0.").append(field.getName()).append(";\n"); - } else if (int.class == type) { + } else if (CtClass.intType == type) { sb.append("$0.").append(field.getName()).append(";\n"); - } else if (long.class == type) { + } else if (CtClass.longType == type) { sb.append("(int)("); sb.append("$0.").append(field.getName()); sb.append(" ^ ("); sb.append("$0.").append(field.getName()); sb.append(" >>> 32));\n"); - } else if (float.class == type) { + } else if (CtClass.floatType == type) { sb.append("java.lang.Float.floatToIntBits("); sb.append("$0.").append(field.getName()); sb.append(");\n"); - } else if (double.class == type) { + } else if (CtClass.doubleType == type) { sb.append("(int)(bits ^ (bits >>> 32));\n"); } else { throw new IllegalArgumentException("Unsupported primitive type: " + type.getName()); @@ -2296,12 +2291,7 @@ private String addIdAccessor(CtClass declaringClass, IdentifiableType type, j } ClassPool classPool = declaringClass.getClassPool(); - CtClass boxedType; - if (idType.isPrimitive()) { - boxedType = pool.get(ReflectionUtils.getWrapperClassOfPrimitve(idType.toClass()).getName()); - } else { - boxedType = idType; - } + CtClass boxedType = autoBox(classPool, idType); String desc = "(" + Descriptor.of(type.getJavaType().getName()) + ")" + Descriptor.of(boxedType); @@ -2367,6 +2357,20 @@ private String addIdAccessor(CtClass declaringClass, IdentifiableType type, j return declaringClass.getName() + "#" + name; } + private CtClass autoBox(ClassPool classPool, CtClass fieldType) { + if (fieldType.isPrimitive()) { + String typeName = fieldType.getName(); + Class type; + try { + type = ReflectionUtils.getWrapperClassOfPrimitve(ReflectionUtils.getClass(typeName)); + return classPool.get(type.getName()); + } catch (Exception ex) { + throw new IllegalArgumentException("Unsupported primitive type: " + typeName, ex); + } + } + return fieldType; + } + private void autoBox(Bytecode code, ClassPool classPool, CtClass fieldType) { if (fieldType.isPrimitive()) { String typeName = fieldType.getName();