Skip to content

Commit 2884539

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

File tree

10 files changed

+427
-35
lines changed

10 files changed

+427
-35
lines changed

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

Lines changed: 44 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import java.lang.reflect.AnnotatedElement;
4343
import java.lang.reflect.Member;
4444
import java.util.*;
45+
import java.util.function.Supplier;
4546
import java.util.regex.Matcher;
4647
import java.util.regex.Pattern;
4748
import java.util.stream.Collectors;
@@ -88,6 +89,7 @@
8889
* @author Eduard Dudar
8990
* @author Yanming Zhou
9091
* @author Alim Naizabek
92+
* @author Jakub Soltys
9193
*/
9294
public abstract class QueryUtils {
9395

@@ -773,11 +775,17 @@ static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath p
773775

774776
boolean isLeafProperty = !property.hasNext();
775777

776-
boolean requiresOuterJoin = requiresOuterJoin(from, property, isForSelection, hasRequiredOuterJoin);
778+
boolean isRelationshipId = isRelationshipId(from, property);
779+
boolean requiresOuterJoin = requiresOuterJoin(from, property, isForSelection, hasRequiredOuterJoin, isLeafProperty, isRelationshipId);
777780

778-
// if it does not require an outer join and is a leaf, simply get the segment
779-
if (!requiresOuterJoin && isLeafProperty) {
780-
return from.get(segment);
781+
// if it does not require an outer join and is a leaf or relationship id, simply get rest of the segment path
782+
if (!requiresOuterJoin && (isLeafProperty || isRelationshipId)) {
783+
Path<T> trailingPath = from.get(segment);
784+
while (property.hasNext()) {
785+
property = property.next();
786+
trailingPath = trailingPath.get(property.getSegment());
787+
}
788+
return trailingPath;
781789
}
782790

783791
// get or create the join
@@ -806,10 +814,12 @@ static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath p
806814
* to generate an explicit outer join in order to prevent Hibernate to use an inner join instead. see
807815
* https://hibernate.atlassian.net/browse/HHH-12999
808816
* @param hasRequiredOuterJoin has a parent already required an outer join?
817+
* @param isLeafProperty is leaf property
818+
* @param isRelationshipId whether property path refers to relationship id
809819
* @return whether an outer join is to be used for integrating this attribute in a query.
810820
*/
811821
static boolean requiresOuterJoin(From<?, ?> from, PropertyPath property, boolean isForSelection,
812-
boolean hasRequiredOuterJoin) {
822+
boolean hasRequiredOuterJoin, boolean isLeafProperty, boolean isRelationshipId) {
813823

814824
// already inner joined so outer join is useless
815825
if (isAlreadyInnerJoined(from, property.getSegment())) {
@@ -818,14 +828,7 @@ static boolean requiresOuterJoin(From<?, ?> from, PropertyPath property, boolean
818828

819829
Bindable<?> model = from.getModel();
820830
ManagedType<?> managedType = getManagedTypeForModel(model);
821-
Bindable<?> propertyPathModel = getModelForPath(property, managedType, from);
822-
823-
// is the attribute of Collection type?
824-
boolean isPluralAttribute = model instanceof PluralAttribute;
825-
826-
if (propertyPathModel == null && isPluralAttribute) {
827-
return true;
828-
}
831+
Bindable<?> propertyPathModel = getModelForPath(property, managedType, () -> from);
829832

830833
if (!(propertyPathModel instanceof Attribute<?, ?> attribute)) {
831834
return false;
@@ -843,14 +846,36 @@ static boolean requiresOuterJoin(From<?, ?> from, PropertyPath property, boolean
843846
boolean isInverseOptionalOneToOne = ONE_TO_ONE == attribute.getPersistentAttributeType()
844847
&& StringUtils.hasText(getAnnotationProperty(attribute, "mappedBy", ""));
845848

846-
boolean isLeafProperty = !property.hasNext();
847-
if (isLeafProperty && !isForSelection && !isCollection && !isInverseOptionalOneToOne && !hasRequiredOuterJoin) {
849+
if ((isLeafProperty || isRelationshipId) && !isForSelection && !isCollection && !isInverseOptionalOneToOne && !hasRequiredOuterJoin) {
848850
return false;
849851
}
850852

851853
return hasRequiredOuterJoin || getAnnotationProperty(attribute, "optional", true);
852854
}
853855

856+
/**
857+
* Checks if this property path is referencing to relationship id.
858+
*
859+
* @param from the {@link From} to check for attribute model.
860+
* @param property the property path
861+
* @return whether in a query is relationship id.
862+
*/
863+
static boolean isRelationshipId(From<?, ?> from, PropertyPath property) {
864+
if (!property.hasNext()) {
865+
return false;
866+
}
867+
868+
Bindable<?> model = from.getModel();
869+
ManagedType<?> managedType = getManagedTypeForModel(model);
870+
Bindable<?> propertyPathModel = getModelForPath(property, managedType, () -> from);
871+
ManagedType<?> propertyPathManagedType = getManagedTypeForModel(propertyPathModel);
872+
Bindable<?> nextPropertyPathModel = getModelForPath(property.next(), propertyPathManagedType, () -> from.get(property.getSegment()));
873+
if (nextPropertyPathModel instanceof SingularAttribute<?, ?>) {
874+
return ((SingularAttribute<?, ?>) nextPropertyPathModel).isId();
875+
}
876+
return false;
877+
}
878+
854879
@SuppressWarnings("unchecked")
855880
static <T> T getAnnotationProperty(Attribute<?, ?> attribute, String propertyName, T defaultValue) {
856881

@@ -954,14 +979,14 @@ static void checkSortExpression(Order order) {
954979
* @param path the current {@link PropertyPath} segment.
955980
* @param managedType primary source for the resulting {@link Bindable}. Can be {@literal null}.
956981
* @param fallback must not be {@literal null}.
957-
* @return the corresponding {@link Bindable} of {@literal null}.
982+
* @return the corresponding {@link Bindable}.
958983
* @see <a href=
959984
* "https://hibernate.atlassian.net/browse/HHH-16144">https://hibernate.atlassian.net/browse/HHH-16144</a>
960985
* @see <a href=
961986
* "https://github.com/jakartaee/persistence/issues/562">https://github.com/jakartaee/persistence/issues/562</a>
962987
*/
963-
private static @Nullable Bindable<?> getModelForPath(PropertyPath path, @Nullable ManagedType<?> managedType,
964-
Path<?> fallback) {
988+
private static Bindable<?> getModelForPath(PropertyPath path, @Nullable ManagedType<?> managedType,
989+
Supplier<Path<?>> fallback) {
965990

966991
String segment = path.getSegment();
967992
if (managedType != null) {
@@ -972,7 +997,7 @@ static void checkSortExpression(Order order) {
972997
}
973998
}
974999

975-
return fallback.get(segment).getModel();
1000+
return fallback.get().get(segment).getModel();
9761001
}
9771002

9781003
/**
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+
}

0 commit comments

Comments
 (0)