HHH-15964 Incorrect results for pageable EntityGraph with Hibernate 6.1.6

This commit is contained in:
Réda Housni Alaoui 2023-01-31 17:49:56 +01:00 committed by Steve Ebersole
parent e1a8cc1672
commit cf5399998e
10 changed files with 359 additions and 59 deletions

View File

@ -737,9 +737,12 @@ public class LoaderSelectBuilder {
// 'entity graph' takes precedence over 'fetch profile'
if ( entityGraphTraversalState != null ) {
traversalResult = entityGraphTraversalState.traverse( fetchParent, fetchable, isKeyFetchable );
fetchTiming = traversalResult.getFetchTiming();
joined = traversalResult.isJoined();
explicitFetch = shouldExplicitFetch( maximumFetchDepth, fetchable, creationState );
EntityGraphTraversalState.FetchStrategy fetchStrategy = traversalResult.getFetchStrategy();
if ( fetchStrategy != null ) {
fetchTiming = fetchStrategy.getFetchTiming();
joined = fetchStrategy.isJoined();
explicitFetch = shouldExplicitFetch( maximumFetchDepth, fetchable, creationState );
}
}
else if ( loadQueryInfluencers.hasEnabledFetchProfiles() ) {
// There is no point in checking the fetch profile if it can't affect this fetchable

View File

@ -0,0 +1,43 @@
/*
* Hibernate, Relational Persistence for Idiomatic Java
*
* License: GNU Lesser General Public License (LGPL), version 2.1 or later
* See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
*/
package org.hibernate.query.sqm.internal;
import org.hibernate.graph.spi.AppliedGraph;
import org.hibernate.graph.spi.AttributeNodeImplementor;
import org.hibernate.graph.spi.GraphImplementor;
import org.hibernate.graph.spi.SubGraphImplementor;
import org.hibernate.metamodel.model.domain.PersistentAttribute;
import org.hibernate.query.spi.QueryOptions;
/**
* @author Réda Housni Alaoui
*/
public class AppliedGraphs {
private AppliedGraphs() {
}
public static boolean containsCollectionFetches(QueryOptions queryOptions) {
final AppliedGraph appliedGraph = queryOptions.getAppliedGraph();
return appliedGraph != null && appliedGraph.getGraph() != null && containsCollectionFetches(appliedGraph.getGraph());
}
private static boolean containsCollectionFetches(GraphImplementor<?> graph) {
for (AttributeNodeImplementor<?> attributeNodeImplementor : graph.getAttributeNodeImplementors()) {
PersistentAttribute<?, ?> attributeDescriptor = attributeNodeImplementor.getAttributeDescriptor();
if (attributeDescriptor.isCollection()) {
return true;
}
for (SubGraphImplementor<?> subGraph : attributeNodeImplementor.getSubGraphMap().values()) {
if (containsCollectionFetches(subGraph)) {
return true;
}
}
}
return false;
}
}

View File

@ -18,10 +18,6 @@ import org.hibernate.engine.spi.EntityKey;
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.engine.spi.SharedSessionContractImplementor;
import org.hibernate.engine.spi.SubselectFetch;
import org.hibernate.graph.spi.AppliedGraph;
import org.hibernate.graph.spi.AttributeNodeImplementor;
import org.hibernate.graph.spi.GraphImplementor;
import org.hibernate.graph.spi.SubGraphImplementor;
import org.hibernate.internal.EmptyScrollableResults;
import org.hibernate.internal.util.collections.ArrayHelper;
import org.hibernate.metamodel.mapping.MappingModelExpressible;
@ -91,7 +87,7 @@ public class ConcreteSqmSelectQueryPlan<R> implements SelectQueryPlan<R> {
this.rowTransformer = determineRowTransformer( sqm, resultType, tupleMetadata, queryOptions );
final ListResultsConsumer.UniqueSemantic uniqueSemantic;
if ( sqm.producesUniqueResults() && !containsCollectionFetches( queryOptions ) ) {
if ( sqm.producesUniqueResults() && !AppliedGraphs.containsCollectionFetches( queryOptions ) ) {
uniqueSemantic = ListResultsConsumer.UniqueSemantic.NONE;
}
else {
@ -169,25 +165,6 @@ public class ConcreteSqmSelectQueryPlan<R> implements SelectQueryPlan<R> {
return new MySqmJdbcExecutionContextAdapter( executionContext, jdbcSelect, subSelectFetchKeyHandler, hql );
}
private static boolean containsCollectionFetches(QueryOptions queryOptions) {
final AppliedGraph appliedGraph = queryOptions.getAppliedGraph();
return appliedGraph != null && appliedGraph.getGraph() != null && containsCollectionFetches( appliedGraph.getGraph() );
}
private static boolean containsCollectionFetches(GraphImplementor<?> graph) {
for ( AttributeNodeImplementor<?> attributeNodeImplementor : graph.getAttributeNodeImplementors() ) {
if ( attributeNodeImplementor.getAttributeDescriptor().isCollection() ) {
return true;
}
for ( SubGraphImplementor<?> subGraph : attributeNodeImplementor.getSubGraphMap().values() ) {
if ( containsCollectionFetches( subGraph ) ) {
return true;
}
}
}
return false;
}
@SuppressWarnings("unchecked")
protected static <T> RowTransformer<T> determineRowTransformer(
SqmSelectStatement<?> sqm,

View File

@ -8,6 +8,7 @@ package org.hibernate.query.sqm.internal;
import java.io.Serializable;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Calendar;
import java.util.Collection;
@ -510,7 +511,8 @@ public class QuerySqmImpl<R>
getSession().prepareForQueryExecution( requiresTxn( getQueryOptions().getLockOptions().findGreatestLockMode() ) );
final SqmSelectStatement<?> sqmStatement = (SqmSelectStatement<?>) getSqmStatement();
final boolean containsCollectionFetches = sqmStatement.containsCollectionFetches();
final boolean containsCollectionFetches = sqmStatement.containsCollectionFetches() || AppliedGraphs.containsCollectionFetches(
getQueryOptions() );
final boolean hasLimit = hasLimit( sqmStatement, getQueryOptions() );
final boolean needsDistinct = containsCollectionFetches
&& ( sqmStatement.usesDistinct() || hasAppliedGraph( getQueryOptions() ) || hasLimit );
@ -534,6 +536,9 @@ public class QuerySqmImpl<R>
else {
toIndex = resultSize;
}
if ( first > resultSize ) {
return new ArrayList<>(0);
}
return list.subList( first, toIndex > resultSize ? resultSize : toIndex );
}
}

View File

@ -7208,10 +7208,14 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
isKeyFetchable
);
fetchTiming = traversalResult.getFetchTiming();
joined = traversalResult.isJoined();
if ( shouldExplicitFetch( maxDepth, fetchable ) ) {
explicitFetch = true;
EntityGraphTraversalState.FetchStrategy fetchStrategy = traversalResult.getFetchStrategy();
if ( fetchStrategy != null ) {
fetchTiming = fetchStrategy.getFetchTiming();
joined = fetchStrategy.isJoined();
if ( shouldExplicitFetch( maxDepth, fetchable ) ) {
explicitFetch = true;
}
}
}
else if ( getLoadQueryInfluencers().hasEnabledFetchProfiles() ) {

View File

@ -9,7 +9,6 @@ package org.hibernate.sql.results.graph;
import org.hibernate.Incubating;
import org.hibernate.engine.FetchTiming;
import org.hibernate.graph.AttributeNode;
import org.hibernate.graph.spi.AttributeNodeImplementor;
import org.hibernate.graph.spi.GraphImplementor;
/**
@ -26,19 +25,32 @@ public interface EntityGraphTraversalState {
*/
class TraversalResult {
private final GraphImplementor<?> previousContext;
private final FetchTiming fetchTiming;
private final boolean joined;
private final FetchStrategy fetchStrategy;
public TraversalResult(GraphImplementor<?> previousContext, FetchTiming fetchTiming, boolean joined) {
public TraversalResult(GraphImplementor<?> previousContext, FetchStrategy fetchStrategy) {
this.previousContext = previousContext;
this.fetchTiming = fetchTiming;
this.joined = joined;
this.fetchStrategy = fetchStrategy;
}
public GraphImplementor<?> getGraph() {
return previousContext;
}
public FetchStrategy getFetchStrategy() {
return fetchStrategy;
}
}
class FetchStrategy {
private final FetchTiming fetchTiming;
private final boolean joined;
public FetchStrategy(FetchTiming fetchTiming, boolean joined) {
assert fetchTiming != null;
this.fetchTiming = fetchTiming;
this.joined = joined;
}
public FetchTiming getFetchTiming() {
return fetchTiming;
}

View File

@ -259,11 +259,10 @@ public class ResultsHelper {
}
}
if ( collectionOwner == null ) {
throw new HibernateException(
"Unable to resolve owner of loading collection [" +
MessageHelper.collectionInfoString( collectionDescriptor, collectionInstance, key, session ) +
"] for second level caching"
);
if ( LOG.isDebugEnabled() ) {
LOG.debugf( "Unable to resolve owner of loading collection for second level caching. Refusing to add to cache.");
}
return;
}
}
version = persistenceContext.getEntry( collectionOwner ).getVersion();

View File

@ -48,7 +48,7 @@ public class StandardEntityGraphTraversalStateImpl implements EntityGraphTravers
public TraversalResult traverse(FetchParent fetchParent, Fetchable fetchable, boolean exploreKeySubgraph) {
assert !(fetchable instanceof CollectionPart);
if ( fetchable instanceof NonAggregatedIdentifierMapping ) {
return new TraversalResult( currentGraphContext, FetchTiming.IMMEDIATE, true );
return new TraversalResult( currentGraphContext, new FetchStrategy( FetchTiming.IMMEDIATE, true ) );
}
final GraphImplementor previousContextRoot = currentGraphContext;
@ -58,12 +58,11 @@ public class StandardEntityGraphTraversalStateImpl implements EntityGraphTravers
}
currentGraphContext = null;
FetchTiming fetchTiming = null;
boolean joined = false;
FetchStrategy fetchStrategy = null;
if ( attributeNode != null ) {
fetchTiming = FetchTiming.IMMEDIATE;
joined = true;
fetchStrategy = new FetchStrategy( FetchTiming.IMMEDIATE, true );
final Map<Class<?>, SubGraphImplementor> subgraphMap;
final Class<?> subgraphMapKey;
@ -89,17 +88,10 @@ public class StandardEntityGraphTraversalStateImpl implements EntityGraphTravers
currentGraphContext = subgraphMap.get( subgraphMapKey );
}
}
if ( fetchTiming == null ) {
if ( graphSemantic == GraphSemantic.FETCH ) {
fetchTiming = FetchTiming.DELAYED;
joined = false;
}
else {
fetchTiming = fetchable.getMappedFetchOptions().getTiming();
joined = fetchable.getMappedFetchOptions().getStyle() == FetchStyle.JOIN;
}
if ( fetchStrategy == null && graphSemantic == GraphSemantic.FETCH ) {
fetchStrategy = new FetchStrategy( FetchTiming.DELAYED, false );
}
return new TraversalResult( previousContextRoot, fetchTiming, joined );
return new TraversalResult( previousContextRoot, fetchStrategy );
}
private Class<?> getEntityCollectionPartJavaClass(CollectionPart collectionPart) {

View File

@ -0,0 +1,129 @@
/*
* Hibernate, Relational Persistence for Idiomatic Java
*
* License: GNU Lesser General Public License (LGPL), version 2.1 or later.
* See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
*/
package org.hibernate.orm.test.jpa.graphs;
import java.util.LinkedHashSet;
import java.util.Set;
import jakarta.persistence.Cacheable;
import jakarta.persistence.ElementCollection;
import jakarta.persistence.Entity;
import jakarta.persistence.EntityGraph;
import jakarta.persistence.EntityManager;
import jakarta.persistence.EnumType;
import jakarta.persistence.Enumerated;
import jakarta.persistence.FetchType;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.OneToOne;
import jakarta.persistence.Version;
import org.hibernate.annotations.Cache;
import org.hibernate.annotations.CacheConcurrencyStrategy;
import org.hibernate.orm.test.jpa.BaseEntityManagerFunctionalTestCase;
import org.hibernate.testing.TestForIssue;
import org.junit.Test;
public class CacheableEntityGraphTest extends BaseEntityManagerFunctionalTestCase {
@Override
protected Class<?>[] getAnnotatedClasses() {
return new Class[]{Product.class, Color.class, Tag.class};
}
@Test
@TestForIssue(jiraKey = "HHH-15964")
public void test() {
EntityManager em = getOrCreateEntityManager();
em.getTransaction().begin();
Tag tag = new Tag(Set.of(TagType.FOO));
em.persist(tag);
Color color = new Color();
em.persist(color);
Product product = new Product(tag, color);
em.persist(product);
em.getTransaction().commit();
em.clear();
EntityGraph<Product> entityGraph = em.createEntityGraph(Product.class);
entityGraph.addAttributeNodes("tag");
em.createQuery(
"select p from org.hibernate.orm.test.jpa.graphs.CacheableEntityGraphTest$Product p",
Product.class)
.setMaxResults(2)
.setHint("jakarta.persistence.loadgraph", entityGraph)
.getSingleResult();
}
@Entity
public static class Product {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
public int id;
@ManyToOne(fetch = FetchType.LAZY)
public Tag tag;
@OneToOne(mappedBy = "product", fetch = FetchType.LAZY)
private Color color;
public Product() {
}
public Product(Tag tag, Color color) {
this.tag = tag;
this.color = color;
}
}
@Entity
public static class Color {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
public int id;
@OneToOne(fetch = FetchType.LAZY)
public Product product;
}
@Cacheable
@Entity
public static class Tag {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
public int id;
@Version
public long version;
@Enumerated(EnumType.STRING)
@ElementCollection(fetch = FetchType.EAGER)
@Cache(usage = CacheConcurrencyStrategy.READ_WRITE)
public final Set<TagType> types = new LinkedHashSet<>();
public Tag() {
}
public Tag(Set<TagType> types) {
this.types.addAll(types);
}
}
public enum TagType {
FOO,
BAR
}
}

View File

@ -9,8 +9,13 @@ package org.hibernate.orm.test.jpa.graphs;
import jakarta.persistence.MapKey;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import jakarta.persistence.Entity;
import jakarta.persistence.EntityGraph;
import jakarta.persistence.EntityManager;
@ -45,7 +50,7 @@ public class EntityGraphTest extends BaseEntityManagerFunctionalTestCase {
@Override
protected Class<?>[] getAnnotatedClasses() {
return new Class[] { Foo.class, Bar.class, Baz.class, Author.class, Book.class,
return new Class[] { Foo.class, Bar.class, Baz.class, Author.class, Book.class, Prize.class,
Company.class, Employee.class, Manager.class, Location.class };
}
@ -328,6 +333,97 @@ public class EntityGraphTest extends BaseEntityManagerFunctionalTestCase {
em.close();
}
@Test
@TestForIssue(jiraKey = "HHH-15964")
public void paginationOverCollectionFetch() {
EntityManager em = getOrCreateEntityManager();
em.getTransaction().begin();
String authorName = UUID.randomUUID().toString();
Set<Integer> authorIds = IntStream.range(0, 3)
.mapToObj(v -> {
Author author = new Author(authorName);
em.persist(author);
em.persist(new Book(author));
em.persist(new Book(author));
return author;
})
.map(author -> author.id)
.collect(Collectors.toSet());
em.getTransaction().commit();
em.clear();
em.getTransaction().begin();
EntityGraph<Author> entityGraph = em.createEntityGraph(Author.class);
entityGraph.addAttributeNodes("books");
CriteriaBuilder criteriaBuilder = em.getCriteriaBuilder();
CriteriaQuery<Author> query = criteriaBuilder.createQuery(Author.class);
Root<Author> root = query.from(Author.class);
query.where(criteriaBuilder.equal(root.get("name"), authorName));
List<Integer> fetchedAuthorIds = em.createQuery(query)
.setFirstResult(0)
.setMaxResults(4)
.setHint("jakarta.persistence.loadgraph", entityGraph)
.getResultList()
.stream()
.map(author -> author.id)
.collect(Collectors.toList());
assertEquals(3, fetchedAuthorIds.size());
assertTrue(fetchedAuthorIds.containsAll(authorIds));
em.getTransaction().commit();
em.close();
}
@Test
@TestForIssue(jiraKey = "HHH-15964")
public void paginationOverEagerCollectionWithEmptyEG() {
EntityManager em = getOrCreateEntityManager();
em.getTransaction().begin();
String authorName = UUID.randomUUID().toString();
Set<Integer> authorIds = IntStream.range(0, 3)
.mapToObj(v -> {
Author author = new Author(authorName);
em.persist(author);
em.persist(new Prize(author));
em.persist(new Prize(author));
return author;
})
.map(author -> author.id)
.collect(Collectors.toSet());
em.getTransaction().commit();
em.clear();
em.getTransaction().begin();
EntityGraph<Author> entityGraph = em.createEntityGraph(Author.class);
CriteriaBuilder criteriaBuilder = em.getCriteriaBuilder();
CriteriaQuery<Author> query = criteriaBuilder.createQuery(Author.class);
Root<Author> root = query.from(Author.class);
query.where(criteriaBuilder.equal(root.get("name"), authorName));
List<Integer> fetchedAuthorIds = em.createQuery(query)
.setFirstResult(0)
.setMaxResults(4)
.setHint("jakarta.persistence.loadgraph", entityGraph)
.getResultList()
.stream()
.map(author -> author.id)
.collect(Collectors.toList());
assertEquals(3, fetchedAuthorIds.size());
assertTrue(fetchedAuthorIds.containsAll(authorIds));
em.getTransaction().commit();
em.close();
}
@Entity
@Table(name = "foo")
public static class Foo {
@ -377,6 +473,33 @@ public class EntityGraphTest extends BaseEntityManagerFunctionalTestCase {
@ManyToOne(fetch = FetchType.LAZY)
private Author author;
public Book() {
}
public Book(Author author) {
this.author = author;
}
}
@Entity
public static class Prize {
@Id
@GeneratedValue
public Integer id;
@ManyToOne(fetch = FetchType.LAZY)
private Author author;
public Prize() {
}
public Prize(Author author) {
this.author = author;
}
}
@Entity
@ -389,5 +512,18 @@ public class EntityGraphTest extends BaseEntityManagerFunctionalTestCase {
@OneToMany(fetch = FetchType.LAZY, mappedBy = "author")
@MapKey
public Map<Integer, Book> books = new HashMap<>();
@OneToMany(fetch = FetchType.EAGER, mappedBy = "author")
public Set<Prize> eagerPrizes = new HashSet<>();
public String name;
public Author() {
}
public Author(String name) {
this.name = name;
}
}
}