Spring/spring-data-jpa
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Use left outer joins for nested optionals.

Browse Source 
Closes #2111
Original pull request #436
This commit is contained in:
Patrice Blanchardie
2020-11-19 10:36:51 +01:00
committed by Jens Schauder
parent eb023fc77a
commit 32b6163c49
6 changed files with 275 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

175
src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java
View File

@@ -29,6 +29,7 @@ import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -41,16 +42,15 @@ import javax.persistence.Parameter;
import javax.persistence.Query;
import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.criteria.Expression;
import javax.persistence.criteria.Fetch;
import javax.persistence.criteria.From;
import javax.persistence.criteria.Join;
import javax.persistence.criteria.JoinType;
import javax.persistence.criteria.Path;
import javax.persistence.metamodel.Attribute;
import javax.persistence.metamodel.Attribute.PersistentAttributeType;
import javax.persistence.metamodel.Bindable;
import javax.persistence.metamodel.ManagedType;
import javax.persistence.metamodel.PluralAttribute;
import javax.persistence.metamodel.SingularAttribute;
import org.springframework.core.annotation.AnnotationUtils;
import org.springframework.dao.InvalidDataAccessApiUsageException;
@@ -620,47 +620,96 @@ public abstract class QueryUtils {
return toExpressionRecursively(from, property, false);
}
@SuppressWarnings("unchecked")
static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath property, boolean isForSelection) {
return toExpressionRecursively(from, property, isForSelection, false);
}
/**
* Creates an expression with proper inner and left joins by recursively navigating the path
*
* @param from the {@link From}
* @param property the property path
* @param isForSelection is the property navigated for the selection or ordering part of the query?
* @param hasRequiredOuterJoin has a parent already required an outer join?
* @param <T> the type of the expression
* @return the expression
*/
@SuppressWarnings("unchecked") static <T> Expression<T> toExpressionRecursively(From<?, ?> from,
PropertyPath property, boolean isForSelection, boolean hasRequiredOuterJoin) {
String segment = property.getSegment();
boolean isLeafProperty = !property.hasNext();
boolean requiresOuterJoin = requiresOuterJoin(from, property, isForSelection, hasRequiredOuterJoin);
// if it does not require an outer join and is a leaf, simply get the segment
if (!requiresOuterJoin && isLeafProperty) {
return from.get(segment);
}
// get or create the join
JoinType joinType = requiresOuterJoin ? JoinType.LEFT : JoinType.INNER;
Join<?, ?> join = getOrCreateJoin(from, segment, joinType);
// if it's a leaf, return the join
if (isLeafProperty) {
return (Expression<T>) join;
}
PropertyPath nextProperty = Objects.requireNonNull(property.next(), "An element of the property path is null!");
// recurse with the next property
return toExpressionRecursively(join, nextProperty, isForSelection, requiresOuterJoin);
}
/**
* Checks if this attribute requires an outer join.
* This is the case eg. if it hadn't already been fetched with an inner join and if it's an a optional association,
* and if previous paths has already required outer joins.
* It also ensures outer joins are used even when Hibernate defaults to inner joins (HHH-12712 and HHH-12999).
*
* @param from the {@link From} to check for fetches.
* @param property the property path
* @param isForSelection is the property navigated for the selection or ordering part of the query? if true,
* we need 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?
* @return whether an outer join is to be used for integrating this attribute in a query.
*/
private static boolean requiresOuterJoin(From<?, ?> from, PropertyPath property, boolean isForSelection,
boolean hasRequiredOuterJoin) {
String segment = property.getSegment();
// already inner joined so outer join is useless
if (isAlreadyInnerJoined(from, segment))
return false;
Bindable<?> propertyPathModel;
Bindable<?> model = from.getModel();
String segment = property.getSegment();
// required for EclipseLink: we try to avoid using from.get as EclipseLink produces an inner join
// regardless of which join operation is specified next
// see: https://bugs.eclipse.org/bugs/show_bug.cgi?id=413892
// still occurs as of 2.7
ManagedType<?> managedType = null;
if (model instanceof ManagedType) {
/*
* Required to keep support for EclipseLink 2.4.x. TODO: Remove once we drop that (probably Dijkstra M1)
* See: https://bugs.eclipse.org/bugs/show_bug.cgi?id=413892
*/
propertyPathModel = (Bindable<?>) ((ManagedType<?>) model).getAttribute(segment);
managedType = (ManagedType<?>) model;
} else if (model instanceof SingularAttribute
&& ((SingularAttribute<?, ?>) model).getType() instanceof ManagedType) {
managedType = (ManagedType<?>) ((SingularAttribute<?, ?>) model).getType();
}
if (managedType != null) {
propertyPathModel = (Bindable<?>) managedType.getAttribute(segment);
} else {
propertyPathModel = from.get(segment).getModel();
}
if (requiresOuterJoin(propertyPathModel, model instanceof PluralAttribute, !property.hasNext(), isForSelection)
&& !isAlreadyFetched(from, segment)) {
Join<?, ?> join = getOrCreateJoin(from, segment);
return (Expression<T>) (property.hasNext() ? toExpressionRecursively(join, property.next(), isForSelection)
: join);
} else {
Path<Object> path = from.get(segment);
return (Expression<T>) (property.hasNext() ? toExpressionRecursively(path, property.next()) : path);
}
}
// is the attribute of Collection type?
boolean isPluralAttribute = model instanceof PluralAttribute;
/**
* Returns whether the given {@code propertyPathModel} requires the creation of a join. This is the case if we find a
* optional association.
*
* @param propertyPathModel may be {@literal null}.
* @param isPluralAttribute is the attribute of Collection type?
* @param isLeafProperty is this the final property navigated by a {@link PropertyPath}?
* @param isForSelection is the property navigated for the selection part of the query?
* @return whether an outer join is to be used for integrating this attribute in a query.
*/
private static boolean requiresOuterJoin(@Nullable Bindable<?> propertyPathModel, boolean isPluralAttribute,
boolean isLeafProperty, boolean isForSelection) {
boolean isLeafProperty = !property.hasNext();
if (propertyPathModel == null && isPluralAttribute) {
return true;
@@ -672,24 +721,23 @@ public abstract class QueryUtils {
Attribute<?, ?> attribute = (Attribute<?, ?>) propertyPathModel;
// not a persistent attribute type association (@OneToOne, @ManyToOne)
if (!ASSOCIATION_TYPES.containsKey(attribute.getPersistentAttributeType())) {
return false;
}
// if this path is an optional one to one attribute navigated from the not owning side we also need an explicit
// outer join to avoid https://hibernate.atlassian.net/browse/HHH-12712 and
// https://github.com/eclipse-ee4j/jpa-api/issues/170
boolean isCollection = attribute.isCollection();
// if this path is an optional one to one attribute navigated from the not owning side we also need an
// explicit outer join to avoid https://hibernate.atlassian.net/browse/HHH-12712
// and https://github.com/eclipse-ee4j/jpa-api/issues/170
boolean isInverseOptionalOneToOne = PersistentAttributeType.ONE_TO_ONE == attribute.getPersistentAttributeType()
&& StringUtils.hasText(getAnnotationProperty(attribute, "mappedBy", ""));
// if this path is part of the select list we need 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.
if (isLeafProperty && !isForSelection && !attribute.isCollection() && !isInverseOptionalOneToOne) {
if (isLeafProperty && !isForSelection && !isCollection && !isInverseOptionalOneToOne && !hasRequiredOuterJoin) {
return false;
}
return getAnnotationProperty(attribute, "optional", true);
return hasRequiredOuterJoin || getAnnotationProperty(attribute, "optional", true);
}
private static <T> T getAnnotationProperty(Attribute<?, ?> attribute, String propertyName, T defaultValue) {
@@ -710,52 +758,37 @@ public abstract class QueryUtils {
return annotation == null ? defaultValue : (T) AnnotationUtils.getValue(annotation, propertyName);
}
static Expression<Object> toExpressionRecursively(Path<Object> path, PropertyPath property) {
Path<Object> result = path.get(property.getSegment());
return property.hasNext() ? toExpressionRecursively(result, property.next()) : result;
}
/**
* Returns an existing join for the given attribute if one already exists or creates a new one if not.
*
* @param from the {@link From} to get the current joins from.
* @param from the {@link From} to get the current joins from.
* @param attribute the {@link Attribute} to look for in the current joins.
* @param joinType the join type to create if none was found
* @return will never be {@literal null}.
*/
private static Join<?, ?> getOrCreateJoin(From<?, ?> from, String attribute) {
for (Join<?, ?> join : from.getJoins()) {
boolean sameName = join.getAttribute().getName().equals(attribute);
if (sameName && join.getJoinType().equals(JoinType.LEFT)) {
return join;
}
}
return from.join(attribute, JoinType.LEFT);
private static Join<?, ?> getOrCreateJoin(From<?, ?> from, String attribute, JoinType joinType) {
return from.getJoins().stream()
.filter(join -> join.getAttribute().getName().equals(attribute))
.findFirst()
.orElseGet(() -> from.join(attribute, joinType));
}
/**
* Return whether the given {@link From} contains a fetch declaration for the attribute with the given name.
* Return whether the given {@link From} contains an inner join for the attribute with the given name.
*
* @param from the {@link From} to check for fetches.
* @param from the {@link From} to check for joins.
* @param attribute the attribute name to check.
* @return
* @return true if the attribute has already been inner joined
*/
private static boolean isAlreadyFetched(From<?, ?> from, String attribute) {
private static boolean isAlreadyInnerJoined(From<?, ?> from, String attribute) {
for (Fetch<?, ?> fetch : from.getFetches()) {
boolean isInnerJoinFetched = from.getFetches().stream().anyMatch(
fetch -> fetch.getAttribute().getName().equals(attribute) && fetch.getJoinType().equals(JoinType.INNER));
boolean sameName = fetch.getAttribute().getName().equals(attribute);
boolean isSimplyInnerJoined = from.getJoins().stream()
.anyMatch(join -> join.getAttribute().getName().equals(attribute) && join.getJoinType().equals(JoinType.INNER));
if (sameName && fetch.getJoinType().equals(JoinType.LEFT)) {
return true;
}
}
return false;
return isInnerJoinFetched || isSimplyInnerJoined;
}
/**

5
src/test/java/org/springframework/data/jpa/domain/sample/Customer.java
View File

@@ -20,9 +20,12 @@ import javax.persistence.Id;
/**
* @author Oliver Gierke
* @author Patrice Blanchardie
*/
@Entity
public class Customer {
@Id Long id;
@Id Long id;
String name;
}

35
src/test/java/org/springframework/data/jpa/domain/sample/Invoice.java Normal file
View File

@@ -0,0 +1,35 @@
/*
* Copyright 2020 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 javax.persistence.Entity;
import javax.persistence.Id;
import javax.persistence.ManyToOne;
import javax.persistence.Table;
/**
* @author Patrice Blanchardie
*/
@Entity
@Table(name = "INVOICES")
public class Invoice {
@Id Long id;
@ManyToOne(optional = false) Customer customer;
@ManyToOne Order order;
}

33
src/test/java/org/springframework/data/jpa/domain/sample/InvoiceItem.java Normal file
View File

@@ -0,0 +1,33 @@
/*
* Copyright 2020 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 javax.persistence.Entity;
import javax.persistence.Id;
import javax.persistence.ManyToOne;
import javax.persistence.Table;
/**
* @author Patrice Blanchardie
*/
@Entity
@Table(name = "INVOICE_ITEMS")
public class InvoiceItem {
@Id Long id;
@ManyToOne(optional = false) Invoice invoice;
}

101
src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java
View File

@@ -35,6 +35,7 @@ import javax.persistence.Persistence;
import javax.persistence.PersistenceContext;
import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.criteria.CriteriaQuery;
import javax.persistence.criteria.From;
import javax.persistence.criteria.Join;
import javax.persistence.criteria.JoinType;
import javax.persistence.criteria.Root;
@@ -49,6 +50,8 @@ import org.mockito.Mockito;
import org.springframework.data.domain.Sort;
import org.springframework.data.domain.Sort.Direction;
import org.springframework.data.jpa.domain.sample.Category;
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.User;
import org.springframework.data.jpa.infrastructure.HibernateTestUtils;
@@ -61,6 +64,7 @@ import org.springframework.test.context.junit.jupiter.SpringExtension;
*
* @author Oliver Gierke
* @author Sébastien Péralta
* @author Patrice Blanchardie
*/
@ExtendWith(SpringExtension.class)
@ContextConfiguration("classpath:infrastructure.xml")
@@ -111,6 +115,88 @@ public class QueryUtilsIntegrationTests {
});
}
@Test // DATAJPA-1822
void createsLeftJoinForOptionalToOneWithNestedNonOptional() {
CriteriaBuilder builder = em.getCriteriaBuilder();
CriteriaQuery<Invoice> query = builder.createQuery(Invoice.class);
Root<Invoice> root = query.from(Invoice.class);
QueryUtils.toExpressionRecursively(root, PropertyPath.from("order.customer.name", Invoice.class), false);
assertThat(getNonInnerJoins(root)).hasSize(1); // left join order
assertThat(getInnerJoins(root)).isEmpty(); // no inner join order
Join<Invoice, ?> leftJoin = root.getJoins().iterator().next();
assertThat(getNonInnerJoins(leftJoin)).hasSize(1); // left join customer
assertThat(getInnerJoins(leftJoin)).isEmpty(); // no inner join customer
}
@Test // DATAJPA-1822
void createsLeftJoinForNonOptionalToOneWithNestedOptional() {
CriteriaBuilder builder = em.getCriteriaBuilder();
CriteriaQuery<InvoiceItem> query = builder.createQuery(InvoiceItem.class);
Root<InvoiceItem> root = query.from(InvoiceItem.class);
QueryUtils
.toExpressionRecursively(root, PropertyPath.from("invoice.order.customer.name", InvoiceItem.class), false);
assertThat(getInnerJoins(root)).hasSize(1); // join invoice
Join<?, ?> rootInnerJoin = getInnerJoins(root).iterator().next();
assertThat(getNonInnerJoins(rootInnerJoin)).hasSize(1); // left join order
assertThat(getInnerJoins(rootInnerJoin)).isEmpty(); // no inner join order
Join<?, ?> leftJoin = getNonInnerJoins(rootInnerJoin).iterator().next();
assertThat(getNonInnerJoins(leftJoin)).hasSize(1); // left join customer
assertThat(getInnerJoins(leftJoin)).isEmpty(); // no inner join customer
}
@Test // DATAJPA-1822
void reusesLeftJoinForNonOptionalToOneWithNestedOptional() {
CriteriaBuilder builder = em.getCriteriaBuilder();
CriteriaQuery<InvoiceItem> query = builder.createQuery(InvoiceItem.class);
Root<InvoiceItem> root = query.from(InvoiceItem.class);
// given existing left joins
root.join("invoice", JoinType.LEFT).join("order", JoinType.LEFT);
// when navigating through a path with nested optionals
QueryUtils
.toExpressionRecursively(root, PropertyPath.from("invoice.order.customer.name", InvoiceItem.class), false);
// assert that existing joins are reused and no additional joins are created
assertThat(getInnerJoins(root)).isEmpty(); // no inner join invoice
assertThat(getNonInnerJoins(root)).hasSize(1); // reused left join invoice
Join<?, ?> rootInnerJoin = getNonInnerJoins(root).iterator().next();
assertThat(getInnerJoins(rootInnerJoin)).isEmpty(); // no inner join order
assertThat(getNonInnerJoins(rootInnerJoin)).hasSize(1); // reused left join order
Join<?, ?> leftJoin = getNonInnerJoins(rootInnerJoin).iterator().next();
assertThat(getNonInnerJoins(leftJoin)).hasSize(1); // left join customer
}
@Test // DATAJPA-1822
void reusesInnerJoinForNonOptionalToOneWithNestedOptional() {
CriteriaBuilder builder = em.getCriteriaBuilder();
CriteriaQuery<InvoiceItem> query = builder.createQuery(InvoiceItem.class);
Root<InvoiceItem> root = query.from(InvoiceItem.class);
// given an existing inner join an nested optional
root.join("invoice").join("order");
QueryUtils
.toExpressionRecursively(root, PropertyPath.from("invoice.order.customer.name", InvoiceItem.class), false);
// assert that no useless left joins are created
assertThat(getInnerJoins(root)).hasSize(1); // join invoice
Join<?, ?> rootInnerJoin = getInnerJoins(root).iterator().next();
assertThat(getNonInnerJoins(rootInnerJoin)).isEmpty(); // no left join order
assertThat(getInnerJoins(rootInnerJoin)).hasSize(1); // inner join order
Join<?, ?> innerJoin = getInnerJoins(rootInnerJoin).iterator().next();
assertThat(getNonInnerJoins(innerJoin)).isEmpty(); // no left join customer
assertThat(getInnerJoins(innerJoin)).hasSize(1); // inner join customer
}
@Test // DATAJPA-1404
void createsNoJoinForOptionalOneToOneInNormalDirection() {
@@ -248,10 +334,17 @@ public class QueryUtilsIntegrationTests {
private Set<Join<?, ?>> getNonInnerJoins(Root<?> root) {
return root.getJoins() //
.stream() //
.filter(j -> j.getJoinType() != JoinType.INNER) //
.collect(Collectors.toSet());
return getNonInnerJoins((From<?, ?>) root);
}
private Set<Join<?, ?>> getNonInnerJoins(From<?, ?> root) {
return root.getJoins().stream().filter(j -> j.getJoinType() != JoinType.INNER).collect(Collectors.toSet());
}
private Set<Join<?, ?>> getInnerJoins(From<?, ?> root) {
return root.getJoins().stream().filter(j -> j.getJoinType() == JoinType.INNER).collect(Collectors.toSet());
}
@Entity

2
src/test/resources/META-INF/persistence.xml
View File

@@ -22,6 +22,8 @@
<class>org.springframework.data.jpa.domain.sample.EmbeddedIdExampleDepartment</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.Invoice</class>
<class>org.springframework.data.jpa.domain.sample.InvoiceItem</class>
<class>org.springframework.data.jpa.domain.sample.Item</class>
<class>org.springframework.data.jpa.domain.sample.ItemSite</class>
<class>org.springframework.data.jpa.domain.sample.MailMessage</class>