From 07c4c795bd5e8bc4fd0e46419778356922a27d71 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 25 Jul 2013 21:37:30 +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 897ad1e3a..35a356f03 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 @@ -54,6 +54,7 @@ import org.springframework.util.StringUtils; * Simple utility class to create JPA queries. * * @author Oliver Gierke + * @author Thomas Darimont */ public abstract class QueryUtils { @@ -64,6 +65,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; @@ -328,8 +330,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 fba5ce310..66f8b02cd 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 { @@ -186,7 +187,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"); } /** @@ -209,6 +210,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)); }