Reuse existing FETCH JOINs when creating Expressions from PropertyPath.

We now consider existing fetch joins for attributes when CriteriaQuery has used a fetch join already. This helps Hibernate to e.g. include mandatory fields in the select list when using DISTINCT using fetch joins.

Closes #2756

Author: mp911de

Diff for: spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java

@@ -889,7 +889,7 @@ private static <T> T getAnnotationProperty(Attribute<?, ?> attribute, String pro
 	}
 
 	/**
-	 * Returns an existing join for the given attribute if one already exists or creates a new one if not.
+	 * Returns an existing (fetch) join for the given attribute if one already exists or creates a new one if not.
 	 *
 	 * @param from the {@link From} to get the current joins from.
 	 * @param attribute the {@link Attribute} to look for in the current joins.
@@ -898,6 +898,13 @@ private static <T> T getAnnotationProperty(Attribute<?, ?> attribute, String pro
 	 */
 	private static Join<?, ?> getOrCreateJoin(From<?, ?> from, String attribute, JoinType joinType) {
 
+		for (Fetch<?, ?> fetch : from.getFetches()) {
+
+			if (fetch instanceof Join<?, ?> join && join.getAttribute().getName().equals(attribute)) {
+				return join;
+			}
+		}
+
 		for (Join<?, ?> join : from.getJoins()) {
 
 			if (join.getAttribute().getName().equals(attribute)) {

Diff for: spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EclipseLinkQueryUtilsIntegrationTests.java

@@ -15,11 +15,25 @@
  */
 package org.springframework.data.jpa.repository.query;
 
+import static org.assertj.core.api.Assertions.*;
+
+import jakarta.persistence.criteria.CriteriaBuilder;
+import jakarta.persistence.criteria.CriteriaQuery;
+import jakarta.persistence.criteria.Path;
+import jakarta.persistence.criteria.Root;
+
+import org.junit.jupiter.api.Test;
+
+import org.springframework.data.jpa.domain.sample.User;
+import org.springframework.data.mapping.PropertyPath;
 import org.springframework.test.context.ContextConfiguration;
 
 /**
+ * EclipseLink variant of {@link QueryUtilsIntegrationTests}.
+ *
  * @author Oliver Gierke
  * @author Jens Schauder
+ * @author Mark Paluch
  */
 @ContextConfiguration("classpath:eclipselink.xml")
 class EclipseLinkQueryUtilsIntegrationTests extends QueryUtilsIntegrationTests {
@@ -28,4 +42,25 @@ int getNumberOfJoinsAfterCreatingAPath() {
 		return 1;
 	}
 
+	@Test // GH-2756
+	@Override
+	void prefersFetchOverJoin() {
+
+		CriteriaBuilder builder = em.getCriteriaBuilder();
+		CriteriaQuery<User> query = builder.createQuery(User.class);
+		Root<User> from = query.from(User.class);
+		from.fetch("manager");
+		from.join("manager");
+
+		PropertyPath managerFirstname = PropertyPath.from("manager.firstname", User.class);
+		PropertyPath managerLastname = PropertyPath.from("manager.lastname", User.class);
+
+		QueryUtils.toExpressionRecursively(from, managerLastname);
+		Path<Object> expr = (Path) QueryUtils.toExpressionRecursively(from, managerFirstname);
+
+		assertThat(expr.getParentPath()).hasFieldOrPropertyWithValue("isFetch", true);
+		assertThat(from.getFetches()).hasSize(1);
+		assertThat(from.getJoins()).hasSize(1);
+	}
+
 }

Diff for: spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java

@@ -32,6 +32,7 @@
 import jakarta.persistence.criteria.From;
 import jakarta.persistence.criteria.Join;
 import jakarta.persistence.criteria.JoinType;
+import jakarta.persistence.criteria.Path;
 import jakarta.persistence.criteria.Root;
 import jakarta.persistence.spi.PersistenceProvider;
 import jakarta.persistence.spi.PersistenceProviderResolver;
@@ -91,6 +92,44 @@ void reusesExistingJoinForExpression() {
 		assertThat(from.getJoins()).hasSize(1);
 	}
 
+	@Test // GH-2756
+	void reusesExistingFetchJoinForExpression() {
+
+		CriteriaBuilder builder = em.getCriteriaBuilder();
+		CriteriaQuery<User> query = builder.createQuery(User.class);
+		Root<User> from = query.from(User.class);
+		from.fetch("manager");
+
+		PropertyPath managerFirstname = PropertyPath.from("manager.firstname", User.class);
+		PropertyPath managerLastname = PropertyPath.from("manager.lastname", User.class);
+
+		QueryUtils.toExpressionRecursively(from, managerLastname);
+		QueryUtils.toExpressionRecursively(from, managerFirstname);
+
+		assertThat(from.getFetches()).hasSize(1);
+		assertThat(from.getJoins()).isEmpty();
+	}
+
+	@Test // GH-2756
+	void prefersFetchOverJoin() {
+
+		CriteriaBuilder builder = em.getCriteriaBuilder();
+		CriteriaQuery<User> query = builder.createQuery(User.class);
+		Root<User> from = query.from(User.class);
+		from.fetch("manager");
+		from.join("manager");
+
+		PropertyPath managerFirstname = PropertyPath.from("manager.firstname", User.class);
+		PropertyPath managerLastname = PropertyPath.from("manager.lastname", User.class);
+
+		QueryUtils.toExpressionRecursively(from, managerLastname);
+		Path<Object> expr = (Path<Object>) QueryUtils.toExpressionRecursively(from, managerFirstname);
+
+		assertThat(expr.getParentPath()).hasFieldOrPropertyWithValue("fetched", true);
+		assertThat(from.getFetches()).hasSize(1);
+		assertThat(from.getJoins()).hasSize(1);
+	}
+
 	@Test // DATAJPA-401, DATAJPA-1238
 	void createsJoinForNavigationAcrossOptionalAssociation() {