Skip to content

Commit 11c5cad

Browse files
beikovbeikov
committed
HHH-10624 Fix nested join fetching of bags and avoid unnecessary processing of non-bags
1 parent 09f4b96 commit 11c5cad

File tree

4 files changed

+181
-3
lines changed

4 files changed

+181
-3
lines changed

‎hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/LoadingCollectionEntry.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import java.util.function.BiConsumer;
99
import java.util.function.Consumer;
1010

11+
import org.checkerframework.checker.nullness.qual.Nullable;
1112
import org.hibernate.collection.spi.PersistentCollection;
1213
import org.hibernate.persister.collection.CollectionPersister;
1314
import org.hibernate.sql.exec.spi.ExecutionContext;
@@ -38,6 +39,12 @@ public interface LoadingCollectionEntry {
3839
*/
3940
PersistentCollection<?> getCollectionInstance();
4041

42+
/**
43+
* The grandparent entities referring to this collection being loaded.
44+
* This is only available for nested join fetches.
45+
*/
46+
@Nullable Object[] getGrandParentEntities();
47+
4148
/**
4249
* Callback for row loading. Allows delayed List creation
4350
*/

‎hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/AbstractImmediateCollectionInitializer.java‎

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
*/
55
package org.hibernate.sql.results.graph.collection.internal;
66

7+
import java.util.ArrayList;
8+
import java.util.Collections;
79
import java.util.List;
810
import java.util.function.BiConsumer;
911

@@ -21,6 +23,7 @@
2123
import org.hibernate.sql.results.graph.InitializerData;
2224
import org.hibernate.sql.results.graph.InitializerParent;
2325
import org.hibernate.sql.results.graph.collection.LoadingCollectionEntry;
26+
import org.hibernate.sql.results.graph.entity.EntityInitializer;
2427
import org.hibernate.sql.results.internal.LoadingCollectionEntryImpl;
2528
import org.hibernate.sql.results.jdbc.spi.RowProcessingState;
2629

@@ -43,6 +46,7 @@ public abstract class AbstractImmediateCollectionInitializer<Data extends Abstra
4346
* refers to the rows entry in the collection. null indicates that the collection is empty
4447
*/
4548
protected final @Nullable DomainResultAssembler<?> collectionValueKeyResultAssembler;
49+
protected final EntityInitializer<InitializerData> @Nullable [] grandParentEntityInitializers;
4650

4751
public static class ImmediateCollectionInitializerData extends CollectionInitializerData {
4852

@@ -84,6 +88,29 @@ public AbstractImmediateCollectionInitializer(
8488
collectionKeyResult == collectionValueKeyResult
8589
? null
8690
: collectionValueKeyResult.createResultAssembler( this, creationState );
91+
this.grandParentEntityInitializers = determineGrandParentEntityInitializers( parent );
92+
}
93+
94+
private static EntityInitializer<InitializerData> @Nullable [] determineGrandParentEntityInitializers(InitializerParent<?> parent) {
95+
EntityInitializer<?> parentEntityInitializer = Initializer.findOwningEntityInitializer( parent );
96+
ArrayList<EntityInitializer<InitializerData>> grandParentEntityInitializers = null;
97+
while ( parentEntityInitializer != null ) {
98+
parentEntityInitializer = Initializer.findOwningEntityInitializer( parentEntityInitializer.getParent() );
99+
if ( parentEntityInitializer != null ) {
100+
if ( grandParentEntityInitializers == null ) {
101+
grandParentEntityInitializers = new ArrayList<>();
102+
}
103+
//noinspection unchecked
104+
grandParentEntityInitializers.add( (EntityInitializer<InitializerData>) parentEntityInitializer );
105+
}
106+
}
107+
if ( grandParentEntityInitializers != null ) {
108+
Collections.reverse( grandParentEntityInitializers );
109+
}
110+
//noinspection unchecked
111+
return grandParentEntityInitializers == null
112+
? null
113+
: grandParentEntityInitializers.toArray( new EntityInitializer[0] );
87114
}
88115

89116
@Override
@@ -253,7 +280,7 @@ public void resolveInstance(Data data) {
253280
if ( existingLoadingEntry != null ) {
254281
data.setCollectionInstance( existingLoadingEntry.getCollectionInstance() );
255282

256-
if ( existingLoadingEntry.getInitializer() == this ) {
283+
if ( shouldContinueLoadingEntry( existingLoadingEntry, rowProcessingState ) ) {
257284
assert !data.shallowCached;
258285
// we are responsible for loading the collection values
259286
data.responsibility = (LoadingCollectionEntryImpl) existingLoadingEntry;
@@ -326,6 +353,26 @@ else if ( !data.shallowCached ) {
326353
}
327354
}
328355

356+
protected boolean shouldContinueLoadingEntry(LoadingCollectionEntry existingLoadingEntry, RowProcessingState rowProcessingState) {
357+
if ( existingLoadingEntry.getInitializer() != this ) {
358+
return false;
359+
}
360+
else {
361+
final Object[] existingGrandParentEntities = existingLoadingEntry.getGrandParentEntities();
362+
if ( existingGrandParentEntities != null ) {
363+
final Object[] grandParentEntities = determineGrandParentEntities( rowProcessingState );
364+
assert grandParentEntities != null;
365+
assert existingGrandParentEntities.length == grandParentEntities.length;
366+
for ( int i = 0; i < existingGrandParentEntities.length; i++ ) {
367+
if ( existingGrandParentEntities[i] != grandParentEntities[i] ) {
368+
return false;
369+
}
370+
}
371+
}
372+
return true;
373+
}
374+
}
375+
329376
protected void initializeShallowCached(Data data) {
330377
assert data.shallowCached;
331378
// If this is a query cache hit with the shallow query cache layout,
@@ -423,13 +470,25 @@ private void resolveCollectionContentState(RowProcessingState rowProcessingState
423470
getElementAssembler().resolveState( rowProcessingState );
424471
}
425472

473+
protected Object @Nullable [] determineGrandParentEntities(RowProcessingState rowProcessingState) {
474+
if ( grandParentEntityInitializers == null ) {
475+
return null;
476+
}
477+
final Object[] entities = new Object[grandParentEntityInitializers.length];
478+
for ( int i = 0; i < grandParentEntityInitializers.length; i++ ) {
479+
entities[i] = grandParentEntityInitializers[i].getResolvedInstance( rowProcessingState );
480+
}
481+
return entities;
482+
}
483+
426484
protected void takeResponsibility(Data data) {
427485
data.responsibility =
428486
new LoadingCollectionEntryImpl(
429487
getCollectionAttributeMapping().getCollectionDescriptor(),
430488
this,
431489
data.collectionKey.getKey(),
432-
data.getCollectionInstance()
490+
data.getCollectionInstance(),
491+
determineGrandParentEntities( data.getRowProcessingState() )
433492
);
434493
data.getRowProcessingState().getJdbcValuesSourceProcessingState()
435494
.registerLoadingCollection( data.collectionKey, data.responsibility );

‎hibernate-core/src/main/java/org/hibernate/sql/results/internal/LoadingCollectionEntryImpl.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.util.function.BiConsumer;
1010
import java.util.function.Consumer;
1111

12+
import org.checkerframework.checker.nullness.qual.Nullable;
1213
import org.hibernate.collection.spi.PersistentCollection;
1314
import org.hibernate.engine.spi.PersistenceContext;
1415
import org.hibernate.engine.spi.SharedSessionContractImplementor;
@@ -27,17 +28,20 @@ public class LoadingCollectionEntryImpl implements LoadingCollectionEntry {
2728
private final CollectionInitializer<?> initializer;
2829
private final Object key;
2930
private final PersistentCollection<?> collectionInstance;
31+
private final Object @Nullable [] grandParentEntities;
3032
private final List<Object> loadingState = new ArrayList<>();
3133

3234
public LoadingCollectionEntryImpl(
3335
CollectionPersister collectionDescriptor,
3436
CollectionInitializer<?> initializer,
3537
Object key,
36-
PersistentCollection<?> collectionInstance) {
38+
PersistentCollection<?> collectionInstance,
39+
Object @Nullable [] grandParentEntities) {
3740
this.collectionDescriptor = collectionDescriptor;
3841
this.initializer = initializer;
3942
this.key = key;
4043
this.collectionInstance = collectionInstance;
44+
this.grandParentEntities = grandParentEntities;
4145

4246
collectionInstance.beforeInitialize( collectionDescriptor, -1 );
4347
collectionInstance.beginRead();
@@ -62,6 +66,11 @@ public LoadingCollectionEntryImpl(
6266
return collectionInstance;
6367
}
6468

69+
@Override
70+
public @Nullable Object[] getGrandParentEntities() {
71+
return grandParentEntities;
72+
}
73+
6574
@Override
6675
public void load(Consumer<List<Object>> loadingEntryConsumer) {
6776
loadingEntryConsumer.accept( loadingState );

‎hibernate-core/src/test/java/org/hibernate/orm/test/query/joinfetch/JoinFetchNestedBagTest.java‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.orm.test.query.joinfetch;
6+
7+
import jakarta.persistence.Entity;
8+
import jakarta.persistence.GeneratedValue;
9+
import jakarta.persistence.Id;
10+
import jakarta.persistence.ManyToOne;
11+
import jakarta.persistence.OneToMany;
12+
import org.hibernate.testing.orm.junit.DomainModel;
13+
import org.hibernate.testing.orm.junit.Jira;
14+
import org.hibernate.testing.orm.junit.SessionFactory;
15+
import org.hibernate.testing.orm.junit.SessionFactoryScope;
16+
import org.junit.jupiter.api.AfterEach;
17+
import org.junit.jupiter.api.Test;
18+
19+
import java.util.ArrayList;
20+
import java.util.Collections;
21+
import java.util.List;
22+
23+
import static org.junit.jupiter.api.Assertions.assertEquals;
24+
25+
@DomainModel(
26+
annotatedClasses = {
27+
JoinFetchNestedBagTest.Node.class,
28+
JoinFetchNestedBagTest.NodeHolder.class
29+
}
30+
)
31+
@SessionFactory
32+
@Jira("https://hibernate.atlassian.net/browse/HHH-10624")
33+
public class JoinFetchNestedBagTest {
34+
35+
@Test
36+
public void test(SessionFactoryScope scope) {
37+
scope.inTransaction( session -> {
38+
Node leafNode = new Node( "leaf", Collections.emptyList() );
39+
session.persist( leafNode );
40+
Node parentNode = new Node( "parent", Collections.singletonList( leafNode ) );
41+
session.persist( parentNode );
42+
session.persist( new NodeHolder( parentNode ) );
43+
session.persist( new NodeHolder( parentNode ) );
44+
} );
45+
scope.inTransaction( session -> {
46+
final List<NodeHolder> holders = session.createQuery(
47+
"from NodeHolder h join fetch h.descendant d join fetch d.children",
48+
NodeHolder.class
49+
).getResultList();
50+
assertEquals( 2, holders.size() );
51+
assertEquals( 1, holders.get( 0 ).getDescendant().getChildren().size() );
52+
} );
53+
}
54+
55+
@AfterEach
56+
public void cleanupData(SessionFactoryScope scope) {
57+
scope.getSessionFactory().getSchemaManager().truncateMappedObjects();
58+
}
59+
60+
@Entity(name = "Node")
61+
public static class Node {
62+
@Id
63+
private String name;
64+
@OneToMany
65+
private List<Node> children;
66+
67+
public Node() {
68+
}
69+
70+
public Node(String name, List<Node> children) {
71+
this.name = name;
72+
this.children = new ArrayList<>( children );
73+
}
74+
75+
public String getName() {
76+
return name;
77+
}
78+
79+
public List<Node> getChildren() {
80+
return children;
81+
}
82+
}
83+
84+
@Entity(name = "NodeHolder")
85+
public static class NodeHolder {
86+
@Id
87+
@GeneratedValue
88+
private Long id;
89+
@ManyToOne
90+
private Node descendant;
91+
92+
public NodeHolder() {
93+
}
94+
95+
public NodeHolder(Node descendant) {
96+
this.descendant = descendant;
97+
}
98+
99+
public Node getDescendant() {
100+
return descendant;
101+
}
102+
}
103+
}

0 commit comments

Comments
 (0)