Skip to content

Commit

Permalink
Created a test reproducing the situation described by the OP.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
schauder committed Jul 13, 2021
1 parent 9eeb5a5 commit 2b107bc
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
public class Product {

@Id @GeneratedValue private Long id;

public Long getId() {
return id;
}
String name;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static java.util.Collections.*;
import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;
import static org.springframework.data.jpa.repository.query.QueryUtils.*;

import java.util.Collections;
import java.util.List;
Expand All @@ -38,6 +39,7 @@
import javax.persistence.criteria.From;
import javax.persistence.criteria.Join;
import javax.persistence.criteria.JoinType;
import javax.persistence.criteria.Predicate;
import javax.persistence.criteria.Root;
import javax.persistence.spi.PersistenceProvider;
import javax.persistence.spi.PersistenceProviderResolver;
Expand All @@ -46,7 +48,6 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mockito;

import org.springframework.data.domain.Sort;
import org.springframework.data.domain.Sort.Direction;
import org.springframework.data.jpa.domain.sample.Category;
Expand Down Expand Up @@ -139,8 +140,8 @@ void createsLeftJoinForNonOptionalToOneWithNestedOptional() {
CriteriaQuery<InvoiceItem> query = builder.createQuery(InvoiceItem.class);
Root<InvoiceItem> root = query.from(InvoiceItem.class);

QueryUtils
.toExpressionRecursively(root, PropertyPath.from("invoice.order.customer.name", InvoiceItem.class), false);
QueryUtils.toExpressionRecursively(root, PropertyPath.from("invoice.order.customer.name", InvoiceItem.class),
false);

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

// when navigating through a path with nested optionals
QueryUtils
.toExpressionRecursively(root, PropertyPath.from("invoice.order.customer.name", InvoiceItem.class), false);
QueryUtils.toExpressionRecursively(root, PropertyPath.from("invoice.order.customer.name", InvoiceItem.class),
false);

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

QueryUtils
.toExpressionRecursively(root, PropertyPath.from("invoice.order.customer.name", InvoiceItem.class), false);
QueryUtils.toExpressionRecursively(root, PropertyPath.from("invoice.order.customer.name", InvoiceItem.class),
false);

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

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

List<javax.persistence.criteria.Order> orders = QueryUtils.toOrders(sort, join, builder);
List<javax.persistence.criteria.Order> orders = toOrders(sort, join, builder);

assertThat(orders).hasSize(1);
}
Expand All @@ -329,6 +330,29 @@ void demonstrateDifferentBehavorOfGetJoin() {
assertThat(root.getJoins()).hasSize(getNumberOfJoinsAfterCreatingAPath());
}

@Test // GH-2253
void orderByMustReuseAJoin() {

CriteriaBuilder builder = em.getCriteriaBuilder();
CriteriaQuery<Category> query = builder.createQuery(Category.class);
Root<Category> root = query.from(Category.class);

// this represents what is done by the specification in the issue
//
query.distinct(true); // this does not belong into a specification
root.fetch("product", JoinType.LEFT);
final Predicate idEquals1 = builder.equal(root.get("id"), 1L);
query.where(idEquals1);

// Also order by a field of the entity referenced by the fetch from the "Specification"
Sort sort = Sort.by(Direction.ASC, "product.name");

query.orderBy(toOrders(sort, root, builder));

assertThat(root.getJoins()).hasSize(1);

}

int getNumberOfJoinsAfterCreatingAPath() {
return 0;
}
Expand Down Expand Up @@ -357,6 +381,7 @@ static class Merchant {
@SuppressWarnings("unused")
static class Address {
@Id String id;
String name;
@OneToOne(mappedBy = "address") Merchant merchant;
}

Expand Down

0 comments on commit 2b107bc

Please sign in to comment.