Skip to content

Commit 817b09f

Browse files
committed
Remove unnecessary join when filtering on relationship id
Closes #3349 Signed-off-by: Jakub Soltys <jsodpad@gmail.com>
1 parent 46453bc commit 817b09f

File tree

5 files changed

+156
-5
lines changed

5 files changed

+156
-5
lines changed

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@
8888
* @author Eduard Dudar
8989
* @author Yanming Zhou
9090
* @author Alim Naizabek
91+
* @author Jakub Soltys
9192
*/
9293
public abstract class QueryUtils {
9394

@@ -780,6 +781,16 @@ static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath p
780781
return from.get(segment);
781782
}
782783

784+
boolean isRelationshipId = isRelationshipId(from, property);
785+
if (isRelationshipId) {
786+
Path<T> relationshipIdPath = from.get(segment);
787+
while (property.hasNext()) {
788+
property = property.next();
789+
relationshipIdPath = relationshipIdPath.get(property.getSegment());
790+
}
791+
return relationshipIdPath;
792+
}
793+
783794
// get or create the join
784795
JoinType joinType = requiresOuterJoin ? JoinType.LEFT : JoinType.INNER;
785796
Join<?, ?> join = getOrCreateJoin(from, segment, joinType);
@@ -851,6 +862,24 @@ static boolean requiresOuterJoin(From<?, ?> from, PropertyPath property, boolean
851862
return hasRequiredOuterJoin || getAnnotationProperty(attribute, "optional", true);
852863
}
853864

865+
/**
866+
* Checks if this property path is pointing to relationship id.
867+
*
868+
* @param from the {@link From} to check for fetches.
869+
* @param property the property path
870+
* @return whether in a query is relationship id.
871+
*/
872+
private static boolean isRelationshipId(From<?, ?> from, PropertyPath property) {
873+
if (!property.hasNext()) {
874+
return false;
875+
}
876+
Bindable<Object> nextSegmentModel = from.get(property.getSegment()).get(property.next().getSegment()).getModel();
877+
if (nextSegmentModel instanceof SingularAttribute<?, ?>) {
878+
return ((SingularAttribute<?, ?>) nextSegmentModel).isId();
879+
}
880+
return false;
881+
}
882+
854883
@SuppressWarnings("unchecked")
855884
static <T> T getAnnotationProperty(Attribute<?, ?> attribute, String propertyName, T defaultValue) {
856885

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
* Copyright 2013-2025 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.jpa.domain.sample;
17+
18+
import jakarta.persistence.*;
19+
20+
@Entity
21+
public class ReferencingEmbeddedIdExampleEmployee {
22+
23+
@Id private long id;
24+
25+
@ManyToOne private//
26+
EmbeddedIdExampleEmployee employee;
27+
28+
public long getId() {
29+
return id;
30+
}
31+
32+
public void setId(long id) {
33+
this.id = id;
34+
}
35+
36+
public EmbeddedIdExampleEmployee getEmployee() {
37+
return employee;
38+
}
39+
40+
public void setEmployee(EmbeddedIdExampleEmployee employee) {
41+
this.employee = employee;
42+
}
43+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Copyright 2013-2025 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.jpa.domain.sample;
17+
18+
import jakarta.persistence.*;
19+
20+
@Entity
21+
public class ReferencingIdClassExampleEmployee {
22+
23+
@Id private long id;
24+
@ManyToOne private IdClassExampleEmployee employee;
25+
26+
public long getId() {
27+
return id;
28+
}
29+
30+
public void setId(long id) {
31+
this.id = id;
32+
}
33+
34+
public IdClassExampleEmployee getEmployee() {
35+
return employee;
36+
}
37+
38+
public void setEmployee(IdClassExampleEmployee employee) {
39+
this.employee = employee;
40+
}
41+
}

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

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,7 @@
5252

5353
import org.springframework.data.domain.Sort;
5454
import org.springframework.data.domain.Sort.Direction;
55-
import org.springframework.data.jpa.domain.sample.Category;
56-
import org.springframework.data.jpa.domain.sample.Invoice;
57-
import org.springframework.data.jpa.domain.sample.InvoiceItem;
58-
import org.springframework.data.jpa.domain.sample.Order;
59-
import org.springframework.data.jpa.domain.sample.User;
55+
import org.springframework.data.jpa.domain.sample.*;
6056
import org.springframework.data.jpa.infrastructure.HibernateTestUtils;
6157
import org.springframework.data.mapping.PropertyPath;
6258
import org.springframework.test.context.ContextConfiguration;
@@ -71,6 +67,7 @@
7167
* @author Patrice Blanchardie
7268
* @author Diego Krupitza
7369
* @author Krzysztof Krason
70+
* @author Jakub Soltys
7471
*/
7572
@ExtendWith(SpringExtension.class)
7673
@ContextConfiguration("classpath:infrastructure.xml")
@@ -387,6 +384,45 @@ void demonstrateDifferentBehaviorOfGetJoin() {
387384
assertThat(root.getJoins()).hasSize(getNumberOfJoinsAfterCreatingAPath());
388385
}
389386

387+
@Test // GH-3349
388+
void doesNotCreateJoinForRelationshipSimpleId() {
389+
390+
CriteriaBuilder builder = em.getCriteriaBuilder();
391+
CriteriaQuery<User> query = builder.createQuery(User.class);
392+
Root<User> from = query.from(User.class);
393+
394+
QueryUtils.toExpressionRecursively(from, PropertyPath.from("manager.id", User.class));
395+
396+
assertThat(from.getFetches()).hasSize(0);
397+
assertThat(from.getJoins()).hasSize(0);
398+
}
399+
400+
@Test // GH-3349
401+
void doesNotCreateJoinForRelationshipEmbeddedId() {
402+
403+
CriteriaBuilder builder = em.getCriteriaBuilder();
404+
CriteriaQuery<ReferencingEmbeddedIdExampleEmployee> query = builder.createQuery(ReferencingEmbeddedIdExampleEmployee.class);
405+
Root<ReferencingEmbeddedIdExampleEmployee> from = query.from(ReferencingEmbeddedIdExampleEmployee.class);
406+
407+
QueryUtils.toExpressionRecursively(from, PropertyPath.from("employee.employeePk.employeeId", ReferencingEmbeddedIdExampleEmployee.class));
408+
409+
assertThat(from.getFetches()).hasSize(0);
410+
assertThat(from.getJoins()).hasSize(0);
411+
}
412+
413+
@Test // GH-3349
414+
void doesNotCreateJoinForRelationshipIdClass() {
415+
416+
CriteriaBuilder builder = em.getCriteriaBuilder();
417+
CriteriaQuery<ReferencingIdClassExampleEmployee> query = builder.createQuery(ReferencingIdClassExampleEmployee.class);
418+
Root<ReferencingIdClassExampleEmployee> from = query.from(ReferencingIdClassExampleEmployee.class);
419+
420+
QueryUtils.toExpressionRecursively(from, PropertyPath.from("employee.empId", ReferencingIdClassExampleEmployee.class));
421+
422+
assertThat(from.getFetches()).hasSize(0);
423+
assertThat(from.getJoins()).hasSize(0);
424+
}
425+
390426
int getNumberOfJoinsAfterCreatingAPath() {
391427
return 0;
392428
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,11 @@
2727
<class>org.springframework.data.jpa.domain.sample.EmbeddedIdExampleEmployeePK</class>
2828
<class>org.springframework.data.jpa.domain.sample.EmbeddedIdExampleEmployee</class>
2929
<class>org.springframework.data.jpa.domain.sample.EmbeddedIdExampleDepartment</class>
30+
<class>org.springframework.data.jpa.domain.sample.ReferencingEmbeddedIdExampleEmployee</class>
3031
<class>org.springframework.data.jpa.domain.sample.EmployeeWithName</class>
3132
<class>org.springframework.data.jpa.domain.sample.IdClassExampleEmployee</class>
3233
<class>org.springframework.data.jpa.domain.sample.IdClassExampleDepartment</class>
34+
<class>org.springframework.data.jpa.domain.sample.ReferencingIdClassExampleEmployee</class>
3335
<class>org.springframework.data.jpa.domain.sample.Invoice</class>
3436
<class>org.springframework.data.jpa.domain.sample.InvoiceItem</class>
3537
<class>org.springframework.data.jpa.domain.sample.Item</class>

0 commit comments

Comments
 (0)