Skip to content

Commit

Permalink
ORM/Panache: rely on ORM getResultCount
Browse files Browse the repository at this point in the history
This allows us to not care about which parameters are set on the count
query, since `order by` parameters are ignored by ORM. Which is not
the case if we write our own count query.

Note that this is not yet available in HR, so we have to keep the count
query logic around until that's available.

Fixes #40962
  • Loading branch information
FroMage committed Jun 6, 2024
1 parent 20d3c9d commit d2dcb5e
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,12 @@ public void close() {
* this is the original Panache-Query, if any (can be null)
*/
private String originalQuery;
protected String countQuery;
/**
* This is only used by the Spring Data JPA extension, due to Spring's Query annotation allowing a custom count query
* See https://docs.spring.io/spring-data/jpa/reference/jpa/query-methods.html#jpa.query-methods.at-query.native
* Otherwise we do not use this, and rely on ORM to generate count queries
*/
protected String customCountQueryForSpring;
private String orderBy;
private EntityManager em;

Expand All @@ -72,7 +77,7 @@ public CommonPanacheQueryImpl(EntityManager em, String query, String originalQue
private CommonPanacheQueryImpl(CommonPanacheQueryImpl<?> previousQuery, String newQueryString, String countQuery) {
this.em = previousQuery.em;
this.query = newQueryString;
this.countQuery = countQuery;
this.customCountQueryForSpring = countQuery;
this.orderBy = previousQuery.orderBy;
this.paramsArrayOrMap = previousQuery.paramsArrayOrMap;
this.page = previousQuery.page;
Expand Down Expand Up @@ -118,13 +123,14 @@ public <T> CommonPanacheQueryImpl<T> project(Class<T> type) {
}

newQuery.append("new ").append(type.getName()).append("(").append(selectClause).append(")").append(from);
return new CommonPanacheQueryImpl<>(this, newQuery.toString(), "select count(*) " + from);
// I think projections do not change the result count, so we can keep the custom count query
return new CommonPanacheQueryImpl<>(this, newQuery.toString(), customCountQueryForSpring);
}

// build select clause with a constructor expression
String selectClause = "SELECT " + getParametersFromClass(type, null);
return new CommonPanacheQueryImpl<>(this, selectClause + selectQuery,
"select count(*) " + selectQuery);
// I think projections do not change the result count, so we can keep the custom count query
return new CommonPanacheQueryImpl<>(this, selectClause + selectQuery, customCountQueryForSpring);
}

private StringBuilder getParametersFromClass(Class<?> type, String parentParameter) {
Expand Down Expand Up @@ -276,35 +282,28 @@ public void withHint(String hintName, Object value) {

// Results

@SuppressWarnings("unchecked")
public long count() {
if (count == null) {
String selectQuery = query;
if (PanacheJpaUtil.isNamedQuery(query)) {
org.hibernate.query.Query q = (org.hibernate.query.Query) em.createNamedQuery(query.substring(1));
selectQuery = q.getQueryString();
}

Query countQuery = em.createQuery(countQuery(selectQuery));
if (paramsArrayOrMap instanceof Map)
AbstractJpaOperations.bindParameters(countQuery, (Map<String, Object>) paramsArrayOrMap);
else
AbstractJpaOperations.bindParameters(countQuery, (Object[]) paramsArrayOrMap);
try (NonThrowingCloseable c = applyFilters()) {
count = (Long) countQuery.getSingleResult();
if (customCountQueryForSpring != null) {
Query countQuery = em.createQuery(customCountQueryForSpring);
if (paramsArrayOrMap instanceof Map)
AbstractJpaOperations.bindParameters(countQuery, (Map<String, Object>) paramsArrayOrMap);
else
AbstractJpaOperations.bindParameters(countQuery, (Object[]) paramsArrayOrMap);
try (NonThrowingCloseable c = applyFilters()) {
count = (Long) countQuery.getSingleResult();
}
} else {
@SuppressWarnings("rawtypes")
org.hibernate.query.Query query = (org.hibernate.query.Query) createBaseQuery();
try (NonThrowingCloseable c = applyFilters()) {
count = query.getResultCount();
}
}
}
return count;
}

private String countQuery(String selectQuery) {
if (countQuery != null) {
return countQuery;
}

return PanacheJpaUtil.getFastCountQuery(selectQuery);
}

@SuppressWarnings("unchecked")
public <T extends Entity> List<T> list() {
Query jpaQuery = createQuery();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public CustomCountPanacheQuery(EntityManager em, Query jpaQuery, String customCo
Object paramsArrayOrMap) {
super(new CommonPanacheQueryImpl<>(em, castQuery(jpaQuery).getQueryString(), null, null, paramsArrayOrMap) {
{
this.countQuery = customCountQuery;
this.customCountQueryForSpring = customCountQuery;
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ public class PanacheJpaUtil {
/**
* This turns an HQL (already expanded from Panache-QL) query into a count query, using text manipulation
* if we can, because it's faster, or fall back to using the ORM HQL parser in {@link #getCountQueryUsingParser(String)}
*
* @deprecated we should use SelectionQuery.getResultCount() when supported by HR (ORM already supports it)
*/
public static String getFastCountQuery(String query) {
// try to generate a good count query from the existing query
Expand Down Expand Up @@ -85,6 +87,8 @@ public static String getFastCountQuery(String query) {
/**
* This turns an HQL (already expanded from Panache-QL) query into a count query, using the
* ORM HQL parser. Slow version, see {@link #getFastCountQuery(String)} for the fast version.
*
* @deprecated we should use SelectionQuery.getResultCount() when supported by HR (ORM already supports it)
*/
public static String getCountQueryUsingParser(String query) {
HqlLexer lexer = new HqlLexer(CharStreams.fromString(query));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package io.quarkus.it.panache;

import jakarta.persistence.Entity;

import io.quarkus.hibernate.orm.panache.PanacheEntity;

@Entity
public class Bug40962Entity extends PanacheEntity {
public String name;
public String location;
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand Down Expand Up @@ -1843,4 +1844,16 @@ public String testBug36496() {
Person.count("WITH id AS (SELECT p.id AS pid FROM Person2 AS p) SELECT count(*) FROM Person2 p"));
return "OK";
}

@GET
@Path("40962")
@Transactional
public String testBug40962() {
// should not throw
// Bug40962Entity.find("name = :name ORDER BY locate(location, :location) DESC",
// Map.of("name", "Demo", "location", "something")).count();
Bug40962Entity.find("FROM Bug40962Entity WHERE name = :name ORDER BY locate(location, :location) DESC",
Map.of("name", "Demo", "location", "something")).count();
return "OK";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -254,4 +254,9 @@ public void testBug26308() {
public void testBug36496() {
RestAssured.when().get("/test/36496").then().body(is("OK"));
}

@Test
public void testBug40962() {
RestAssured.when().get("/test/40962").then().body(is("OK"));
}
}

0 comments on commit d2dcb5e

Please sign in to comment.