Skip to content

Commit 151f045

Browse files
committed
Remove unnecessary join when filtering on relationship id
Closes #3349 Signed-off-by: Jakub Soltys <[email protected]>
1 parent 46453bc commit 151f045

File tree

5 files changed

+161
-1
lines changed

5 files changed

+161
-1
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 attribute model.
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,44 @@
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+
19+
import jakarta.persistence.Entity;
20+
import jakarta.persistence.Id;
21+
import jakarta.persistence.ManyToOne;
22+
23+
@Entity
24+
public class ReferencingEmbeddedIdExampleEmployee {
25+
26+
@Id private long id;
27+
@ManyToOne private EmbeddedIdExampleEmployee employee;
28+
29+
public long getId() {
30+
return id;
31+
}
32+
33+
public void setId(long id) {
34+
this.id = id;
35+
}
36+
37+
public EmbeddedIdExampleEmployee getEmployee() {
38+
return employee;
39+
}
40+
41+
public void setEmployee(EmbeddedIdExampleEmployee employee) {
42+
this.employee = employee;
43+
}
44+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
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+
19+
import jakarta.persistence.Entity;
20+
import jakarta.persistence.Id;
21+
import jakarta.persistence.ManyToOne;
22+
23+
@Entity
24+
public class ReferencingIdClassExampleEmployee {
25+
26+
@Id private long id;
27+
@ManyToOne private IdClassExampleEmployee employee;
28+
29+
public long getId() {
30+
return id;
31+
}
32+
33+
public void setId(long id) {
34+
this.id = id;
35+
}
36+
37+
public IdClassExampleEmployee getEmployee() {
38+
return employee;
39+
}
40+
41+
public void setEmployee(IdClassExampleEmployee employee) {
42+
this.employee = employee;
43+
}
44+
}

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

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import jakarta.persistence.criteria.JoinType;
3535
import jakarta.persistence.criteria.Nulls;
3636
import jakarta.persistence.criteria.Path;
37-
import jakarta.persistence.criteria.Nulls;
3837
import jakarta.persistence.criteria.Root;
3938
import jakarta.persistence.spi.PersistenceProvider;
4039
import jakarta.persistence.spi.PersistenceProviderResolver;
@@ -56,6 +55,8 @@
5655
import org.springframework.data.jpa.domain.sample.Invoice;
5756
import org.springframework.data.jpa.domain.sample.InvoiceItem;
5857
import org.springframework.data.jpa.domain.sample.Order;
58+
import org.springframework.data.jpa.domain.sample.ReferencingEmbeddedIdExampleEmployee;
59+
import org.springframework.data.jpa.domain.sample.ReferencingIdClassExampleEmployee;
5960
import org.springframework.data.jpa.domain.sample.User;
6061
import org.springframework.data.jpa.infrastructure.HibernateTestUtils;
6162
import org.springframework.data.mapping.PropertyPath;
@@ -71,6 +72,7 @@
7172
* @author Patrice Blanchardie
7273
* @author Diego Krupitza
7374
* @author Krzysztof Krason
75+
* @author Jakub Soltys
7476
*/
7577
@ExtendWith(SpringExtension.class)
7678
@ContextConfiguration("classpath:infrastructure.xml")
@@ -387,6 +389,45 @@ void demonstrateDifferentBehaviorOfGetJoin() {
387389
assertThat(root.getJoins()).hasSize(getNumberOfJoinsAfterCreatingAPath());
388390
}
389391

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

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)