Remove unnecessary join when filtering on relationship id

Closes #3349

Signed-off-by: Jakub Soltys <jsodpad@gmail.com>

Author: bukajsytlos

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

@@ -42,6 +42,7 @@
 import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.Member;
 import java.util.*;
+import java.util.function.Supplier;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
@@ -88,6 +89,7 @@
  * @author Eduard Dudar
  * @author Yanming Zhou
  * @author Alim Naizabek
+ * @author Jakub Soltys
  */
 public abstract class QueryUtils {
 
@@ -773,11 +775,17 @@ static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath p
 
 		boolean isLeafProperty = !property.hasNext();
 
-		boolean requiresOuterJoin = requiresOuterJoin(from, property, isForSelection, hasRequiredOuterJoin);
+		boolean isRelationshipId = isRelationshipId(from, property);
+		boolean requiresOuterJoin = requiresOuterJoin(from, property, isForSelection, hasRequiredOuterJoin, isLeafProperty, isRelationshipId);
 
-		// if it does not require an outer join and is a leaf, simply get the segment
-		if (!requiresOuterJoin && isLeafProperty) {
-			return from.get(segment);
+		// if it does not require an outer join and is a leaf or relationship id, simply get rest of the segment path
+		if (!requiresOuterJoin && (isLeafProperty || isRelationshipId)) {
+			Path<T> trailingPath = from.get(segment);
+			while (property.hasNext()) {
+				property = property.next();
+				trailingPath = trailingPath.get(property.getSegment());
+			}
+			return trailingPath;
 		}
 
 		// get or create the join
@@ -806,10 +814,12 @@ static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath p
 	 *          to generate an explicit outer join in order to prevent Hibernate to use an inner join instead. see
 	 *          https://hibernate.atlassian.net/browse/HHH-12999
 	 * @param hasRequiredOuterJoin has a parent already required an outer join?
+	 * @param isLeafProperty is leaf property
+	 * @param isRelationshipId whether property path refers to relationship id
 	 * @return whether an outer join is to be used for integrating this attribute in a query.
 	 */
 	static boolean requiresOuterJoin(From<?, ?> from, PropertyPath property, boolean isForSelection,
-			boolean hasRequiredOuterJoin) {
+			boolean hasRequiredOuterJoin, boolean isLeafProperty, boolean isRelationshipId) {
 
 		// already inner joined so outer join is useless
 		if (isAlreadyInnerJoined(from, property.getSegment())) {
@@ -818,14 +828,7 @@ static boolean requiresOuterJoin(From<?, ?> from, PropertyPath property, boolean
 
 		Bindable<?> model = from.getModel();
 		ManagedType<?> managedType = getManagedTypeForModel(model);
-		Bindable<?> propertyPathModel = getModelForPath(property, managedType, from);
-
-		// is the attribute of Collection type?
-		boolean isPluralAttribute = model instanceof PluralAttribute;
-
-		if (propertyPathModel == null && isPluralAttribute) {
-			return true;
-		}
+		Bindable<?> propertyPathModel = getModelForPath(property, managedType, () -> from);
 
 		if (!(propertyPathModel instanceof Attribute<?, ?> attribute)) {
 			return false;
@@ -843,14 +846,36 @@ static boolean requiresOuterJoin(From<?, ?> from, PropertyPath property, boolean
 		boolean isInverseOptionalOneToOne = ONE_TO_ONE == attribute.getPersistentAttributeType()
 				&& StringUtils.hasText(getAnnotationProperty(attribute, "mappedBy", ""));
 
-		boolean isLeafProperty = !property.hasNext();
-		if (isLeafProperty && !isForSelection && !isCollection && !isInverseOptionalOneToOne && !hasRequiredOuterJoin) {
+		if ((isLeafProperty || isRelationshipId) && !isForSelection && !isCollection && !isInverseOptionalOneToOne && !hasRequiredOuterJoin) {
 			return false;
 		}
 
 		return hasRequiredOuterJoin || getAnnotationProperty(attribute, "optional", true);
 	}
 
+	/**
+	 * Checks if this property path is referencing to relationship id.
+	 *
+	 * @param from the {@link From} to check for attribute model.
+	 * @param property the property path
+	 * @return whether in a query is relationship id.
+	 */
+	static boolean isRelationshipId(From<?, ?> from, PropertyPath property) {
+		if (!property.hasNext()) {
+			return false;
+		}
+
+		Bindable<?> model = from.getModel();
+		ManagedType<?> managedType = getManagedTypeForModel(model);
+		Bindable<?> propertyPathModel = getModelForPath(property, managedType, () -> from);
+		ManagedType<?> propertyPathManagedType = getManagedTypeForModel(propertyPathModel);
+		Bindable<?> nextPropertyPathModel = getModelForPath(property.next(), propertyPathManagedType, () -> from.get(property.getSegment()));
+		if (nextPropertyPathModel instanceof SingularAttribute<?, ?>) {
+			return ((SingularAttribute<?, ?>) nextPropertyPathModel).isId();
+		}
+		return false;
+	}
+
 	@SuppressWarnings("unchecked")
 	static <T> T getAnnotationProperty(Attribute<?, ?> attribute, String propertyName, T defaultValue) {
 
@@ -954,14 +979,14 @@ static void checkSortExpression(Order order) {
 	 * @param path the current {@link PropertyPath} segment.
 	 * @param managedType primary source for the resulting {@link Bindable}. Can be {@literal null}.
 	 * @param fallback must not be {@literal null}.
-	 * @return the corresponding {@link Bindable} of {@literal null}.
+	 * @return the corresponding {@link Bindable}.
 	 * @see <a href=
 	 *      "https://hibernate.atlassian.net/browse/HHH-16144">https://hibernate.atlassian.net/browse/HHH-16144</a>
 	 * @see <a href=
 	 *      "https://github.com/jakartaee/persistence/issues/562">https://github.com/jakartaee/persistence/issues/562</a>
 	 */
-	private static @Nullable Bindable<?> getModelForPath(PropertyPath path, @Nullable ManagedType<?> managedType,
-			Path<?> fallback) {
+	private static Bindable<?> getModelForPath(PropertyPath path, @Nullable ManagedType<?> managedType,
+			Supplier<Path<?>> fallback) {
 
 		String segment = path.getSegment();
 		if (managedType != null) {
@@ -972,7 +997,7 @@ static void checkSortExpression(Order order) {
 			}
 		}
 
-		return fallback.get(segment).getModel();
+		return fallback.get().get(segment).getModel();
 	}
 
 	/**

spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/ReferencingEmbeddedIdExampleEmployee.java

@@ -0,0 +1,44 @@
+/*
+ * Copyright 2013-2025 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.jpa.domain.sample;
+
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import jakarta.persistence.ManyToOne;
+
+@Entity
+public class ReferencingEmbeddedIdExampleEmployee {
+
+	@Id private long id;
+	@ManyToOne private EmbeddedIdExampleEmployee employee;
+
+	public long getId() {
+		return id;
+	}
+
+	public void setId(long id) {
+		this.id = id;
+	}
+
+	public EmbeddedIdExampleEmployee getEmployee() {
+		return employee;
+	}
+
+	public void setEmployee(EmbeddedIdExampleEmployee employee) {
+		this.employee = employee;
+	}
+}

spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/ReferencingIdClassExampleEmployee.java

@@ -0,0 +1,44 @@
+/*
+ * Copyright 2013-2025 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.jpa.domain.sample;
+
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import jakarta.persistence.ManyToOne;
+
+@Entity
+public class ReferencingIdClassExampleEmployee {
+
+	@Id private long id;
+	@ManyToOne private IdClassExampleEmployee employee;
+
+	public long getId() {
+		return id;
+	}
+
+	public void setId(long id) {
+		this.id = id;
+	}
+
+	public IdClassExampleEmployee getEmployee() {
+		return employee;
+	}
+
+	public void setEmployee(IdClassExampleEmployee employee) {
+		this.employee = employee;
+	}
+}

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/RepositoryWithCompositeKeyTests.java

@@ -39,6 +39,8 @@
 import org.springframework.data.jpa.domain.sample.QIdClassExampleEmployee;
 import org.springframework.data.jpa.repository.sample.EmployeeRepositoryWithEmbeddedId;
 import org.springframework.data.jpa.repository.sample.EmployeeRepositoryWithIdClass;
+import org.springframework.data.jpa.repository.sample.ReferencingEmployeeRepositoryWithEmbeddedIdRepository;
+import org.springframework.data.jpa.repository.sample.ReferencingEmployeeRepositoryWithIdClassRepository;
 import org.springframework.data.jpa.repository.sample.SampleConfig;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit.jupiter.SpringExtension;
@@ -61,6 +63,10 @@ class RepositoryWithCompositeKeyTests {
 
 	@Autowired EmployeeRepositoryWithIdClass employeeRepositoryWithIdClass;
 	@Autowired EmployeeRepositoryWithEmbeddedId employeeRepositoryWithEmbeddedId;
+	@Autowired
+	ReferencingEmployeeRepositoryWithEmbeddedIdRepository referencingEmployeeRepositoryWithEmbeddedIdRepository;
+	@Autowired
+	ReferencingEmployeeRepositoryWithIdClassRepository referencingEmployeeRepositoryWithIdClassRepository;
 	@Autowired EntityManager em;
 
 	/**
@@ -360,4 +366,148 @@ void shouldExecuteExistsQueryForEntitiesWithCompoundIdClassKeys() {
 		assertThat(employeeRepositoryWithIdClass.existsByName(emp1.getName())).isTrue();
 		assertThat(employeeRepositoryWithIdClass.existsByName("Walter")).isFalse();
 	}
+
+	@Test // GH-3349
+	void findByRelationshipPartialEmbeddedId() {
+
+		EmbeddedIdExampleDepartment dep1 = new EmbeddedIdExampleDepartment();
+		dep1.setDepartmentId(1L);
+		dep1.setName("Dep1");
+
+		EmbeddedIdExampleDepartment dep2 = new EmbeddedIdExampleDepartment();
+		dep2.setDepartmentId(2L);
+		dep2.setName("Dep2");
+
+		EmbeddedIdExampleEmployee emp1 = new EmbeddedIdExampleEmployee();
+		emp1.setEmployeePk(new EmbeddedIdExampleEmployeePK(1L, 1L));
+		emp1.setDepartment(dep1);
+		emp1 = employeeRepositoryWithEmbeddedId.save(emp1);
+
+		EmbeddedIdExampleEmployee emp2 = new EmbeddedIdExampleEmployee();
+		emp2.setEmployeePk(new EmbeddedIdExampleEmployeePK(1L, 2L));
+		emp2.setDepartment(dep2);
+		emp2 = employeeRepositoryWithEmbeddedId.save(emp2);
+
+		ReferencingEmbeddedIdExampleEmployee refEmp1 = new ReferencingEmbeddedIdExampleEmployee();
+		refEmp1.setId(1L);
+		refEmp1.setEmployee(emp1);
+		refEmp1 = referencingEmployeeRepositoryWithEmbeddedIdRepository.save(refEmp1);
+
+		ReferencingEmbeddedIdExampleEmployee refEmp2 = new ReferencingEmbeddedIdExampleEmployee();
+		refEmp2.setId(2L);
+		refEmp2.setEmployee(emp2);
+		refEmp2 = referencingEmployeeRepositoryWithEmbeddedIdRepository.save(refEmp2);
+
+		List<ReferencingEmbeddedIdExampleEmployee> result = referencingEmployeeRepositoryWithEmbeddedIdRepository.findByEmployee_EmployeePk_employeeId(1L);
+
+		assertThat(result).isNotNull();
+		assertThat(result).hasSize(2);
+		assertThat(result).containsOnly(refEmp1, refEmp2);
+
+		List<ReferencingEmbeddedIdExampleEmployee> result2 = referencingEmployeeRepositoryWithEmbeddedIdRepository.findByEmployee_EmployeePk_DepartmentId(2L);
+
+		assertThat(result2).isNotNull();
+		assertThat(result2).hasSize(1);
+		assertThat(result2).containsOnly(refEmp2);
+	}
+
+	@Test // GH-3349
+	void findByRelationshipPartialIdClass() {
+
+		IdClassExampleDepartment dep1 = new IdClassExampleDepartment();
+		dep1.setDepartmentId(1L);
+		dep1.setName("Dep1");
+
+		IdClassExampleDepartment dep2 = new IdClassExampleDepartment();
+		dep2.setDepartmentId(2L);
+		dep2.setName("Dep2");
+
+		IdClassExampleEmployee emp1 = new IdClassExampleEmployee();
+		emp1.setEmpId(1L);
+		emp1.setDepartment(dep1);
+		emp1 = employeeRepositoryWithIdClass.save(emp1);
+
+		IdClassExampleEmployee emp2 = new IdClassExampleEmployee();
+		emp2.setEmpId(1L);
+		emp2.setDepartment(dep2);
+		emp2 = employeeRepositoryWithIdClass.save(emp2);
+
+		ReferencingIdClassExampleEmployee refEmp1 = new ReferencingIdClassExampleEmployee();
+		refEmp1.setId(1L);
+		refEmp1.setEmployee(emp1);
+		refEmp1 = referencingEmployeeRepositoryWithIdClassRepository.save(refEmp1);
+
+		ReferencingIdClassExampleEmployee refEmp2 = new ReferencingIdClassExampleEmployee();
+		refEmp2.setId(2L);
+		refEmp2.setEmployee(emp2);
+		refEmp2 = referencingEmployeeRepositoryWithIdClassRepository.save(refEmp2);
+
+		List<ReferencingIdClassExampleEmployee> result = referencingEmployeeRepositoryWithIdClassRepository.findByEmployee_EmpId(1L);
+
+		assertThat(result).isNotNull();
+		assertThat(result).hasSize(2);
+		assertThat(result).containsOnly(refEmp1, refEmp2);
+
+		List<ReferencingIdClassExampleEmployee> result2 = referencingEmployeeRepositoryWithIdClassRepository.findByEmployee_Department_DepartmentId(2L);
+
+		assertThat(result2).isNotNull();
+		assertThat(result2).hasSize(1);
+		assertThat(result2).containsOnly(refEmp2);
+	}
+
+	@Test
+	void findByPartialRelationshipIdClass() {
+
+		IdClassExampleDepartment dep1 = new IdClassExampleDepartment();
+		dep1.setDepartmentId(1L);
+		dep1.setName("Dep1");
+
+		IdClassExampleDepartment dep2 = new IdClassExampleDepartment();
+		dep2.setDepartmentId(2L);
+		dep2.setName("Dep2");
+
+		IdClassExampleEmployee emp1 = new IdClassExampleEmployee();
+		emp1.setEmpId(1L);
+		emp1.setDepartment(dep1);
+		emp1 = employeeRepositoryWithIdClass.save(emp1);
+
+		IdClassExampleEmployee emp2 = new IdClassExampleEmployee();
+		emp2.setEmpId(1L);
+		emp2.setDepartment(dep2);
+		employeeRepositoryWithIdClass.save(emp2);
+
+		List<IdClassExampleEmployee> result = employeeRepositoryWithIdClass.findAllByDepartment_DepartmentId(1L);
+
+		assertThat(result).isNotNull();
+		assertThat(result).hasSize(1);
+		assertThat(result).containsOnly(emp1);
+	}
+
+	@Test
+	void findByPartialDirectIdClass() {
+
+		IdClassExampleDepartment dep1 = new IdClassExampleDepartment();
+		dep1.setDepartmentId(1L);
+		dep1.setName("Dep1");
+
+		IdClassExampleDepartment dep2 = new IdClassExampleDepartment();
+		dep2.setDepartmentId(2L);
+		dep2.setName("Dep2");
+
+		IdClassExampleEmployee emp1 = new IdClassExampleEmployee();
+		emp1.setEmpId(1L);
+		emp1.setDepartment(dep1);
+		emp1 = employeeRepositoryWithIdClass.save(emp1);
+
+		IdClassExampleEmployee emp2 = new IdClassExampleEmployee();
+		emp2.setEmpId(1L);
+		emp2.setDepartment(dep2);
+		emp2 = employeeRepositoryWithIdClass.save(emp2);
+
+		List<IdClassExampleEmployee> result = employeeRepositoryWithIdClass.findAllByEmpId(1L);
+
+		assertThat(result).isNotNull();
+		assertThat(result).hasSize(2);
+		assertThat(result).containsOnly(emp1, emp2);
+	}
 }