diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaCountQueryCreator.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaCountQueryCreator.java index af9114592..6b6c2a660 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaCountQueryCreator.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaCountQueryCreator.java @@ -32,9 +32,12 @@ import org.springframework.lang.Nullable; * @author Oliver Gierke * @author Marc Lefrançois * @author Mark Paluch + * @author Greg Turnquist */ public class JpaCountQueryCreator extends JpaQueryCreator { + private boolean distinct; + /** * Creates a new {@link JpaCountQueryCreator}. * @@ -46,6 +49,7 @@ public class JpaCountQueryCreator extends JpaQueryCreator { public JpaCountQueryCreator(PartTree tree, ReturnedType type, CriteriaBuilder builder, ParameterMetadataProvider provider) { super(tree, type, builder, provider); + this.distinct = tree.isDistinct(); } @Override @@ -63,7 +67,7 @@ public class JpaCountQueryCreator extends JpaQueryCreator { } @SuppressWarnings("rawtypes") - private static Expression getCountQuery(CriteriaQuery query, CriteriaBuilder builder, Root root) { - return query.isDistinct() ? builder.countDistinct(root) : builder.count(root); + private Expression getCountQuery(CriteriaQuery query, CriteriaBuilder builder, Root root) { + return distinct ? builder.countDistinct(root) : builder.count(root); } } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryCreator.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryCreator.java index c23eef900..8a6527b11 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryCreator.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryCreator.java @@ -18,12 +18,6 @@ package org.springframework.data.jpa.repository.query; import static org.springframework.data.jpa.repository.query.QueryUtils.*; import static org.springframework.data.repository.query.parser.Part.Type.*; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Iterator; -import java.util.List; -import java.util.stream.Collectors; - import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaQuery; import jakarta.persistence.criteria.Expression; @@ -34,6 +28,12 @@ import jakarta.persistence.criteria.Root; import jakarta.persistence.criteria.Selection; import jakarta.persistence.metamodel.SingularAttribute; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; +import java.util.List; +import java.util.stream.Collectors; + import org.springframework.data.domain.Sort; import org.springframework.data.jpa.repository.query.ParameterMetadataProvider.ParameterMetadata; import org.springframework.data.mapping.PropertyPath; @@ -84,7 +84,7 @@ public class JpaQueryCreator extends AbstractQueryCreator criteriaQuery = createCriteriaQuery(builder, type); this.builder = builder; - this.query = criteriaQuery.distinct(tree.isDistinct()); + this.query = criteriaQuery.distinct(tree.isDistinct() && !tree.isCountProjection()); this.root = query.from(type.getDomainType()); this.provider = provider; this.returnedType = type; diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryRewriteIntegrationTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryRewriteIntegrationTests.java index 61d0db240..37497195e 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryRewriteIntegrationTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryRewriteIntegrationTests.java @@ -35,6 +35,7 @@ import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.jpa.domain.sample.User; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.JpaSpecificationExecutor; import org.springframework.data.jpa.repository.Query; import org.springframework.data.jpa.repository.QueryRewriter; import org.springframework.data.jpa.repository.config.EnableJpaRepositories; @@ -145,7 +146,21 @@ public class JpaQueryRewriteIntegrationTests { entry(SORT, Sort.unsorted().toString())); } - public interface UserRepositoryWithRewriter extends JpaRepository, QueryRewriter { + @Test // GH-1380 + void counting() { + + repository.saveAllAndFlush(List.of( // + new User("Frodo", "Baggins", "ringdude@aol.com"), // + new User("Bilbo", "Baggins", "riddler@hotmail.com"), // + new User("Samwise", "Gamgee", "gardener@gmail.com"))); + + assertThat(repository.count()).isEqualTo(3); + assertThat(repository.countDistinctByLastname("Baggins")).isEqualTo(2); + assertThat(repository.countDistinctByLastname("Gamgee")).isEqualTo(1); + } + + public interface UserRepositoryWithRewriter + extends JpaRepository, QueryRewriter, JpaSpecificationExecutor { @Query(value = "select original_user_alias.* from SD_USER original_user_alias", nativeQuery = true, queryRewriter = TestQueryRewriter.class) @@ -170,6 +185,8 @@ public class JpaQueryRewriteIntegrationTests { queryRewriter = UserRepositoryWithRewriter.class) List findByNativeQueryUsingRepository(String param); + long countDistinctByLastname(String lastname); + @Override default String rewrite(String query, Sort sort) { return replaceAlias(query, sort); diff --git a/spring-data-jpa/src/test/resources/logback.xml b/spring-data-jpa/src/test/resources/logback.xml index 2bb9893fd..d7b83cb95 100644 --- a/spring-data-jpa/src/test/resources/logback.xml +++ b/spring-data-jpa/src/test/resources/logback.xml @@ -10,12 +10,13 @@ - - - + + + + - \ No newline at end of file + diff --git a/src/main/asciidoc/jpa.adoc b/src/main/asciidoc/jpa.adoc index 9d1a7c24b..125dd5094 100644 --- a/src/main/asciidoc/jpa.adoc +++ b/src/main/asciidoc/jpa.adoc @@ -253,6 +253,27 @@ The following table describes the keywords supported for JPA and what a method c NOTE: `In` and `NotIn` also take any subclass of `Collection` as a parameter as well as arrays or varargs. For other syntactical versions of the same logical operator, check "`<>`". +[WARNING] +==== +`DISTINCT` can be tricky and not always producing the results you expect. +For example, `select distinct u from User u` will produce a complete different result than `select distinct u.lastname from User u`. +In the first case, since you are including `User.id`, nothing will duplicated, hence you'll get the whole table, and it would be of `User` objects. + +However, that latter query would narrow the focus to just `User.lastname` and find all unique last names for that table. +This would also yield a `List` result set instead of a `List result set. + + +`countDistinctByLastname(String lastname)` can also produce unexpected results. +Spring Data JPA will derive `select count(distinct u.id) from User u where u.lastname = ?1`. +Again, since `u.id` won't hit any duplicates, this query will count up all the users that had the binding last name. +Which would the same as `countByLastname(String lastname)`! + +What is the point of this query anyway? To find the number of people with a given last name? To find the number of _distinct_ people with that binding last name? +To find the number of _distinct last names_? (That last one is an entirely different query!) +Using `distinct` sometimes requires writing the query by hand and using `@Query` to best capture the information you seek, since you also may be needing a projection +to capture the result set. +==== + [[jpa.query-methods.named-queries]] === Using JPA Named Queries