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/query/EclipseLinkQueryUtilsIntegrationTests.java

@@ -63,4 +63,22 @@ void prefersFetchOverJoin() {
 		assertThat(from.getJoins()).hasSize(1);
 	}
 
+
+	@Test // GH-3349
+	@Override
+	void doesNotCreateJoinForRelationshipSimpleId() {
+		//eclipse link produces join for path.get(relationship)
+	}
+
+	@Test // GH-3349
+	@Override
+	void doesNotCreateJoinForRelationshipEmbeddedId() {
+		//eclipse link produces join for path.get(relationship)
+	}
+
+	@Test // GH-3349
+	@Override
+	void doesNotCreateJoinForRelationshipIdClass() {
+		//eclipse link produces join for path.get(relationship)
+	}
 }

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

@@ -34,7 +34,6 @@
 import jakarta.persistence.criteria.JoinType;
 import jakarta.persistence.criteria.Nulls;
 import jakarta.persistence.criteria.Path;
-import jakarta.persistence.criteria.Nulls;
 import jakarta.persistence.criteria.Root;
 import jakarta.persistence.spi.PersistenceProvider;
 import jakarta.persistence.spi.PersistenceProviderResolver;
@@ -56,6 +55,8 @@
 import org.springframework.data.jpa.domain.sample.Invoice;
 import org.springframework.data.jpa.domain.sample.InvoiceItem;
 import org.springframework.data.jpa.domain.sample.Order;
+import org.springframework.data.jpa.domain.sample.ReferencingEmbeddedIdExampleEmployee;
+import org.springframework.data.jpa.domain.sample.ReferencingIdClassExampleEmployee;
 import org.springframework.data.jpa.domain.sample.User;
 import org.springframework.data.jpa.infrastructure.HibernateTestUtils;
 import org.springframework.data.mapping.PropertyPath;
@@ -71,6 +72,7 @@
  * @author Patrice Blanchardie
  * @author Diego Krupitza
  * @author Krzysztof Krason
+ * @author Jakub Soltys
  */
 @ExtendWith(SpringExtension.class)
 @ContextConfiguration("classpath:infrastructure.xml")
@@ -387,6 +389,45 @@ void demonstrateDifferentBehaviorOfGetJoin() {
 		assertThat(root.getJoins()).hasSize(getNumberOfJoinsAfterCreatingAPath());
 	}
 
+	@Test // GH-3349
+	void doesNotCreateJoinForRelationshipSimpleId() {
+
+		CriteriaBuilder builder = em.getCriteriaBuilder();
+		CriteriaQuery<User> query = builder.createQuery(User.class);
+		Root<User> from = query.from(User.class);
+
+		QueryUtils.toExpressionRecursively(from, PropertyPath.from("manager.id", User.class));
+
+		assertThat(from.getFetches()).hasSize(0);
+		assertThat(from.getJoins()).hasSize(0);
+	}
+
+	@Test // GH-3349
+	void doesNotCreateJoinForRelationshipEmbeddedId() {
+
+		CriteriaBuilder builder = em.getCriteriaBuilder();
+		CriteriaQuery<ReferencingEmbeddedIdExampleEmployee> query = builder.createQuery(ReferencingEmbeddedIdExampleEmployee.class);
+		Root<ReferencingEmbeddedIdExampleEmployee> from = query.from(ReferencingEmbeddedIdExampleEmployee.class);
+
+		QueryUtils.toExpressionRecursively(from, PropertyPath.from("employee.employeePk.employeeId", ReferencingEmbeddedIdExampleEmployee.class));
+
+		assertThat(from.getFetches()).hasSize(0);
+		assertThat(from.getJoins()).hasSize(0);
+	}
+
+	@Test // GH-3349
+	void doesNotCreateJoinForRelationshipIdClass() {
+
+		CriteriaBuilder builder = em.getCriteriaBuilder();
+		CriteriaQuery<ReferencingIdClassExampleEmployee> query = builder.createQuery(ReferencingIdClassExampleEmployee.class);
+		Root<ReferencingIdClassExampleEmployee> from = query.from(ReferencingIdClassExampleEmployee.class);
+
+		QueryUtils.toExpressionRecursively(from, PropertyPath.from("employee.empId", ReferencingIdClassExampleEmployee.class));
+
+		assertThat(from.getFetches()).hasSize(0);
+		assertThat(from.getJoins()).hasSize(0);
+	}
+
 	int getNumberOfJoinsAfterCreatingAPath() {
 		return 0;
 	}

spring-data-jpa/src/test/resources/META-INF/persistence.xml

@@ -27,9 +27,11 @@
 		<class>org.springframework.data.jpa.domain.sample.EmbeddedIdExampleEmployeePK</class>
 		<class>org.springframework.data.jpa.domain.sample.EmbeddedIdExampleEmployee</class>
 		<class>org.springframework.data.jpa.domain.sample.EmbeddedIdExampleDepartment</class>
+		<class>org.springframework.data.jpa.domain.sample.ReferencingEmbeddedIdExampleEmployee</class>
 		<class>org.springframework.data.jpa.domain.sample.EmployeeWithName</class>
 		<class>org.springframework.data.jpa.domain.sample.IdClassExampleEmployee</class>
 		<class>org.springframework.data.jpa.domain.sample.IdClassExampleDepartment</class>
+		<class>org.springframework.data.jpa.domain.sample.ReferencingIdClassExampleEmployee</class>
 		<class>org.springframework.data.jpa.domain.sample.Invoice</class>
 		<class>org.springframework.data.jpa.domain.sample.InvoiceItem</class>
 		<class>org.springframework.data.jpa.domain.sample.Item</class>