Skip to content

Commit 2b107bc

Browse files
committed
Created a test reproducing the situation described by the OP.
The test is green. The problem seems to be that root has a "fetch" and a "join" which both end up being translated to a `JOIN` when converted to SQL. See #2253
1 parent 9eeb5a5 commit 2b107bc

File tree

2 files changed

+34
-9
lines changed

2 files changed

+34
-9
lines changed

src/test/java/org/springframework/data/jpa/domain/sample/Product.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88
public class Product {
99

1010
@Id @GeneratedValue private Long id;
11-
1211
public Long getId() {
1312
return id;
1413
}
14+
String name;
1515
}

src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java

+33-8
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static java.util.Collections.*;
1919
import static org.assertj.core.api.Assertions.*;
2020
import static org.mockito.Mockito.*;
21+
import static org.springframework.data.jpa.repository.query.QueryUtils.*;
2122

2223
import java.util.Collections;
2324
import java.util.List;
@@ -38,6 +39,7 @@
3839
import javax.persistence.criteria.From;
3940
import javax.persistence.criteria.Join;
4041
import javax.persistence.criteria.JoinType;
42+
import javax.persistence.criteria.Predicate;
4143
import javax.persistence.criteria.Root;
4244
import javax.persistence.spi.PersistenceProvider;
4345
import javax.persistence.spi.PersistenceProviderResolver;
@@ -46,7 +48,6 @@
4648
import org.junit.jupiter.api.Test;
4749
import org.junit.jupiter.api.extension.ExtendWith;
4850
import org.mockito.Mockito;
49-
5051
import org.springframework.data.domain.Sort;
5152
import org.springframework.data.domain.Sort.Direction;
5253
import org.springframework.data.jpa.domain.sample.Category;
@@ -139,8 +140,8 @@ void createsLeftJoinForNonOptionalToOneWithNestedOptional() {
139140
CriteriaQuery<InvoiceItem> query = builder.createQuery(InvoiceItem.class);
140141
Root<InvoiceItem> root = query.from(InvoiceItem.class);
141142

142-
QueryUtils
143-
.toExpressionRecursively(root, PropertyPath.from("invoice.order.customer.name", InvoiceItem.class), false);
143+
QueryUtils.toExpressionRecursively(root, PropertyPath.from("invoice.order.customer.name", InvoiceItem.class),
144+
false);
144145

145146
assertThat(getInnerJoins(root)).hasSize(1); // join invoice
146147
Join<?, ?> rootInnerJoin = getInnerJoins(root).iterator().next();
@@ -162,8 +163,8 @@ void reusesLeftJoinForNonOptionalToOneWithNestedOptional() {
162163
root.join("invoice", JoinType.LEFT).join("order", JoinType.LEFT);
163164

164165
// when navigating through a path with nested optionals
165-
QueryUtils
166-
.toExpressionRecursively(root, PropertyPath.from("invoice.order.customer.name", InvoiceItem.class), false);
166+
QueryUtils.toExpressionRecursively(root, PropertyPath.from("invoice.order.customer.name", InvoiceItem.class),
167+
false);
167168

168169
// assert that existing joins are reused and no additional joins are created
169170
assertThat(getInnerJoins(root)).isEmpty(); // no inner join invoice
@@ -185,8 +186,8 @@ void reusesInnerJoinForNonOptionalToOneWithNestedOptional() {
185186
// given an existing inner join an nested optional
186187
root.join("invoice").join("order");
187188

188-
QueryUtils
189-
.toExpressionRecursively(root, PropertyPath.from("invoice.order.customer.name", InvoiceItem.class), false);
189+
QueryUtils.toExpressionRecursively(root, PropertyPath.from("invoice.order.customer.name", InvoiceItem.class),
190+
false);
190191

191192
// assert that no useless left joins are created
192193
assertThat(getInnerJoins(root)).hasSize(1); // join invoice
@@ -307,7 +308,7 @@ void toOrdersCanSortByJoinColumn() {
307308

308309
Sort sort = Sort.by(Direction.ASC, "age");
309310

310-
List<javax.persistence.criteria.Order> orders = QueryUtils.toOrders(sort, join, builder);
311+
List<javax.persistence.criteria.Order> orders = toOrders(sort, join, builder);
311312

312313
assertThat(orders).hasSize(1);
313314
}
@@ -329,6 +330,29 @@ void demonstrateDifferentBehavorOfGetJoin() {
329330
assertThat(root.getJoins()).hasSize(getNumberOfJoinsAfterCreatingAPath());
330331
}
331332

333+
@Test // GH-2253
334+
void orderByMustReuseAJoin() {
335+
336+
CriteriaBuilder builder = em.getCriteriaBuilder();
337+
CriteriaQuery<Category> query = builder.createQuery(Category.class);
338+
Root<Category> root = query.from(Category.class);
339+
340+
// this represents what is done by the specification in the issue
341+
//
342+
query.distinct(true); // this does not belong into a specification
343+
root.fetch("product", JoinType.LEFT);
344+
final Predicate idEquals1 = builder.equal(root.get("id"), 1L);
345+
query.where(idEquals1);
346+
347+
// Also order by a field of the entity referenced by the fetch from the "Specification"
348+
Sort sort = Sort.by(Direction.ASC, "product.name");
349+
350+
query.orderBy(toOrders(sort, root, builder));
351+
352+
assertThat(root.getJoins()).hasSize(1);
353+
354+
}
355+
332356
int getNumberOfJoinsAfterCreatingAPath() {
333357
return 0;
334358
}
@@ -357,6 +381,7 @@ static class Merchant {
357381
@SuppressWarnings("unused")
358382
static class Address {
359383
@Id String id;
384+
String name;
360385
@OneToOne(mappedBy = "address") Merchant merchant;
361386
}
362387

0 commit comments

Comments
 (0)