Skip to content

Commit b1437ad

Browse files
jradlicajradlica
authored
Optimized distinct query evaluation in join (#265)
* Optimized distinct query evaluation in join
1 parent f86f245 commit b1437ad

3 files changed

Lines changed: 119 additions & 25 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,50 @@
1+
v3.1.1
2+
=======
3+
* Optimized distinct query evaluation in join
4+
* From now on, during join evaluation, the query is set as `distinct` only when the join is actually being evaluated.
5+
* Previously, `distinct` was set (depending on the config) unconditionally (even when the join was not evaluated), which could lead to performance issues.
6+
* For example, previously for the following specification:
7+
```java
8+
@RequestMapping("/join-distinct/customers/optionalParams")
9+
public Object joinCountSpecificationOptionalParams(
10+
@Join(path = "badges", alias = "b", type = LEFT)
11+
@And({
12+
@Spec(path = "b.badgeType", params = "badge", spec = NotEqual.class),
13+
@Spec(path = "lastName", spec = Equal.class)
14+
}) Specification<Customer> spec) {
15+
16+
return customerRepository.findAll(spec).stream()
17+
.map(CustomerDto::from)
18+
.collect(toList());
19+
}
20+
```
21+
and requests:
22+
* r1: `GET /join-distinct-customers/optionalParams?page=1&size=1` - no filtering
23+
* r2: `GET /join-distinct-customers/optionalParams/?page=1&size=1&lastName=Simpson` - filtering on non-joined columns
24+
25+
* following queries were previously generated:
26+
* r1 - no filtering
27+
```sql
28+
select distinct c1_0.id,c1_0.street,... from customer c1_0 offset ? rows fetch first ? rows only
29+
select distinct count(distinct c1_0.id) from customer c1_0
30+
```
31+
* r2 - filtering on non-joined columns
32+
```sql
33+
select distinct c1_0.id,c1_0.street,... from customer c1_0 where c1_0.last_name=? offset ? rows fetch first ? rows only,
34+
select distinct count(distinct c1_0.id) from customer c1_0 where c1_0.last_name=?
35+
```
36+
* from now, these queries are generated:
37+
* r1 - no filtering
38+
```sql
39+
select c1_0.id,c1_0.street, ... from customer c1_0 offset ? rows fetch first ? rows only;
40+
select count(c1_0.id) from customer c1_0;
41+
```
42+
* r2 - filtering on non-joined columns
43+
```sql
44+
select c1_0.id,c1_0.street, ... from customer c1_0 where c1_0.last_name=? offset ? rows fetch first ? rows only;
45+
select count(c1_0.id) from customer c1_0 where c1_0.last_name=?;
46+
```
47+
148
v3.1.0
249
=======
350
* Json body parameters no longer throws when:

‎src/main/java/net/kaczmarzyk/spring/data/jpa/domain/Join.java‎

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,16 @@
1515
*/
1616
package net.kaczmarzyk.spring.data.jpa.domain;
1717

18+
import jakarta.persistence.criteria.*;
1819
import net.kaczmarzyk.spring.data.jpa.utils.QueryContext;
1920
import org.springframework.data.jpa.domain.Specification;
2021

2122
import java.util.Objects;
22-
23-
import jakarta.persistence.criteria.CriteriaBuilder;
24-
import jakarta.persistence.criteria.CriteriaQuery;
25-
import jakarta.persistence.criteria.JoinType;
26-
import jakarta.persistence.criteria.Predicate;
27-
import jakarta.persistence.criteria.Root;
23+
import java.util.function.Function;
2824

2925
import static net.kaczmarzyk.spring.data.jpa.utils.JoinPathUtils.pathToJoinContainsAlias;
3026
import static net.kaczmarzyk.spring.data.jpa.utils.JoinPathUtils.pathToJoinSplittedByDot;
3127

32-
import java.util.function.Function;
33-
3428
/**
3529
* @author Tomasz Kaczmarzyk
3630
* @author Jakub Radlica
@@ -56,33 +50,35 @@ public Join(QueryContext queryContext, String pathToJoinOn, String alias, JoinTy
5650

5751
@Override
5852
public Predicate toPredicate(Root<T> root, CriteriaQuery<?> query, CriteriaBuilder builder) {
59-
query.distinct(distinctQuery);
60-
6153
if (!pathToJoinContainsAlias(pathToJoinOn)) {
62-
if(!queryContext.existsJoin(alias, root)) {
63-
putValToQueryContext(alias, root, (r) -> r.join(pathToJoinOn, joinType));
64-
}
54+
if (!queryContext.existsJoin(alias, root)) {
55+
putValToQueryContext(alias, root, (r) -> {
56+
query.distinct(distinctQuery);
57+
return r.join(pathToJoinOn, joinType);
58+
});
59+
}
6560
} else {
6661
String[] pathToJoinOnSplittedByDot = pathToJoinSplittedByDot(pathToJoinOn);
6762

6863
String extractedAlias = pathToJoinOnSplittedByDot[0];
69-
64+
7065
if (!queryContext.existsJoin(extractedAlias, root)) {
7166
throw new IllegalArgumentException(
7267
"Join definition with alias: '" + extractedAlias + "' not found! " +
73-
"Make sure that join with the alias '" + extractedAlias +"' is defined before the join with path: '" + pathToJoinOn + "'"
68+
"Make sure that join with the alias '" + extractedAlias + "' is defined before the join with path: '" + pathToJoinOn + "'"
7469
);
7570
}
7671

7772
String extractedPathToJoin = pathToJoinOnSplittedByDot[1];
78-
putValToQueryContext(
79-
alias,
80-
root,
81-
(r) -> {
82-
jakarta.persistence.criteria.Join<?, ?> evaluated = queryContext.getEvaluated(extractedAlias, root);
83-
return evaluated.join(extractedPathToJoin, joinType);
84-
}
85-
);
73+
putValToQueryContext(
74+
alias,
75+
root,
76+
(r) -> {
77+
query.distinct(distinctQuery);
78+
jakarta.persistence.criteria.Join<?, ?> evaluated = queryContext.getEvaluated(extractedAlias, root);
79+
return evaluated.join(extractedPathToJoin, joinType);
80+
}
81+
);
8682
}
8783
return null;
8884
}

‎src/test/java/net/kaczmarzyk/JoinDistinctE2eTest.java‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,19 @@ public Object joinCountSpecification(
5959
.collect(toList());
6060
}
6161

62+
@RequestMapping("/join-distinct/customers/optionalParams")
63+
public Object joinCountSpecificationOptionalParams(
64+
@Join(path = "badges", alias = "b", type = LEFT)
65+
@And({
66+
@Spec(path = "b.badgeType", params = "badge", spec = NotEqual.class),
67+
@Spec(path = "lastName", spec = Equal.class)
68+
}) Specification<Customer> spec, Pageable pageable) {
69+
70+
return customerRepository.findAll(spec, pageable).stream()
71+
.map(CustomerDto::from)
72+
.collect(toList());
73+
}
74+
6275
@RequestMapping("/join-distinct/customers/distinct-false")
6376
public Object joinCountSpecification_distinctAttributeSetToFalse(
6477
@Join(path = "badges", alias = "b", distinct = false, type = LEFT)
@@ -121,7 +134,7 @@ public Object joinCountQuery_distinctAttributeSetToFalse(
121134
}
122135

123136
@Test
124-
public void createsDistinctQueryByDefault() throws Exception {
137+
public void createsDistinctQueryByDefaultWhenJoinIsUsedInFiltering() throws Exception {
125138
mockMvc.perform(get("/join-distinct/customers")
126139
.param("lastName", "Simpson")
127140
.accept(MediaType.APPLICATION_JSON))
@@ -132,11 +145,49 @@ public void createsDistinctQueryByDefault() throws Exception {
132145
.andExpect(jsonPath("$[2].firstName").value("Bart"))
133146
.andExpect(jsonPath("$[3]").doesNotExist());
134147

135-
136148
assertThatInterceptedStatements()
137149
.hasOneClause("distinct");
138150
}
139151

152+
@Test
153+
public void doesNotMarkQueryAsDistinctWhenJoinIsNotBeingEvaluated_requestWithoutAnyParam() throws Exception {
154+
mockMvc.perform(get("/join-distinct/customers/optionalParams?page=1&size=1")
155+
.accept(MediaType.APPLICATION_JSON))
156+
.andExpect(status().isOk())
157+
.andExpect(jsonPath("$").isArray());
158+
159+
assertThatInterceptedStatements()
160+
.doesNotHaveClause("distinct")
161+
.doesNotHaveClause("join");
162+
}
163+
164+
@Test
165+
public void doesNotMarkQueryAsDistinctWhenJoinIsNotBeingEvaluated_requestWithParamForNonJoinedColumn() throws Exception {
166+
mockMvc.perform(get("/join-distinct/customers/optionalParams?page=1&size=1")
167+
.param("lastName", "Simpson")
168+
.accept(MediaType.APPLICATION_JSON))
169+
.andExpect(status().isOk())
170+
.andExpect(jsonPath("$").isArray());
171+
172+
assertThatInterceptedStatements()
173+
.doesNotHaveClause("distinct")
174+
.doesNotHaveClause("join");
175+
}
176+
177+
@Test
178+
public void marksQueryAsDistinctWhenJoinIsBeingEvaluated_requestWithParamForJoinedColumn() throws Exception {
179+
mockMvc.perform(get("/join-distinct/customers/optionalParams")
180+
.param("lastName", "Simpson")
181+
.param("badge", "CyberMondayBadge")
182+
.accept(MediaType.APPLICATION_JSON))
183+
.andExpect(status().isOk())
184+
.andExpect(jsonPath("$").isArray());
185+
186+
assertThatInterceptedStatements()
187+
.hasOneClause("distinct")
188+
.hasOneClause("join");
189+
}
190+
140191
@Test
141192
public void returnsTheDeduplicatedEntitiesDespiteTheFactThatDistinctAttributeIsSetToFalse() throws Exception {
142193
mockMvc.perform(get("/join-distinct/customers/distinct-false")

0 commit comments

Comments
 (0)