Remove duplicate 'distinct' applied in count operations.

When performing a count operation, we are using countDistinct from JPA, and hence, don't need the JpaQueryCreator applying distinct outside the whole thing.

Also added some details in the ref docs to help guide users on writing proper distinct-based queries.

See #1380.
This commit is contained in:
Greg L. Turnquist
2022-05-09 09:03:52 -05:00
parent c0cadfa400
commit 8ea70c1e42
5 changed files with 57 additions and 14 deletions

View File

@@ -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);
}
}

View File

@@ -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<? extend
CriteriaQuery<?> 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;

View File

@@ -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<User, Integer>, 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<User, Integer>, QueryRewriter, JpaSpecificationExecutor<User> {
@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<User> findByNativeQueryUsingRepository(String param);
long countDistinctByLastname(String lastname);
@Override
default String rewrite(String query, Sort sort) {
return replaceAlias(query, sort);

View File

@@ -10,12 +10,13 @@
<logger name="org.springframework.data" level="error"/>
<!-- Uncomment these sections to debug -->
<!-- <logger name="org.springframework.data.jpa" level="trace" />-->
<!-- <logger name="org.springframework.jdbc" level="debug" />-->
<!-- <logger name="org.testcontainers" level="debug" />-->
<!-- <logger name="org.springframework.data.jpa" level="trace" />-->
<!-- <logger name="org.springframework.jdbc" level="debug" />-->
<!-- <logger name="org.hibernate.SQL" level="debug" />-->
<!-- <logger name="org.testcontainers" level="debug" />-->
<root level="error">
<appender-ref ref="console"/>
</root>
</configuration>
</configuration>

View File

@@ -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 "`<<repository-query-keywords>>`".
[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<String>` result set instead of a `List<User`> 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