Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Group by with paginated criteria builder use case #194

Closed
beikov opened this issue Mar 14, 2016 · 7 comments · Fixed by #611
Closed

Group by with paginated criteria builder use case #194

beikov opened this issue Mar 14, 2016 · 7 comments · Fixed by #611

Comments

@beikov
Copy link
Member

beikov commented Mar 14, 2016

Today we had a use case for using a paginated criteria builder in combination with a group by.
Right now, we don't support that, since we need some group by transformations to make a paginated criteria builder possible.

Think about what we can do to make this use case possible.

We had a UserAction entity with a composite id consisting of properties userId, action, objectId.
An entity referenced a set of UserActions via a collection table and in a view, we wanted to visualize the set of UserActions in a paginated table grouped by userId.

Apart from the group by not being allowed, the entity view also required that the id attribute must match the id attribute of the entity. Since group by is used, this would actually not be necessary.

First we should think about lifting the requirement that group by is not allowed with paginated criteria builders.
Finally we might also want to make it possible to define a "custom" id attribute that must not match the entities id attribute. (This will always require a group by?)

@beikov
Copy link
Member Author

beikov commented Mar 14, 2016

I think that giving the possibility to define the id attribute by which the PaginatedCriteriaBuilder should paginate is enough. That would make custom groupings possible which entity views could directly make use of as they are requiring an I'd mapping anyway.

@jwgmeligmeyling
Copy link
Collaborator

I think I stumbled up on this issue too, can you confirm?

CriteriaBuilder

        return scopeEntityManager.getRecursiveScopeChildrenCriteriaBuilder(scope, Tuple.class)
            .from(Contract.class, "contract")
            .joinDefault("contract.scopes", "contractScope", JoinType.INNER)
            .where("contractScope.id").in().from(ScopeCte.class, "s").select("s.id").end()
            .whereOr().where("contract.endDate").isNull().where("contract.endDate").gt(LocalDateTime.now()).endOr()
            .select("contract.employee.id", "id")
            .select("contract.employee.name", "name")
            .select("contract.employee.surname", "surname")
            .select("contract.employee.email", "email")
            .select("contract.employee.mobilePhone", "mobilePhone")
            .select("FUNCTION('GROUP_CONCAT', contract.function.name, 'SEPARATOR', ', ', 'ORDER BY', contract.function.name, 'ASC')", "functions")
            .orderByAsc("contract.employee.surname").orderByAsc("contract.employee.name").orderByAsc("contract.employee.id")
            .page(firstResult, maxResults)
            .getResultList();

Error message

java.lang.IllegalStateException: The last order by item must be unique!
	at com.blazebit.persistence.impl.PaginatedCriteriaBuilderImpl.prepareAndCheck(PaginatedCriteriaBuilderImpl.java:326)
	at com.blazebit.persistence.impl.PaginatedCriteriaBuilderImpl.getQuery(PaginatedCriteriaBuilderImpl.java:189)
	at com.blazebit.persistence.impl.PaginatedCriteriaBuilderImpl.getResultList(PaginatedCriteriaBuilderImpl.java:222)
	at com.pallasathenagroup.entitymanagers.EmployeeEntityManager.listEmployeesWithFunctionsInScope(EmployeeEntityManager.java:170)
	at com.pallasathena.export.FinancialOverviewExporter.main(FinancialOverviewExporter.java:117)

@beikov
Copy link
Member Author

beikov commented Nov 26, 2017

The message tells you already :)
The last order by item must be a unique attribute from the perspective of the query root, otherwise the pagination wouldn't be deterministic. If a contract is identified by an employee, then you are hitting another problem which would be #418

@jwgmeligmeyling
Copy link
Collaborator

Actually, now that I know this limitation, I changed my Criteria expressions to:

    return scopeEntityManager.getRecursiveScopeChildrenCriteriaBuilder(scope, Tuple.class)
        .from(Employee.class, "employee")
        .joinDefault("employee.contracts", "contract", JoinType.INNER)
        .joinDefault("contract.scopes", "contractScope", JoinType.INNER)
        .where("contractScope.id").in().from(ScopeCte.class, "s").select("s.id").end()
        .whereOr().where("contract.endDate").isNull().where("contract.endDate").gt(LocalDateTime.now()).endOr()
        .select("employee.id", "id")
        .select("employee.name", "name")
        .select("employee.surname", "surname")
        .select("employee.email", "email")
        .select("employee.mobilePhone", "mobilePhone")
        .select("FUNCTION('GROUP_CONCAT', contract.function.name, 'SEPARATOR', ', ', 'ORDER BY', contract.function.name, 'ASC')", "functions")
        .orderByAsc("employee.surname").orderByAsc("employee.name").orderByAsc("employee.id")
        .page(firstResult, maxResults)
        .getResultList();

And it seems to pass my initial test.

beikov added a commit to beikov/blaze-persistence that referenced this issue Jul 25, 2018
…nce handling when order by item is not nullable. Fixes Blazebit#418. Fixes Blazebit#569. Fixes Blazebit#194
@beikov beikov added this to the 1.3.0 milestone Jul 25, 2018
@beikov beikov self-assigned this Jul 25, 2018
beikov added a commit to beikov/blaze-persistence that referenced this issue Jul 25, 2018
…nce handling when order by item is not nullable. Fixes Blazebit#418. Fixes Blazebit#569. Fixes Blazebit#194
beikov added a commit to beikov/blaze-persistence that referenced this issue Jul 25, 2018
…nce handling when order by item is not nullable. Fixes Blazebit#418. Fixes Blazebit#569. Fixes Blazebit#194
beikov added a commit to beikov/blaze-persistence that referenced this issue Jul 27, 2018
…nce handling when order by item is not nullable. Fixes Blazebit#418. Fixes Blazebit#569. Fixes Blazebit#194
beikov added a commit to beikov/blaze-persistence that referenced this issue Jul 30, 2018
…nce handling when order by item is not nullable. Fixes Blazebit#418. Fixes Blazebit#569. Fixes Blazebit#194
beikov added a commit to beikov/blaze-persistence that referenced this issue Jul 30, 2018
…nce handling when order by item is not nullable. Fixes Blazebit#418. Fixes Blazebit#569. Fixes Blazebit#194
beikov added a commit to beikov/blaze-persistence that referenced this issue Jul 31, 2018
…nce handling when order by item is not nullable. Fixes Blazebit#418. Fixes Blazebit#569. Fixes Blazebit#194
beikov added a commit to beikov/blaze-persistence that referenced this issue Aug 1, 2018
…nce handling when order by item is not nullable. Fixes Blazebit#418. Fixes Blazebit#569. Fixes Blazebit#194
beikov added a commit to beikov/blaze-persistence that referenced this issue Aug 1, 2018
…nce handling when order by item is not nullable. Fixes Blazebit#418. Fixes Blazebit#569. Fixes Blazebit#194
beikov added a commit to beikov/blaze-persistence that referenced this issue Aug 1, 2018
…y null precedence handling when order by item is not nullable
beikov added a commit that referenced this issue Aug 2, 2018
beikov added a commit to beikov/blaze-persistence that referenced this issue Aug 2, 2018
…cit group by when expressions are equivalent to the minimal group by clause
beikov added a commit to beikov/blaze-persistence that referenced this issue Aug 2, 2018
…cit group by when expressions are equivalent to the minimal group by clause. Fix hashCode generation for primitive booleans in flat views
beikov added a commit that referenced this issue Aug 4, 2018
…p by when expressions are equivalent to the minimal group by clause. Fix hashCode generation for primitive booleans in flat views
@jwgmeligmeyling
Copy link
Collaborator

The scenario where I would use this for is still not supported:

CriteriaBuilder<Contract> resultsQuery = someBaseCriteriaBuilder
                .orderByAsc("employeeContract.employee.name")
                .orderByAsc("employeeContract.employee.surname")
                .orderByAsc("employeeContract.employee.id")
                .groupBy("employeeContract.employee.id");

            EntityViewSetting<ContractBasedEmployeeView, PaginatedCriteriaBuilder<ContractBasedEmployeeView>> setting =
                EntityViewSetting.create(ContractBasedEmployeeView.class, firstResult, maxResults);
            return entityViewManager.applySetting(setting, resultsQuery)
                .getResultList();

Fails with:

java.lang.IllegalStateException: The order by items of the query builder are not guaranteed to produce unique tuples! Consider also ordering by the entity identifier!

	at com.blazebit.persistence.impl.PaginatedCriteriaBuilderImpl.prepareAndCheck(PaginatedCriteriaBuilderImpl.java:447)
	at com.blazebit.persistence.impl.PaginatedCriteriaBuilderImpl.getQuery(PaginatedCriteriaBuilderImpl.java:256)
	at com.blazebit.persistence.impl.PaginatedCriteriaBuilderImpl.getResultList(PaginatedCriteriaBuilderImpl.java:305)
	at com.pallasathenagroup.entitymanagers.EmployeeEntityManager$ScopedQuery.getEmployees(EmployeeEntityManager.java:181)
	at com.pallasathenagroup.entitymanagers.EmployeeEntityManagerTest.testGetEmployeesInScope(EmployeeEntityManagerTest.java:61)

I'd assume uniqueness of the employeeContract.employee.id path (which is the last order by item) would be inferred by the group by clause.

@jwgmeligmeyling
Copy link
Collaborator

Simplified the query, omitted the EntityView which adds conflicting group by clauses on its own. I don't think we have to look into the above exception, because that use of EV's will be very rare.

Basically what I'm trying to achieve comes down to the following Tuple projection:

return getEmployeeContractsCriteriaBuilder(traverseUp, traverseDown, includeInactive, javax.persistence.Tuple.class)
                .orderByAsc("employeeContract.employee.name")
                .orderByAsc("employeeContract.employee.surname")
                .orderByAsc("employeeContract.employee.id")
                .groupBy("employeeContract.employee.id")
                .select("employeeContract.employee.id", "id")
                .select("employeeContract.employee.name", "name")
                .select("employeeContract.employee.surname", "surname")
                .select("CASE WHEN (FUNCTION('MAX_DATE_TIME_NON_NULL', employeeContract.endDate) < CURRENT_TIMESTAMP) THEN false ELSE true END", "active")
                .select("FUNCTION('GROUP_CONCAT', 'DISTINCT', employeeContract.function.name, 'SEPARATOR', ', ')", "function")
                .page(firstResult, maxResults)
                .getResultList();

This fails however with another exception, one that leads me to believe we're not constructing the count clause properly:

java.lang.IllegalArgumentException: org.hibernate.hql.internal.ast.QuerySyntaxException: Invalid path: 'employee_1.name' [SELECT count_tuple('DISTINCT',employeeContract.employee.id, employee_1.name, employee_1.surname) FROM com.pallasathenagroup.entities.iris.Contract employeeContract JOIN employeeContract.function function_1 JOIN employeeContract.scopes contractScope WHERE contractScope IN (SELECT employeeContract.employee.id FROM com.pallasathenagroup.cte.ScopeCte scopeCte) AND (employeeContract.endDate IS NULL OR employeeContract.endDate > :param_1) AND function_1.name <> :param_2]

	at org.hibernate.internal.ExceptionConverterImpl.convert(ExceptionConverterImpl.java:138)
	at org.hibernate.internal.ExceptionConverterImpl.convert(ExceptionConverterImpl.java:181)
	at org.hibernate.internal.ExceptionConverterImpl.convert(ExceptionConverterImpl.java:188)
	at org.hibernate.internal.AbstractSharedSessionContract.createQuery(AbstractSharedSessionContract.java:713)
	at org.hibernate.internal.AbstractSharedSessionContract.createQuery(AbstractSharedSessionContract.java:729)
	at org.hibernate.internal.AbstractSessionImpl.createQuery(AbstractSessionImpl.java:23)
	at com.blazebit.persistence.impl.PaginatedCriteriaBuilderImpl.getCountQuery(PaginatedCriteriaBuilderImpl.java:232)
	at com.blazebit.persistence.impl.PaginatedCriteriaBuilderImpl.getQuery(PaginatedCriteriaBuilderImpl.java:265)
	at com.blazebit.persistence.impl.PaginatedCriteriaBuilderImpl.getResultList(PaginatedCriteriaBuilderImpl.java:305)
	at com.pallasathenagroup.entitymanagers.EmployeeEntityManager$ScopedQuery.getEmployees(EmployeeEntityManager.java:184)
	at com.pallasathenagroup.entitymanagers.EmployeeEntityManagerTest.testGetEmployeesInScope(EmployeeEntityManagerTest.java:62)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
Caused by: org.hibernate.hql.internal.ast.QuerySyntaxException: Invalid path: 'employee_1.name' [SELECT count_tuple('DISTINCT',employeeContract.employee.id, employee_1.name, employee_1.surname) FROM com.pallasathenagroup.entities.iris.Contract employeeContract JOIN employeeContract.function function_1 JOIN employeeContract.scopes contractScope WHERE contractScope IN (SELECT employeeContract.employee.id FROM com.pallasathenagroup.cte.ScopeCte scopeCte) AND (employeeContract.endDate IS NULL OR employeeContract.endDate > :param_1) AND function_1.name <> :param_2]
	at org.hibernate.hql.internal.ast.QuerySyntaxException.convert(QuerySyntaxException.java:74)
	at org.hibernate.hql.internal.ast.ErrorTracker.throwQueryException(ErrorTracker.java:93)
	at org.hibernate.hql.internal.ast.QueryTranslatorImpl.analyze(QueryTranslatorImpl.java:278)
	at org.hibernate.hql.internal.ast.QueryTranslatorImpl.doCompile(QueryTranslatorImpl.java:192)
	at org.hibernate.hql.internal.ast.QueryTranslatorImpl.compile(QueryTranslatorImpl.java:144)
	at org.hibernate.engine.query.spi.HQLQueryPlan.<init>(HQLQueryPlan.java:118)
	at org.hibernate.engine.query.spi.HQLQueryPlan.<init>(HQLQueryPlan.java:78)
	at org.hibernate.engine.query.spi.QueryPlanCache.getHQLQueryPlan(QueryPlanCache.java:158)
	at org.hibernate.internal.AbstractSharedSessionContract.getQueryPlan(AbstractSharedSessionContract.java:595)
	at org.hibernate.internal.AbstractSharedSessionContract.createQuery(AbstractSharedSessionContract.java:704)
	... 33 more

@jwgmeligmeyling
Copy link
Collaborator

My conclusion is that:

  • the count query invalidly omits any join besides the query root
  • even if a group by clause is specified, getGroupByIdentifierExpressions will return any property of that class. A workaround is to group by and order by every single field but it would be more logical to implicitly add all entity props if its identifier is present in the order by / group by clauses (for convenience, because in large tuple projections/domains you may end up with a dozen of paths here)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment