Skip to content

Commit

Permalink
HHH-13756 fix some bug in EmbeddableFetchImpl
Browse files Browse the repository at this point in the history
  • Loading branch information
@NathanQingyangXu @sebersole
NathanQingyangXu authored and sebersole committed Mar 19, 2020
1 parent 979e146 commit d4746da
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 78 deletions.
10 changes: 6 additions & 4 deletions ...rc/main/java/org/hibernate/sql/results/graph/embeddable/internal/EmbeddableFetchImpl.java
View file
Expand Up @@ -14,6 +14,7 @@
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;
Expand Down Expand Up @@ -54,19 +55,20 @@ public EmbeddableFetchImpl(
creationState.getSqlAstCreationState().getFromClauseAccess().resolveTableGroup(
getNavigablePath(),
np -> {
final TableGroup lhsTableGroup = creationState.getSqlAstCreationState()
.getFromClauseAccess()
.findTableGroup( fetchParent.getNavigablePath() );
final TableGroupJoin tableGroupJoin = getReferencedMappingContainer().createTableGroupJoin(
getNavigablePath(),
creationState.getSqlAstCreationState()
.getFromClauseAccess()
.findTableGroup( fetchParent.getNavigablePath() ),
lhsTableGroup,
null,
nullable ? SqlAstJoinType.LEFT : SqlAstJoinType.INNER,
LockMode.NONE,
stem -> creationState.getSqlAliasBaseManager().createSqlAliasBase( stem ),
creationState.getSqlAstCreationState().getSqlExpressionResolver(),
creationState.getSqlAstCreationState().getCreationContext()
);

lhsTableGroup.addTableGroupJoin( tableGroupJoin );
return tableGroupJoin.getJoinedGroup();
}

Expand Down
23 changes: 20 additions & 3 deletions ...re/src/main/java/org/hibernate/sql/results/internal/StandardEntityGraphNavigatorImpl.java
View file
Expand Up @@ -16,8 +16,10 @@
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;
Expand Down Expand Up @@ -69,19 +71,24 @@ assert exploreKeySubgraph && isJpaMapCollectionType( pluralAttributeMapping )

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 {
assert !exploreKeySubgraph;
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;
Expand All @@ -100,6 +107,16 @@ assert exploreKeySubgraph && isJpaMapCollectionType( pluralAttributeMapping )
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;
Expand Down
96 changes: 46 additions & 50 deletions .../test/java/org/hibernate/orm/test/loading/entitygraph/EntityGraphLoadPlanBuilderTest.java
View file
Expand Up @@ -225,7 +225,7 @@ void testBasicElementCollectionsLoadGraph(SessionFactoryScope scope) {
final SelectStatement sqlAst = buildSqlSelectAst( Dog.class, eg, GraphSemantic.LOAD, scope );

// Check the from-clause
assertPluralAttributeJoinedGroup( sqlAst, "favorites" );
assertPluralAttributeJoinedGroup( sqlAst, "favorites", tableGroup -> {} );
}
);
}
Expand All @@ -240,13 +240,13 @@ void testBasicElementCollectionsFetchGraph(SessionFactoryScope scope) {
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 );
Expand All @@ -259,7 +259,40 @@ void testEmbeddedCollectionLoadSubgraph(SessionFactoryScope scope) {
);

// 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() )
);

}
);
Expand All @@ -283,40 +316,33 @@ private void assertEntityValuedJoinedGroup(SelectStatement sqlAst, String expect
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 ) );

final TableGroup root = fromClause.getRoots().get( 0 );
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 ) );

final PluralAttributeMapping pluralAttributeMapping = (PluralAttributeMapping) joinedGroup.getModelPart();
assertThat( pluralAttributeMapping.getAttributeName(), is( expectedPluralAttributeName ) );
assertThat( joinedGroup.getTableGroupJoins(), isEmpty() );
tableGroupConsumer.accept( joinedGroup );
}

private void assertPersonHomeAddressJoinedGroup(TableGroup tableGroup) {
assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) );

final TableGroupJoin tableGroupJoin = tableGroup.getTableGroupJoins().iterator().next();
assertThat( tableGroupJoin.getJoinedGroup(), instanceOf( CompositeTableGroup.class ) );

final CompositeTableGroup compositeTableGroup = (CompositeTableGroup) tableGroupJoin.getJoinedGroup();
assertThat( compositeTableGroup.getModelPart(), instanceOf( EmbeddedAttributeMapping.class ) );

final EmbeddedAttributeMapping embeddedAttributeMapping = (EmbeddedAttributeMapping) compositeTableGroup.getModelPart();
assertThat( embeddedAttributeMapping.getPartName(), is( "homeAddress" ) );
final TableGroup joinedGroup = tableGroup.getTableGroupJoins().iterator().next().getJoinedGroup();
assertThat( joinedGroup.getModelPart().getPartName(), is( "homeAddress" ) );
assertThat( joinedGroup.getModelPart(), instanceOf( EmbeddedAttributeMapping.class ) );
assertThat( joinedGroup, instanceOf( CompositeTableGroup.class ) );
}

// util methods for verifying 'domain-result' graph ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down Expand Up @@ -345,36 +371,6 @@ private void assertDomainResult(SelectStatement sqlAst,
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,
Expand Down
67 changes: 46 additions & 21 deletions ...e-core/src/test/java/org/hibernate/orm/test/query/hql/entitygraph/HqlEntityGraphTest.java
View file
Expand Up @@ -228,7 +228,7 @@ void testBasicElementCollectionsLoadGraph(SessionFactoryScope scope) {
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 -> {} );
}
);
}
Expand All @@ -243,13 +243,13 @@ void testBasicElementCollectionsFetchGraph(SessionFactoryScope scope) {
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 );
Expand All @@ -263,7 +263,39 @@ void testEmbeddedCollectionLoadSubgraph(SessionFactoryScope scope) {
);

// 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() ) );

}
);
Expand All @@ -287,40 +319,33 @@ private void assertEntityValuedJoinedGroup(SelectStatement sqlAst, String expect
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 ) );

final TableGroup root = fromClause.getRoots().get( 0 );
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 ) );

final PluralAttributeMapping pluralAttributeMapping = (PluralAttributeMapping) joinedGroup.getModelPart();
assertThat( pluralAttributeMapping.getAttributeName(), is( expectedPluralAttributeName ) );
assertThat( joinedGroup.getTableGroupJoins(), isEmpty() );
tableGroupConsumer.accept( joinedGroup );
}

private void assertPersonHomeAddressJoinedGroup(TableGroup tableGroup) {
assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) );

final TableGroupJoin tableGroupJoin = tableGroup.getTableGroupJoins().iterator().next();
assertThat( tableGroupJoin.getJoinedGroup(), instanceOf( CompositeTableGroup.class ) );

final CompositeTableGroup compositeTableGroup = (CompositeTableGroup) tableGroupJoin.getJoinedGroup();
assertThat( compositeTableGroup.getModelPart(), instanceOf( EmbeddedAttributeMapping.class ) );

final EmbeddedAttributeMapping embeddedAttributeMapping = (EmbeddedAttributeMapping) compositeTableGroup.getModelPart();
assertThat( embeddedAttributeMapping.getPartName(), is( "homeAddress" ) );
final TableGroup joinedGroup = tableGroup.getTableGroupJoins().iterator().next().getJoinedGroup();
assertThat( joinedGroup.getModelPart().getPartName(), is( "homeAddress" ) );
assertThat( joinedGroup.getModelPart(), instanceOf( EmbeddedAttributeMapping.class ) );
assertThat( joinedGroup, instanceOf( CompositeTableGroup.class ) );
}

// util methods for verifying 'domain-result' graph ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down Expand Up @@ -415,7 +440,7 @@ public static class Person {

@Embeddable
public static class Address {
@ManyToOne
@ManyToOne(fetch = FetchType.EAGER)
Country country;
}

Expand Down

0 comments on commit d4746da

Please sign in to comment.