HHH-13756 fix some bug in EmbeddableFetchImpl

hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/EmbeddableFetchImpl.java

@@ -14,6 +14,7 @@ import org.hibernate.metamodel.mapping.EmbeddableMappingType;
 import org.hibernate.metamodel.mapping.EmbeddableValuedModelPart;
 import org.hibernate.query.NavigablePath;
 import org.hibernate.sql.ast.SqlAstJoinType;
+import org.hibernate.sql.ast.tree.from.TableGroup;
 import org.hibernate.sql.ast.tree.from.TableGroupJoin;
 import org.hibernate.sql.results.graph.AbstractFetchParent;
 import org.hibernate.sql.results.graph.AssemblerCreationState;
@@ -54,11 +55,12 @@ public class EmbeddableFetchImpl extends AbstractFetchParent implements Embeddab
 		creationState.getSqlAstCreationState().getFromClauseAccess().resolveTableGroup(
 				getNavigablePath(),
 				np -> {
+					final TableGroup lhsTableGroup = creationState.getSqlAstCreationState()
+							.getFromClauseAccess()
+							.findTableGroup( fetchParent.getNavigablePath() );
 					final TableGroupJoin tableGroupJoin = getReferencedMappingContainer().createTableGroupJoin(
 							getNavigablePath(),
-							creationState.getSqlAstCreationState()
+							lhsTableGroup,
-									.getFromClauseAccess()
-									.findTableGroup( fetchParent.getNavigablePath() ),
 							null,
 							nullable ? SqlAstJoinType.LEFT : SqlAstJoinType.INNER,
 							LockMode.NONE,
@@ -66,7 +68,7 @@ public class EmbeddableFetchImpl extends AbstractFetchParent implements Embeddab
 							creationState.getSqlAstCreationState().getSqlExpressionResolver(),
 							creationState.getSqlAstCreationState().getCreationContext()
 					);
-
+					lhsTableGroup.addTableGroupJoin( tableGroupJoin );
 					return tableGroupJoin.getJoinedGroup();
 				}
 

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

@@ -16,8 +16,10 @@ import org.hibernate.graph.GraphSemantic;
 import org.hibernate.graph.spi.AttributeNodeImplementor;
 import org.hibernate.graph.spi.GraphImplementor;
 import org.hibernate.graph.spi.SubGraphImplementor;
+import org.hibernate.metamodel.mapping.CollectionPart;
 import org.hibernate.metamodel.mapping.EntityMappingType;
 import org.hibernate.metamodel.mapping.PluralAttributeMapping;
+import org.hibernate.metamodel.mapping.internal.EntityCollectionPart;
 import org.hibernate.metamodel.model.domain.EntityDomainType;
 import org.hibernate.sql.results.graph.EntityGraphNavigator;
 import org.hibernate.sql.results.graph.FetchParent;
@@ -69,11 +71,11 @@ public class StandardEntityGraphNavigatorImpl implements EntityGraphNavigator {
 
 					if ( exploreKeySubgraph ) {
 						subgraphMap = attributeNode.getKeySubGraphMap();
-						subgraphMapKey = pluralAttributeMapping.getIndexDescriptor().getClass();
+						subgraphMapKey = getEntityCollectionPartJavaClass( pluralAttributeMapping.getIndexDescriptor() );
 					}
 					else {
 						subgraphMap = attributeNode.getSubGraphMap();
-						subgraphMapKey = pluralAttributeMapping.getElementDescriptor().getClass();
+						subgraphMapKey = getEntityCollectionPartJavaClass( pluralAttributeMapping.getElementDescriptor() );
 					}
 				}
 				else {
@@ -81,7 +83,12 @@ public class StandardEntityGraphNavigatorImpl implements EntityGraphNavigator {
 					subgraphMap = attributeNode.getSubGraphMap();
 					subgraphMapKey = fetchable.getJavaTypeDescriptor().getJavaType();
 				}
-				currentGraphContext = subgraphMap == null ? null : subgraphMap.get( subgraphMapKey );
+				if ( subgraphMap == null || subgraphMapKey == null ) {
+					currentGraphContext = null;
+				}
+				else {
+					currentGraphContext = subgraphMap.get( subgraphMapKey );
+				}
 			}
 			else {
 				currentGraphContext = null;
@@ -100,6 +107,16 @@ public class StandardEntityGraphNavigatorImpl implements EntityGraphNavigator {
 		return new Navigation( previousContextRoot, fetchTiming, joined );
 	}
 
+	private Class<?> getEntityCollectionPartJavaClass(CollectionPart collectionPart) {
+		if ( collectionPart instanceof EntityCollectionPart ) {
+			EntityCollectionPart entityCollectionPart = (EntityCollectionPart) collectionPart;
+			return entityCollectionPart.getEntityMappingType().getJavaTypeDescriptor().getJavaType();
+		}
+		else {
+			return null;
+		}
+	}
+
 	private boolean appliesTo(FetchParent fetchParent) {
 		if ( currentGraphContext == null || !( fetchParent instanceof EntityResultGraphNode ) ) {
 			return false;

hibernate-core/src/test/java/org/hibernate/orm/test/loading/entitygraph/EntityGraphLoadPlanBuilderTest.java

@@ -225,7 +225,7 @@ public class EntityGraphLoadPlanBuilderTest {
 					final SelectStatement sqlAst = buildSqlSelectAst( Dog.class, eg, GraphSemantic.LOAD, scope );
 
 					// Check the from-clause
-					assertPluralAttributeJoinedGroup( sqlAst, "favorites" );
+					assertPluralAttributeJoinedGroup( sqlAst, "favorites", tableGroup -> {} );
 				}
 		);
 	}
@@ -240,13 +240,13 @@ public class EntityGraphLoadPlanBuilderTest {
 					final SelectStatement sqlAst = buildSqlSelectAst( Dog.class, eg, GraphSemantic.FETCH, scope );
 
 					// Check the from-clause
-					assertPluralAttributeJoinedGroup( sqlAst, "favorites" );
+					assertPluralAttributeJoinedGroup( sqlAst, "favorites", tableGroup -> {} );
 				}
 		);
 	}
 
 	@Test
-	void testEmbeddedCollectionLoadSubgraph(SessionFactoryScope scope) {
+	void testEmbeddedCollectionLoadGraph(SessionFactoryScope scope) {
 		scope.inTransaction(
 				em -> {
 					final RootGraphImplementor<ExpressCompany> eg = em.createEntityGraph( ExpressCompany.class );
@@ -259,7 +259,40 @@ public class EntityGraphLoadPlanBuilderTest {
 					);
 
 					// Check the from-clause
-					assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses" );
+					assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses", tableGroup -> {
+						assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) );
+
+						final TableGroup compositeTableGroup = tableGroup.getTableGroupJoins().iterator().next().getJoinedGroup();
+						assertThat( compositeTableGroup, instanceOf( CompositeTableGroup.class ) );
+						assertThat( compositeTableGroup.getTableGroupJoins(), hasSize( 1 ) );
+
+						final TableGroup countryTableGroup = compositeTableGroup.getTableGroupJoins().iterator().next().getJoinedGroup();
+						assertThat( countryTableGroup.getModelPart().getPartName(), is( "country" ) );
+
+						assertThat( countryTableGroup.getTableGroupJoins(), isEmpty() );
+					} );
+
+				}
+		);
+	}
+
+	@Test
+	void testEmbeddedCollectionFetchGraph(SessionFactoryScope scope) {
+		scope.inTransaction(
+				em -> {
+					final RootGraphImplementor<ExpressCompany> eg = em.createEntityGraph( ExpressCompany.class );
+					eg.addAttributeNodes( "shipAddresses" );
+
+					final SelectStatement sqlAst = buildSqlSelectAst(
+							ExpressCompany.class,
+							eg, GraphSemantic.FETCH,
+							scope
+					);
+
+					// Check the from-clause
+					assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses", tableGroup ->
+						assertThat( tableGroup.getTableGroupJoins(), isEmpty() )
+					);
 
 				}
 		);
@@ -283,15 +316,14 @@ public class EntityGraphLoadPlanBuilderTest {
 		assertThat( rootTableGroup.getTableGroupJoins(), hasSize( 1 ) );
 
 		final TableGroup joinedGroup = rootTableGroup.getTableGroupJoins().iterator().next().getJoinedGroup();
+		assertThat( joinedGroup.getModelPart().getPartName(), is( expectedAttributeName ) );
+		assertThat( joinedGroup.getModelPart().getJavaTypeDescriptor().getJavaType(), assignableTo( expectedEntityJpaClass ) );
 		assertThat( joinedGroup.getModelPart(), instanceOf( EntityValuedModelPart.class ) );
 
-		final EntityValuedModelPart entityValuedModelPart = (EntityValuedModelPart) joinedGroup.getModelPart();
-		assertThat( entityValuedModelPart.getPartName(), is( expectedAttributeName ) );
-		assertThat( entityValuedModelPart.getEntityMappingType().getJavaTypeDescriptor().getJavaType(), assignableTo( expectedEntityJpaClass ) );
 		tableGroupConsumer.accept( joinedGroup );
 	}
 
-	private void assertPluralAttributeJoinedGroup(SelectStatement sqlAst, String expectedPluralAttributeName) {
+	private void assertPluralAttributeJoinedGroup(SelectStatement sqlAst, String expectedPluralAttributeName, Consumer<TableGroup> tableGroupConsumer) {
 		final FromClause fromClause = sqlAst.getQuerySpec().getFromClause();
 		assertThat( fromClause.getRoots(), hasSize( 1 ) );
 
@@ -299,24 +331,18 @@ public class EntityGraphLoadPlanBuilderTest {
 		assertThat( root.getTableGroupJoins(), hasSize( 1 ) );
 
 		final TableGroup joinedGroup = root.getTableGroupJoins().iterator().next().getJoinedGroup();
+		assertThat( joinedGroup.getModelPart().getPartName(), is( expectedPluralAttributeName ) );
 		assertThat( joinedGroup.getModelPart(), instanceOf( PluralAttributeMapping.class ) );
-
+		tableGroupConsumer.accept( joinedGroup );
-		final PluralAttributeMapping pluralAttributeMapping = (PluralAttributeMapping) joinedGroup.getModelPart();
-		assertThat( pluralAttributeMapping.getAttributeName(), is( expectedPluralAttributeName ) );
-		assertThat( joinedGroup.getTableGroupJoins(), isEmpty() );
 	}
 
 	private void assertPersonHomeAddressJoinedGroup(TableGroup tableGroup) {
 		assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) );
 
-		final TableGroupJoin tableGroupJoin = tableGroup.getTableGroupJoins().iterator().next();
+		final TableGroup joinedGroup = tableGroup.getTableGroupJoins().iterator().next().getJoinedGroup();
-		assertThat( tableGroupJoin.getJoinedGroup(), instanceOf( CompositeTableGroup.class ) );
+		assertThat( joinedGroup.getModelPart().getPartName(), is( "homeAddress" ) );
-
+		assertThat( joinedGroup.getModelPart(), instanceOf( EmbeddedAttributeMapping.class ) );
-		final CompositeTableGroup compositeTableGroup = (CompositeTableGroup) tableGroupJoin.getJoinedGroup();
+		assertThat( joinedGroup, instanceOf( CompositeTableGroup.class ) );
-		assertThat( compositeTableGroup.getModelPart(), instanceOf( EmbeddedAttributeMapping.class ) );
-
-		final EmbeddedAttributeMapping embeddedAttributeMapping = (EmbeddedAttributeMapping) compositeTableGroup.getModelPart();
-		assertThat( embeddedAttributeMapping.getPartName(), is( "homeAddress" ) );
 	}
 
 	// util methods for verifying 'domain-result' graph ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -345,36 +371,6 @@ public class EntityGraphLoadPlanBuilderTest {
 		entityFetchConsumer.accept( entityFetch );
 	}
 
-	@Test
-	@TestForIssue( jiraKey = "HHH-13756" )
-	void testEmbeddedCollectionFetchSubgraph(SessionFactoryScope scope) {
-		scope.inTransaction(
-				em -> {
-					final RootGraphImplementor<ExpressCompany> eg = em.createEntityGraph( ExpressCompany.class );
-					eg.addAttributeNodes( "shipAddresses" );
-
-					final SelectStatement sqlAst = buildSqlSelectAst(
-							ExpressCompany.class,
-							eg, GraphSemantic.FETCH,
-							scope
-					);
-
-					final FromClause fromClause = sqlAst.getQuerySpec().getFromClause();
-					assertThat( fromClause.getRoots(), hasSize( 1 ) );
-
-					final TableGroup root = fromClause.getRoots().get( 0 );
-					assertThat( root.getTableGroupJoins(), hasSize( 1 ) );
-
-					final TableGroup joinedGroup = root.getTableGroupJoins().iterator().next().getJoinedGroup();
-					assertThat( joinedGroup.getModelPart(), instanceOf( PluralAttributeMapping.class ) );
-
-					final PluralAttributeMapping pluralAttributeMapping = (PluralAttributeMapping) joinedGroup.getModelPart();
-					assertThat( pluralAttributeMapping.getAttributeName(), is( "shipAddresses" ) );
-					assertThat( joinedGroup.getTableGroupJoins(), isEmpty() );
-				}
-		);
-	}
-
 	private <T> SelectStatement buildSqlSelectAst(
 			Class<T> entityType,
 			RootGraphImplementor<T> entityGraph,

hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/entitygraph/HqlEntityGraphTest.java

@@ -228,7 +228,7 @@ public class HqlEntityGraphTest {
 					final SelectStatement sqlAst = buildSqlSelectAst( HqlEntityGraphTest.Dog.class, "select d from Dog as d", eg, GraphSemantic.LOAD, session );
 
 					// Check the from-clause
-					assertPluralAttributeJoinedGroup( sqlAst, "favorites" );
+					assertPluralAttributeJoinedGroup( sqlAst, "favorites", tableGroup -> {} );
 				}
 		);
 	}
@@ -243,13 +243,13 @@ public class HqlEntityGraphTest {
 					final SelectStatement sqlAst = buildSqlSelectAst( HqlEntityGraphTest.Dog.class, "select d from Dog as d", eg, GraphSemantic.FETCH, session );
 
 					// Check the from-clause
-					assertPluralAttributeJoinedGroup( sqlAst, "favorites" );
+					assertPluralAttributeJoinedGroup( sqlAst, "favorites", tableGroup -> {} );
 				}
 		);
 	}
 
 	@Test
-	void testEmbeddedCollectionLoadSubgraph(SessionFactoryScope scope) {
+	void testEmbeddedCollectionLoadGraph(SessionFactoryScope scope) {
 		scope.inTransaction(
 				session -> {
 					final RootGraphImplementor<HqlEntityGraphTest.ExpressCompany> eg = session.createEntityGraph( HqlEntityGraphTest.ExpressCompany.class );
@@ -263,7 +263,39 @@ public class HqlEntityGraphTest {
 					);
 
 					// Check the from-clause
-					assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses" );
+					assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses", tableGroup -> {
+						assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) );
+
+						final TableGroup compositeTableGroup = tableGroup.getTableGroupJoins().iterator().next().getJoinedGroup();
+						assertThat( compositeTableGroup, instanceOf( CompositeTableGroup.class ) );
+						assertThat( compositeTableGroup.getTableGroupJoins(), hasSize( 1 ) );
+
+						final TableGroup countryTableGroup = compositeTableGroup.getTableGroupJoins().iterator().next().getJoinedGroup();
+						assertThat( countryTableGroup.getModelPart().getPartName(), is( "country" ) );
+
+						assertThat( countryTableGroup.getTableGroupJoins(), isEmpty() );
+					} );
+
+				}
+		);
+	}
+
+	@Test
+	void testEmbeddedCollectionFetchGraph(SessionFactoryScope scope) {
+		scope.inTransaction(
+				session -> {
+					final RootGraphImplementor<HqlEntityGraphTest.ExpressCompany> eg = session.createEntityGraph( HqlEntityGraphTest.ExpressCompany.class );
+					eg.addAttributeNodes( "shipAddresses" );
+
+					final SelectStatement sqlAst = buildSqlSelectAst(
+							HqlEntityGraphTest.ExpressCompany.class,
+							"select company from ExpressCompany as company",
+							eg, GraphSemantic.FETCH,
+							session
+					);
+
+					// Check the from-clause
+					assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses", tableGroup -> assertThat( tableGroup.getTableGroupJoins(), isEmpty() ) );
 
 				}
 		);
@@ -287,15 +319,14 @@ public class HqlEntityGraphTest {
 		assertThat( rootTableGroup.getTableGroupJoins(), hasSize( 1 ) );
 
 		final TableGroup joinedGroup = rootTableGroup.getTableGroupJoins().iterator().next().getJoinedGroup();
+		assertThat( joinedGroup.getModelPart().getPartName(), is( expectedAttributeName ) );
+		assertThat( joinedGroup.getModelPart().getJavaTypeDescriptor().getJavaType(), assignableTo( expectedEntityJpaClass ) );
 		assertThat( joinedGroup.getModelPart(), instanceOf( EntityValuedModelPart.class ) );
 
-		final EntityValuedModelPart entityValuedModelPart = (EntityValuedModelPart) joinedGroup.getModelPart();
-		assertThat( entityValuedModelPart.getPartName(), is( expectedAttributeName ) );
-		assertThat( entityValuedModelPart.getEntityMappingType().getJavaTypeDescriptor().getJavaType(), assignableTo( expectedEntityJpaClass ) );
 		tableGroupConsumer.accept( joinedGroup );
 	}
 
-	private void assertPluralAttributeJoinedGroup(SelectStatement sqlAst, String expectedPluralAttributeName) {
+	private void assertPluralAttributeJoinedGroup(SelectStatement sqlAst, String expectedPluralAttributeName, Consumer<TableGroup> tableGroupConsumer) {
 		final FromClause fromClause = sqlAst.getQuerySpec().getFromClause();
 		assertThat( fromClause.getRoots(), hasSize( 1 ) );
 
@@ -303,24 +334,18 @@ public class HqlEntityGraphTest {
 		assertThat( root.getTableGroupJoins(), hasSize( 1 ) );
 
 		final TableGroup joinedGroup = root.getTableGroupJoins().iterator().next().getJoinedGroup();
+		assertThat( joinedGroup.getModelPart().getPartName(), is( expectedPluralAttributeName ) );
 		assertThat( joinedGroup.getModelPart(), instanceOf( PluralAttributeMapping.class ) );
-
+		tableGroupConsumer.accept( joinedGroup );
-		final PluralAttributeMapping pluralAttributeMapping = (PluralAttributeMapping) joinedGroup.getModelPart();
-		assertThat( pluralAttributeMapping.getAttributeName(), is( expectedPluralAttributeName ) );
-		assertThat( joinedGroup.getTableGroupJoins(), isEmpty() );
 	}
 
 	private void assertPersonHomeAddressJoinedGroup(TableGroup tableGroup) {
 		assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) );
 
-		final TableGroupJoin tableGroupJoin = tableGroup.getTableGroupJoins().iterator().next();
+		final TableGroup joinedGroup = tableGroup.getTableGroupJoins().iterator().next().getJoinedGroup();
-		assertThat( tableGroupJoin.getJoinedGroup(), instanceOf( CompositeTableGroup.class ) );
+		assertThat( joinedGroup.getModelPart().getPartName(), is( "homeAddress" ) );
-
+		assertThat( joinedGroup.getModelPart(), instanceOf( EmbeddedAttributeMapping.class ) );
-		final CompositeTableGroup compositeTableGroup = (CompositeTableGroup) tableGroupJoin.getJoinedGroup();
+		assertThat( joinedGroup, instanceOf( CompositeTableGroup.class ) );
-		assertThat( compositeTableGroup.getModelPart(), instanceOf( EmbeddedAttributeMapping.class ) );
-
-		final EmbeddedAttributeMapping embeddedAttributeMapping = (EmbeddedAttributeMapping) compositeTableGroup.getModelPart();
-		assertThat( embeddedAttributeMapping.getPartName(), is( "homeAddress" ) );
 	}
 
 	// util methods for verifying 'domain-result' graph ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -415,7 +440,7 @@ public class HqlEntityGraphTest {
 
 	@Embeddable
 	public static class Address {
-		@ManyToOne
+		@ManyToOne(fetch = FetchType.EAGER)
 		Country country;
 	}