HHH-13725 Work on circular fetc detection

This commit is contained in:
Andrea Boriero 2019-11-16 13:40:21 +00:00 committed by Steve Ebersole
parent 032fdb5d2e
commit 97f9d4ce00
12 changed files with 313 additions and 77 deletions

View File

@ -14,6 +14,7 @@ import java.util.SortedSet;
import java.util.function.Consumer;
import org.hibernate.FetchMode;
import org.hibernate.HibernateException;
import org.hibernate.LockMode;
import org.hibernate.MappingException;
import org.hibernate.NotYetImplementedFor6Exception;
@ -90,6 +91,7 @@ import org.hibernate.type.AssociationType;
import org.hibernate.type.BasicType;
import org.hibernate.type.CompositeType;
import org.hibernate.type.EntityType;
import org.hibernate.type.ForeignKeyDirection;
import org.hibernate.type.Type;
import org.hibernate.type.descriptor.java.ImmutableMutabilityPlan;
import org.hibernate.type.descriptor.java.JavaTypeDescriptor;
@ -895,6 +897,7 @@ public class MappingModelCreationHelper {
final BasicValuedModelPart simpleFkTarget = (BasicValuedModelPart) fkTarget;
return new SimpleForeignKeyDescriptor(
( (AssociationType) bootValueMapping.getType() ).getForeignKeyDirection(),
bootValueMapping.getKey().getTable().getName(),
bootValueMapping.getKey().getColumnIterator().next().getText( dialect ),
simpleFkTarget.getContainingTableExpression(),
@ -947,6 +950,7 @@ public class MappingModelCreationHelper {
keyColumnExpression = identifier.getText();
}
return new SimpleForeignKeyDescriptor(
((AssociationType)bootValueMapping.getType()).getForeignKeyDirection(),
identifier.getText(),
keyColumnExpression,
simpleFkTarget.getContainingTableExpression(),

View File

@ -31,6 +31,7 @@ import org.hibernate.sql.ast.tree.predicate.Predicate;
import org.hibernate.sql.results.internal.domain.basic.BasicResult;
import org.hibernate.sql.results.spi.DomainResult;
import org.hibernate.sql.results.spi.DomainResultCreationState;
import org.hibernate.type.ForeignKeyDirection;
import org.hibernate.type.descriptor.java.JavaTypeDescriptor;
import org.hibernate.type.spi.TypeConfiguration;
@ -43,13 +44,16 @@ public class SimpleForeignKeyDescriptor implements ForeignKeyDescriptor {
private final String targetColumnContainingTable;
private final String targetColumnExpression;
private final JdbcMapping jdbcMapping;
private final ForeignKeyDirection fKeyDirection;
public SimpleForeignKeyDescriptor(
ForeignKeyDirection fKeyDirection,
String keyColumnContainingTable,
String keyColumnExpression,
String targetColumnContainingTable,
String targetColumnExpression,
JdbcMapping jdbcMapping) {
this.fKeyDirection = fKeyDirection;
this.keyColumnContainingTable = keyColumnContainingTable;
this.keyColumnExpression = keyColumnExpression;
this.targetColumnContainingTable = targetColumnContainingTable;
@ -64,20 +68,25 @@ public class SimpleForeignKeyDescriptor implements ForeignKeyDescriptor {
DomainResultCreationState creationState) {
final SqlAstCreationState sqlAstCreationState = creationState.getSqlAstCreationState();
final SqlExpressionResolver sqlExpressionResolver = sqlAstCreationState.getSqlExpressionResolver();
final TableReference keyTableKeyReference = getKeyTableReference( tableGroup, tableGroup );
TableReference tableReference = tableGroup.getPrimaryTableReference();
if ( fKeyDirection == ForeignKeyDirection.FROM_PARENT ) {
tableReference = tableGroup.resolveTableReference( keyColumnContainingTable );
}
String identificationVariable = tableReference.getIdentificationVariable();
final SqlSelection sqlSelection = sqlExpressionResolver.resolveSqlSelection(
sqlExpressionResolver.resolveSqlExpression(
SqlExpressionResolver.createColumnReferenceKey(
keyTableKeyReference,
tableReference,
keyColumnExpression
),
s -> new ColumnReference(
keyTableKeyReference,
keyColumnExpression,
jdbcMapping,
creationState.getSqlAstCreationState().getCreationContext().getSessionFactory()
)
s -> {
return new ColumnReference(
identificationVariable,
keyColumnExpression,
jdbcMapping,
creationState.getSqlAstCreationState().getCreationContext().getSessionFactory()
);
}
),
jdbcMapping.getJavaTypeDescriptor(),
sqlAstCreationState.getCreationContext().getDomainModel().getTypeConfiguration()
@ -98,49 +107,90 @@ public class SimpleForeignKeyDescriptor implements ForeignKeyDescriptor {
JoinType joinType,
SqlExpressionResolver sqlExpressionResolver,
SqlAstCreationContext creationContext) {
final TableReference targetTableReference = lhs.resolveTableReference( targetColumnContainingTable );
final ColumnReference targetReference = (ColumnReference) sqlExpressionResolver.resolveSqlExpression(
SqlExpressionResolver.createColumnReferenceKey( targetTableReference, keyColumnExpression ),
s -> new ColumnReference(
targetTableReference.getIdentificationVariable(),
targetColumnExpression,
jdbcMapping,
creationContext.getSessionFactory()
)
final TableReference lhsTableReference = getTableReference( lhs, tableGroup, keyColumnContainingTable );
ColumnReference lhsColumnReference;
lhsColumnReference = new ColumnReference(
lhsTableReference,
keyColumnExpression,
jdbcMapping,
creationContext.getSessionFactory()
);
final TableReference keyTableKeyReference = getKeyTableReference( lhs, tableGroup );
final ColumnReference keyReference = (ColumnReference) sqlExpressionResolver.resolveSqlExpression(
SqlExpressionResolver.createColumnReferenceKey(
keyTableKeyReference,
keyColumnExpression
),
s -> new ColumnReference(
keyTableKeyReference.getIdentificationVariable(),
keyColumnExpression,
jdbcMapping,
creationContext.getSessionFactory()
)
final TableReference rhsTableKeyReference = getRhsTableReference(
lhs,
tableGroup,
targetColumnContainingTable
);
ColumnReference rhsColumnReference;
if ( targetColumnContainingTable.equals( keyColumnContainingTable ) ) {
rhsColumnReference = new ColumnReference(
rhsTableKeyReference.getIdentificationVariable(),
targetColumnExpression,
jdbcMapping,
creationContext.getSessionFactory()
);
}
else {
rhsColumnReference = (ColumnReference) sqlExpressionResolver.resolveSqlExpression(
SqlExpressionResolver.createColumnReferenceKey(
rhsTableKeyReference,
targetColumnExpression
),
s -> new ColumnReference(
rhsTableKeyReference.getIdentificationVariable(),
targetColumnExpression,
jdbcMapping,
creationContext.getSessionFactory()
)
);
}
return new ComparisonPredicate(
targetReference,
lhsColumnReference,
ComparisonOperator.EQUAL,
keyReference
rhsColumnReference
);
}
protected TableReference getKeyTableReference(TableGroup lhs, TableGroup tableGroup) {
protected TableReference getRhsTableReference(TableGroup lhs, TableGroup tableGroup, String table) {
if ( tableGroup.getPrimaryTableReference().getTableExpression().equals( table ) ) {
return tableGroup.getPrimaryTableReference();
}
if ( lhs.getPrimaryTableReference().getTableExpression().equals( table ) ) {
return lhs.getPrimaryTableReference();
}
for ( TableReferenceJoin tableJoin : lhs.getTableReferenceJoins() ) {
if ( tableJoin.getJoinedTableReference().getTableExpression().equals( keyColumnContainingTable ) ) {
if ( tableJoin.getJoinedTableReference().getTableExpression().equals( table ) ) {
return tableJoin.getJoinedTableReference();
}
}
return tableGroup.getPrimaryTableReference();
throw new IllegalStateException( "Could not resolve binding for table `" + table + "`" );
}
protected TableReference getTableReference(TableGroup lhs, TableGroup tableGroup, String table) {
if ( lhs.getPrimaryTableReference().getTableExpression().equals( table ) ) {
return lhs.getPrimaryTableReference();
}
else if ( tableGroup.getPrimaryTableReference().getTableExpression().equals( table ) ) {
return tableGroup.getPrimaryTableReference();
}
else if ( tableGroup.getPrimaryTableReference().getTableExpression().equals( table ) ) {
return tableGroup.getPrimaryTableReference();
}
for ( TableReferenceJoin tableJoin : lhs.getTableReferenceJoins() ) {
if ( tableJoin.getJoinedTableReference().getTableExpression().equals( table ) ) {
return tableJoin.getJoinedTableReference();
}
}
throw new IllegalStateException( "Could not resolve binding for table `" + table + "`" );
}
@Override
public JavaTypeDescriptor getJavaTypeDescriptor() {
return jdbcMapping.getJavaTypeDescriptor();

View File

@ -14,6 +14,8 @@ import org.hibernate.metamodel.mapping.EntityValuedModelPart;
import org.hibernate.metamodel.mapping.ForeignKeyDescriptor;
import org.hibernate.metamodel.mapping.ManagedMappingType;
import org.hibernate.metamodel.mapping.StateArrayContributorMetadataAccess;
import org.hibernate.metamodel.model.domain.NavigableRole;
import org.hibernate.persister.entity.EntityPersister;
import org.hibernate.property.access.spi.PropertyAccess;
import org.hibernate.query.NavigablePath;
import org.hibernate.sql.ast.JoinType;
@ -34,6 +36,7 @@ import org.hibernate.sql.results.internal.domain.entity.EntityFetch;
import org.hibernate.sql.results.spi.DomainResultCreationState;
import org.hibernate.sql.results.spi.Fetch;
import org.hibernate.sql.results.spi.FetchParent;
import org.hibernate.sql.results.spi.FetchableContainer;
/**
* @author Steve Ebersole
@ -43,6 +46,7 @@ public class SingularAssociationAttributeMapping extends AbstractSingularAttribu
private final String sqlAliasStem;
private final boolean isNullable;
private final ForeignKeyDescriptor foreignKeyDescriptor;
private String mappedBy;
public SingularAssociationAttributeMapping(
String name,
@ -66,6 +70,7 @@ public class SingularAssociationAttributeMapping extends AbstractSingularAttribu
this.sqlAliasStem = SqlAliasStemHelper.INSTANCE.generateStemFromAttributeName( name );
this.isNullable = isNullable;
this.foreignKeyDescriptor = foreignKeyDescriptor;
this.mappedBy = null;
}
@Override
@ -220,4 +225,21 @@ public class SingularAssociationAttributeMapping extends AbstractSingularAttribu
);
}
@Override
public boolean isCircular(FetchParent fetchParent) {
NavigablePath navigablePath = fetchParent.getNavigablePath();
if ( navigablePath.getParent() == null ) {
return false;
}
else {
NavigablePath parentParentNavigablePath = navigablePath.getParent();
if ( parentParentNavigablePath.getLocalName().equals( getMappedTypeDescriptor().getEntityName() ) ) {
return true;
}
else {
return false;
}
}
}
}

View File

@ -48,7 +48,7 @@ public interface SqlExpressionResolver {
final String qualifier = tableReference.getIdentificationVariable() == null
? tableReference.getTableExpression()
: tableReference.getIdentificationVariable();
return qualifier + columnExpression;
return qualifier + '.' + columnExpression;
}
/**

View File

@ -56,19 +56,21 @@ public class StandardTableGroup extends AbstractTableGroup {
@Override
public TableReference resolveTableReferenceInternal(String tableExpression) {
TableReference tableReference = super.resolveTableReferenceInternal( tableExpression );
if ( tableReference == null ) {
for ( TableReferenceJoin tableJoin : tableJoins ) {
if ( tableJoin.getJoinedTableReference().getTableExpression().equals( tableExpression ) ) {
return tableJoin.getJoinedTableReference();
}
if ( tableReference != null ) {
return tableReference;
}
for ( TableReferenceJoin tableJoin : tableJoins ) {
if ( tableJoin.getJoinedTableReference().getTableExpression().equals( tableExpression ) ) {
return tableJoin.getJoinedTableReference();
}
}
for ( TableGroupJoin tableGroupJoin : getTableGroupJoins() ) {
final TableReference primaryTableReference = tableGroupJoin.getJoinedGroup().getPrimaryTableReference();
if ( primaryTableReference.getTableExpression().equals( tableExpression ) ) {
return primaryTableReference;
}
}
return tableReference;
return null;
}
}

View File

@ -14,6 +14,7 @@ import org.hibernate.query.NavigablePath;
import org.hibernate.sql.results.spi.DomainResultCreationState;
import org.hibernate.sql.results.spi.Fetch;
import org.hibernate.sql.results.spi.FetchParent;
import org.hibernate.sql.results.spi.Fetchable;
import org.hibernate.sql.results.spi.FetchableContainer;
import org.hibernate.type.descriptor.java.JavaTypeDescriptor;
@ -68,7 +69,9 @@ public abstract class AbstractFetchParent implements FetchParent {
public Fetch findFetch(String fetchableName) {
if ( fetches != null ) {
for ( Fetch fetch : fetches ) {
if ( fetch.getFetchedMapping().getFetchableName().equals( fetchableName ) ) {
final Fetchable fetchedMapping = fetch.getFetchedMapping();
if ( fetchedMapping != null && fetchedMapping
.getFetchableName().equals( fetchableName ) ) {
return fetch;
}
}

View File

@ -17,7 +17,6 @@ import org.hibernate.sql.results.spi.AssemblerCreationState;
import org.hibernate.sql.results.spi.BiDirectionalFetch;
import org.hibernate.sql.results.spi.DomainResultAssembler;
import org.hibernate.sql.results.spi.DomainResultCreationState;
import org.hibernate.sql.results.spi.EntityResult;
import org.hibernate.sql.results.spi.Fetch;
import org.hibernate.sql.results.spi.FetchParent;
import org.hibernate.sql.results.spi.FetchParentAccess;
@ -32,16 +31,19 @@ import org.hibernate.type.descriptor.java.JavaTypeDescriptor;
*/
public class RootBiDirectionalFetchImpl implements BiDirectionalFetch, Fetchable {
private final NavigablePath navigablePath;
private Fetchable fetchable;
private NavigablePath referencedNavigablePath;
private final FetchParent fetchParent;
private final EntityResult referencedRoot;
public RootBiDirectionalFetchImpl(
NavigablePath navigablePath,
FetchParent fetchParent,
EntityResult referencedRoot) {
Fetchable fetchable,
NavigablePath referencedNavigablePath) {
this.fetchParent = fetchParent;
this.referencedRoot = referencedRoot;
this.navigablePath = navigablePath;
this.fetchable = fetchable;
this.referencedNavigablePath = referencedNavigablePath;
}
@Override
@ -51,7 +53,7 @@ public class RootBiDirectionalFetchImpl implements BiDirectionalFetch, Fetchable
@Override
public NavigablePath getReferencedPath() {
return referencedRoot.getNavigablePath();
return referencedNavigablePath;
}
@Override
@ -76,7 +78,7 @@ public class RootBiDirectionalFetchImpl implements BiDirectionalFetch, Fetchable
AssemblerCreationState creationState) {
return new CircularFetchAssembler(
getReferencedPath(),
referencedRoot.getResultJavaTypeDescriptor()
fetchable.getJavaTypeDescriptor()
);
}
@ -93,7 +95,7 @@ public class RootBiDirectionalFetchImpl implements BiDirectionalFetch, Fetchable
@Override
public JavaTypeDescriptor getJavaTypeDescriptor() {
return referencedRoot.getResultJavaTypeDescriptor();
return fetchable.getJavaTypeDescriptor();
}
@Override

View File

@ -92,7 +92,7 @@ public class DelayedEntityFetchInitializer extends AbstractFetchParentAccess imp
}
}
else {
entityInstance = concreteDescriptor.load( fkValue, null, lockMode, rowProcessingState.getSession() );
entityInstance = rowProcessingState.getSession().immediateLoad( concreteDescriptor.getEntityName(), fkValue );
}
notifyParentResolutionListeners( entityInstance );

View File

@ -24,15 +24,14 @@ public class CircularFetchDetector {
return null;
}
assert fetchParent instanceof Fetch;
final Fetch fetchParentAsFetch = (Fetch) fetchParent;
if ( fetchParent instanceof Fetch ) {
final Fetch fetchParentAsFetch = (Fetch) fetchParent;
final NavigablePath parentParentPath = fetchParent.getNavigablePath().getParent();
assert fetchParent.getNavigablePath().getParent() != null;
final NavigablePath parentParentPath = fetchParent.getNavigablePath().getParent();
assert fetchParent.getNavigablePath().getParent() != null;
assert fetchParentAsFetch.getFetchParent().getNavigablePath().equals( parentParentPath );
assert fetchParentAsFetch.getFetchParent().getNavigablePath().equals( parentParentPath );
if ( fetchParentAsFetch.getFetchParent() instanceof Fetch ) {
return new BiDirectionalFetchImpl(
fetchParent.getNavigablePath().append( fetchable.getFetchableName() ),
fetchParent,
@ -40,15 +39,15 @@ public class CircularFetchDetector {
);
}
else {
assert fetchParentAsFetch instanceof EntityResult;
// note : the "`fetchParentAsFetch` is `RootBiDirectionalFetchImpl`" case would
// be handled in the `Fetch` block since `RootBiDirectionalFetchImpl` is a Fetch
return new RootBiDirectionalFetchImpl(
fetchParent.getNavigablePath().append( fetchable.getFetchableName() ),
new NavigablePath( fetchable.getJavaTypeDescriptor().getJavaType().getName() ),
fetchParent,
(EntityResult) fetchParentAsFetch
fetchable,
fetchParent.getNavigablePath()
);
}
}

View File

@ -0,0 +1,150 @@
/*
* 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.sql.exec.manytoone;
import java.util.List;
import org.hibernate.testing.orm.domain.gambit.EntityWithManyToOneSelfReference;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.FailureExpected;
import org.hibernate.testing.orm.junit.ServiceRegistry;
import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
/**
* @author Steve Ebersole
*/
@DomainModel(
annotatedClasses = {
EntityWithManyToOneSelfReference.class
}
)
@ServiceRegistry
@SessionFactory(generateStatistics = true)
public class EntityWithManyToOneSelfReferenceTest {
@BeforeEach
public void setUp(SessionFactoryScope scope) {
final EntityWithManyToOneSelfReference entity1 = new EntityWithManyToOneSelfReference(
1,
"first",
Integer.MAX_VALUE
);
final EntityWithManyToOneSelfReference entity2 = new EntityWithManyToOneSelfReference(
2,
"second",
Integer.MAX_VALUE,
entity1
);
scope.inTransaction( session -> {
session.save( entity1 );
session.save( entity2 );
} );
}
@AfterEach
public void tearDown(SessionFactoryScope scope) {
scope.inTransaction(
session -> {
session.createQuery( "delete from EntityWithManyToOneSelfReference" ).executeUpdate();
}
);
}
@Test
public void testHqlSelect(SessionFactoryScope scope) {
scope.inTransaction(
session -> {
final EntityWithManyToOneSelfReference queryResult = session.createQuery(
"select e from EntityWithManyToOneSelfReference e where e.other.name = 'first'",
EntityWithManyToOneSelfReference.class
).uniqueResult();
assertThat( queryResult.getName(), equalTo( "second" ) );
}
);
}
@Test
@FailureExpected
public void testGetEntity(SessionFactoryScope scope) {
scope.inTransaction(
session -> {
final EntityWithManyToOneSelfReference loaded = session.get(
EntityWithManyToOneSelfReference.class,
2
);
assert loaded != null;
assertThat( loaded.getName(), equalTo( "second" ) );
assert loaded.getOther() != null;
assertThat( loaded.getOther().getName(), equalTo( "first" ) );
}
);
scope.inTransaction(
session -> {
final EntityWithManyToOneSelfReference loaded = session.get(
EntityWithManyToOneSelfReference.class,
1
);
assert loaded != null;
assertThat( loaded.getName(), equalTo( "first" ) );
assertThat( loaded.getOther(), nullValue() );
}
);
scope.inTransaction(
session -> {
final List<EntityWithManyToOneSelfReference> list = session.byMultipleIds(
EntityWithManyToOneSelfReference.class )
.multiLoad( 1, 3 );
assert list.size() == 1;
final EntityWithManyToOneSelfReference loaded = list.get( 0 );
assert loaded != null;
assertThat( loaded.getName(), equalTo( "first" ) );
}
);
scope.inTransaction(
session -> {
final List<EntityWithManyToOneSelfReference> list = session.byMultipleIds(
EntityWithManyToOneSelfReference.class )
.multiLoad( 2, 3 );
assert list.size() == 1;
final EntityWithManyToOneSelfReference loaded = list.get( 0 );
assert loaded != null;
assertThat( loaded.getName(), equalTo( "second" ) );
assert loaded.getOther() != null;
assertThat( loaded.getOther().getName(), equalTo( "first" ) );
}
);
// todo (6.0) : the restriction here uses the wrong table alias...
scope.inTransaction(
session -> {
final String value = session.createQuery(
"select e.name from EntityWithManyToOneSelfReference e where e.other.name = 'first'",
String.class
).uniqueResult();
assertThat( value, equalTo( "second" ) );
}
);
}
}

View File

@ -6,11 +6,11 @@
*/
package org.hibernate.orm.test.sql.exec.manytoone;
import java.sql.Statement;
import javax.persistence.Entity;
import javax.persistence.FetchType;
import javax.persistence.Id;
import javax.persistence.ManyToOne;
import javax.persistence.OneToOne;
import javax.persistence.Table;
import org.hibernate.Hibernate;
@ -280,21 +280,16 @@ public class ManyToOneTest {
@AfterEach
public void tearDown(SessionFactoryScope scope) {
scope.inTransaction(
session ->
session.doWork(
work -> {
Statement statement = work.createStatement();
statement.execute( "delete from mapping_other_entity" );
statement.execute( "delete from mapping_another_simple_entity" );
statement.execute( "delete from mapping_simple_entity" );
statement.close();
}
)
session -> {
session.createQuery( "delete from OtherEntity" ).executeUpdate();
session.createQuery( "delete from SimpleEntity" ).executeUpdate();
session.createQuery( "delete from AnotherSimpleEntity" ).executeUpdate();
}
);
}
@Entity(name = "OtherEntity")
@Table(name = "mapping_other_entity")
@Table(name = "other_entity")
public static class OtherEntity {
private Integer id;
private String name;
@ -340,7 +335,7 @@ public class ManyToOneTest {
}
@Entity(name = "SimpleEntity")
@Table(name = "mapping_simple_entity")
@Table(name = "simple_entity")
public static class SimpleEntity {
private Integer id;
private String name;
@ -364,7 +359,7 @@ public class ManyToOneTest {
}
@Entity(name = "AnotherSimpleEntity")
@Table(name = "mapping_another_simple_entity")
@Table(name = "another_simple_entity")
public static class AnotherSimpleEntity {
private Integer id;
private String name;

View File

@ -9,6 +9,8 @@ package org.hibernate.testing.orm.domain.gambit;
import javax.persistence.Entity;
import javax.persistence.FetchType;
import javax.persistence.Id;
import javax.persistence.JoinColumn;
import javax.persistence.JoinColumns;
import javax.persistence.JoinTable;
import javax.persistence.ManyToOne;
@ -52,7 +54,14 @@ public class EntityWithManyToOneJoinTable {
}
@ManyToOne
@JoinTable(name = "ENTITY_OTHER")
@JoinTable(name = "ENTITY_OTHER",
joinColumns = {
@JoinColumn( name = "LHS_ID")
},
inverseJoinColumns = {
@JoinColumn(name="RHS_ID")
}
)
public SimpleEntity getOther() {
return other;
}