From 5e5f494ac59c41dfdc77518cf7039eb10520f04d Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Thu, 25 Jul 2013 17:53:04 +0200 Subject: [PATCH] DATAJPA-377 - Remove "order by"-part from generated count query. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adjusted createCountQueryFor(…) in QueryUtils to leave out the order by part in the generated query. This avoids problems with databases that require columns specified in the order by clause to be in the select / group by list for count queries (e.g. H2). In addition to that this should give us a little performance boost if the database did not already optimize the query execution. Original pull request: #29. --- .../data/jpa/repository/query/QueryUtils.java | 6 +++++- .../jpa/repository/query/QueryUtilsUnitTests.java | 15 +++++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java index 668430615..c3d071399 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java @@ -55,6 +55,7 @@ import org.springframework.util.StringUtils; * * @author Oliver Gierke * @author Kevin Raymond + * @author Thomas Darimont */ public abstract class QueryUtils { @@ -65,6 +66,7 @@ public abstract class QueryUtils { private static final String COUNT_REPLACEMENT_TEMPLATE = "select count(%s) $5$6$7"; private static final String SIMPLE_COUNT_VALUE = "$2"; private static final String COMPLEX_COUNT_VALUE = "$3$6"; + private static final String ORDER_BY_PART = "(?iu)\\s+order\\s+by\\s+.*$"; private static final Pattern ALIAS_MATCH; private static final Pattern COUNT_MATCH; @@ -332,8 +334,10 @@ public abstract class QueryUtils { boolean useVariable = StringUtils.hasText(variable) && !variable.startsWith("new") && !variable.startsWith("count("); - return matcher.replaceFirst(String.format(COUNT_REPLACEMENT_TEMPLATE, useVariable ? SIMPLE_COUNT_VALUE + String countQuery = matcher.replaceFirst(String.format(COUNT_REPLACEMENT_TEMPLATE, useVariable ? SIMPLE_COUNT_VALUE : COMPLEX_COUNT_VALUE)); + + return countQuery.replaceFirst(ORDER_BY_PART, ""); } /** diff --git a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java index e8729bfe2..158933555 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2012 the original author or authors. + * Copyright 2008-2013 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,6 +29,7 @@ import org.springframework.data.domain.Sort; * Unit test for {@link QueryUtils}. * * @author Oliver Gierke + * @author Thomas Darimont */ public class QueryUtilsUnitTests { @@ -210,7 +211,7 @@ public class QueryUtilsUnitTests { public void usesReturnedVariableInCOuntProjectionIfSet() { assertCountQuery("select distinct m.genre from Media m where m.user = ?1 order by m.genre asc", - "select count(distinct m.genre) from Media m where m.user = ?1 order by m.genre asc"); + "select count(distinct m.genre) from Media m where m.user = ?1"); } /** @@ -233,6 +234,16 @@ public class QueryUtilsUnitTests { assertThat(applySorting("select p from Person p", sort, "p"), endsWith("order by sum(foo) asc")); } + /** + * @see DATAJPA-377 + */ + @Test + public void removesOrderByInGeneratedCountQueryFromOriginalQueryIfPresent() { + + assertCountQuery("select distinct m.genre from Media m where m.user = ?1 OrDer By m.genre ASC", + "select count(distinct m.genre) from Media m where m.user = ?1"); + } + private void assertCountQuery(String originalQuery, String countQuery) { assertThat(createCountQueryFor(originalQuery), is(countQuery)); }