From 560a397a01e4ff7043a7c82bce4091e18ed8c49b Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Wed, 10 Apr 2013 16:17:27 -0700 Subject: [PATCH] HHH-7841 - Redesign Loader --- .../internal/AbstractEntityLoadQueryImpl.java | 9 +- .../internal/AbstractLoadQueryImpl.java | 53 +----- .../internal/EntityLoadQueryBuilderImpl.java | 89 ++++++++- .../loader/internal/EntityLoadQueryImpl.java | 5 +- .../internal/JoinableAssociationImpl.java | 60 +++++- .../ResultSetProcessingContextImpl.java | 139 ++++++++++++-- .../internal/ResultSetProcessorImpl.java | 2 + .../plan/internal/LoadPlanBuildingHelper.java | 7 +- .../spi/AbstractLoadPlanBuilderStrategy.java | 1 + .../loader/plan/spi/CollectionFetch.java | 2 + .../loader/plan/spi/LoadPlanVisitor.java | 12 +- .../spi/MetadataDrivenModelGraphVisitor.java | 2 +- ...ityAssociationResultSetProcessorTest.java} | 4 +- ...yWithCollectionResultSetProcessorTest.java | 174 ++++++++++++++++++ 14 files changed, 475 insertions(+), 84 deletions(-) rename hibernate-core/src/test/java/org/hibernate/loader/{AssociationResultSetProcessorTest.java => EntityAssociationResultSetProcessorTest.java} (98%) create mode 100644 hibernate-core/src/test/java/org/hibernate/loader/EntityWithCollectionResultSetProcessorTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/loader/internal/AbstractEntityLoadQueryImpl.java b/hibernate-core/src/main/java/org/hibernate/loader/internal/AbstractEntityLoadQueryImpl.java index b5c1243515..d2b4a1ff90 100755 --- a/hibernate-core/src/main/java/org/hibernate/loader/internal/AbstractEntityLoadQueryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/internal/AbstractEntityLoadQueryImpl.java @@ -46,9 +46,8 @@ public abstract class AbstractEntityLoadQueryImpl extends AbstractLoadQueryImpl public AbstractEntityLoadQueryImpl( SessionFactoryImplementor factory, EntityReturn entityReturn, - List associations, - List suffixes) { - super( factory, associations, suffixes ); + List associations) { + super( factory, associations ); this.entityReturn = entityReturn; } @@ -68,6 +67,10 @@ public abstract class AbstractEntityLoadQueryImpl extends AbstractLoadQueryImpl JoinFragment ojf = mergeOuterJoins(); + // If no projection, then the last suffix should be for the entity return. + // TODO: simplify how suffixes are generated/processed. + + Select select = new Select( getDialect() ) .setLockOptions( lockOptions ) .setSelectClause( diff --git a/hibernate-core/src/main/java/org/hibernate/loader/internal/AbstractLoadQueryImpl.java b/hibernate-core/src/main/java/org/hibernate/loader/internal/AbstractLoadQueryImpl.java index 1ccd8ff2b8..e934434b6b 100755 --- a/hibernate-core/src/main/java/org/hibernate/loader/internal/AbstractLoadQueryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/internal/AbstractLoadQueryImpl.java @@ -22,7 +22,6 @@ * Boston, MA 02110-1301 USA */ package org.hibernate.loader.internal; -import java.util.Iterator; import java.util.List; import org.hibernate.MappingException; @@ -47,18 +46,12 @@ public abstract class AbstractLoadQueryImpl { private final SessionFactoryImplementor factory; private final List associations; - private final List suffixes; - - private String[] collectionSuffixes; protected AbstractLoadQueryImpl( SessionFactoryImplementor factory, - List associations, - List suffixes) { + List associations) { this.factory = factory; this.associations = associations; - // TODO: we should be able to get the suffixes out of associations. - this.suffixes = suffixes; } protected SessionFactoryImplementor getFactory() { @@ -104,30 +97,10 @@ public abstract class AbstractLoadQueryImpl { return outerjoin; } - /** - * Count the number of instances of Joinable which are actually - * also instances of PersistentCollection which are being fetched - * by outer join - */ - protected static final int countCollectionPersisters(List associations) - throws MappingException { - int result = 0; - Iterator iter = associations.iterator(); - while ( iter.hasNext() ) { - JoinableAssociationImpl oj = (JoinableAssociationImpl) iter.next(); - if ( oj.getJoinType()==JoinType.LEFT_OUTER_JOIN && - oj.getJoinable().isCollection() && - ! oj.hasRestriction() ) { - result++; - } - } - return result; - } - /** * Get the order by string required for collection fetching */ - protected static final String orderBy(List associations) + protected static String orderBy(List associations) throws MappingException { StringBuilder buf = new StringBuilder(); JoinableAssociationImpl last = null; @@ -156,7 +129,9 @@ public abstract class AbstractLoadQueryImpl { } last = oj; } - if ( buf.length()>0 ) buf.setLength( buf.length()-2 ); + if ( buf.length() > 0 ) { + buf.setLength( buf.length() - 2 ); + } return buf.toString(); } @@ -168,7 +143,9 @@ public abstract class AbstractLoadQueryImpl { // if not a composite key, use "foo in (?, ?, ?)" for batching // if no batch, and not a composite key, use "foo = ?" InFragment in = new InFragment().setColumn( alias, columnNames[0] ); - for ( int i=0; i= suffixes.size() ) - ? null - : suffixes.get( entityAliasCount ); - final String collectionSuffix = ( collectionSuffixes == null || collectionAliasCount >= collectionSuffixes.length ) - ? null - : collectionSuffixes[collectionAliasCount]; final String selectFragment = joinable.selectFragment( next == null ? null : next.getJoinable(), next == null ? null : next.getRHSAlias(), join.getRHSAlias(), - entitySuffix, - collectionSuffix, + associations.get( i ).getCurrentEntitySuffix(), + associations.get( i ).getCurrentCollectionSuffix(), join.getJoinType()==JoinType.LEFT_OUTER_JOIN ); if (selectFragment.trim().length() > 0) { buf.append(", ").append(selectFragment); } - if ( joinable.consumesEntityAlias() ) entityAliasCount++; - if ( joinable.consumesCollectionAlias() && join.getJoinType()==JoinType.LEFT_OUTER_JOIN ) collectionAliasCount++; } return buf.toString(); } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/internal/EntityLoadQueryBuilderImpl.java b/hibernate-core/src/main/java/org/hibernate/loader/internal/EntityLoadQueryBuilderImpl.java index e0d6e2246f..be11977031 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/internal/EntityLoadQueryBuilderImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/internal/EntityLoadQueryBuilderImpl.java @@ -23,11 +23,15 @@ */ package org.hibernate.loader.internal; +import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Deque; import java.util.List; import org.hibernate.engine.spi.LoadQueryInfluencers; import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.loader.CollectionAliases; +import org.hibernate.loader.EntityAliases; import org.hibernate.loader.plan.spi.CollectionFetch; import org.hibernate.loader.plan.spi.CompositeFetch; import org.hibernate.loader.plan.spi.EntityFetch; @@ -35,8 +39,10 @@ import org.hibernate.loader.plan.spi.EntityReturn; import org.hibernate.loader.plan.spi.LoadPlan; import org.hibernate.loader.plan.spi.LoadPlanVisitationStrategyAdapter; import org.hibernate.loader.plan.spi.LoadPlanVisitor; +import org.hibernate.loader.plan.spi.Return; import org.hibernate.loader.spi.LoadQueryBuilder; import org.hibernate.persister.entity.OuterJoinLoadable; +import org.hibernate.persister.walking.spi.WalkingException; /** * @author Gail Badner @@ -46,7 +52,6 @@ public class EntityLoadQueryBuilderImpl implements LoadQueryBuilder { private final LoadQueryInfluencers loadQueryInfluencers; private final LoadPlan loadPlan; private final List associations; - private final List suffixes; public EntityLoadQueryBuilderImpl( SessionFactoryImplementor sessionFactory, @@ -58,7 +63,6 @@ public class EntityLoadQueryBuilderImpl implements LoadQueryBuilder { LocalVisitationStrategy strategy = new LocalVisitationStrategy(); LoadPlanVisitor.visit( loadPlan, strategy ); this.associations = strategy.associations; - this.suffixes = strategy.suffixes; } @Override @@ -70,8 +74,7 @@ public class EntityLoadQueryBuilderImpl implements LoadQueryBuilder { final EntityLoadQueryImpl loadQuery = new EntityLoadQueryImpl( sessionFactory, getRootEntityReturn(), - associations, - suffixes + associations ); return loadQuery.generateSql( uniqueKey, batchSize, getRootEntityReturn().getLockMode() ); } @@ -85,7 +88,8 @@ public class EntityLoadQueryBuilderImpl implements LoadQueryBuilder { } private class LocalVisitationStrategy extends LoadPlanVisitationStrategyAdapter { private final List associations = new ArrayList(); - private final List suffixes = new ArrayList(); + private Deque entityAliasStack = new ArrayDeque(); + private Deque collectionAliasStack = new ArrayDeque(); private EntityReturn entityRootReturn; @@ -94,32 +98,71 @@ public class EntityLoadQueryBuilderImpl implements LoadQueryBuilder { this.entityRootReturn = rootEntityReturn; } + @Override + public void startingRootReturn(Return rootReturn) { + if ( !EntityReturn.class.isInstance( rootReturn ) ) { + throw new WalkingException( + String.format( + "Unexpected type of return; expected [%s]; instead it was [%s]", + EntityReturn.class.getName(), + rootReturn.getClass().getName() + ) + ); + } + this.entityRootReturn = (EntityReturn) rootReturn; + pushToStack( entityAliasStack, entityRootReturn.getEntityAliases() ); + } + + @Override + public void finishingRootReturn(Return rootReturn) { + if ( !EntityReturn.class.isInstance( rootReturn ) ) { + throw new WalkingException( + String.format( + "Unexpected type of return; expected [%s]; instead it was [%s]", + EntityReturn.class.getName(), + rootReturn.getClass().getName() + ) + ); + } + popFromStack( entityAliasStack, ( (EntityReturn) rootReturn ).getEntityAliases() ); + } + @Override public void startingEntityFetch(EntityFetch entityFetch) { JoinableAssociationImpl assoc = new JoinableAssociationImpl( entityFetch, + getCurrentCollectionSuffix(), "", // getWithClause( entityFetch.getPropertyPath() ) false, // hasRestriction( entityFetch.getPropertyPath() ) sessionFactory, loadQueryInfluencers.getEnabledFilters() ); associations.add( assoc ); - suffixes.add( entityFetch.getEntityAliases().getSuffix() ); + pushToStack( entityAliasStack, entityFetch.getEntityAliases() ); } @Override public void finishingEntityFetch(EntityFetch entityFetch) { - //To change body of implemented methods use File | Settings | File Templates. + popFromStack( entityAliasStack, entityFetch.getEntityAliases() ); } @Override public void startingCollectionFetch(CollectionFetch collectionFetch) { - //To change body of implemented methods use File | Settings | File Templates. + JoinableAssociationImpl assoc = new JoinableAssociationImpl( + collectionFetch, + getCurrentEntitySuffix(), + "", // getWithClause( entityFetch.getPropertyPath() ) + false, // hasRestriction( entityFetch.getPropertyPath() ) + sessionFactory, + loadQueryInfluencers.getEnabledFilters() + ); + associations.add( assoc ); + pushToStack( collectionAliasStack, collectionFetch.getCollectionAliases() ); } @Override public void finishingCollectionFetch(CollectionFetch collectionFetch) { - //To change body of implemented methods use File | Settings | File Templates. + popFromStack( collectionAliasStack, collectionFetch.getCollectionAliases() ); } @Override @@ -134,7 +177,33 @@ public class EntityLoadQueryBuilderImpl implements LoadQueryBuilder { @Override public void finish(LoadPlan loadPlan) { - //suffixes.add( entityRootReturn.getEntityAliases().getSuffix() ); + entityAliasStack.clear(); + collectionAliasStack.clear(); + } + + private String getCurrentEntitySuffix() { + return entityAliasStack.peekFirst() == null ? null : entityAliasStack.peekFirst().getSuffix(); + } + + private String getCurrentCollectionSuffix() { + return collectionAliasStack.peekFirst() == null ? null : collectionAliasStack.peekFirst().getSuffix(); + } + + private void pushToStack(Deque stack, T value) { + stack.push( value ); + } + + private void popFromStack(Deque stack, T expectedValue) { + T poppedValue = stack.pop(); + if ( poppedValue != expectedValue ) { + throw new WalkingException( + String.format( + "Unexpected value from stack. Expected=[%s]; instead it was [%s].", + expectedValue, + poppedValue + ) + ); + } } } } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/internal/EntityLoadQueryImpl.java b/hibernate-core/src/main/java/org/hibernate/loader/internal/EntityLoadQueryImpl.java index a5fe1f22d4..2eedf8a88f 100755 --- a/hibernate-core/src/main/java/org/hibernate/loader/internal/EntityLoadQueryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/internal/EntityLoadQueryImpl.java @@ -42,9 +42,8 @@ public class EntityLoadQueryImpl extends AbstractEntityLoadQueryImpl { public EntityLoadQueryImpl( final SessionFactoryImplementor factory, EntityReturn entityReturn, - List associations, - List suffixes) throws MappingException { - super( factory, entityReturn, associations, suffixes ); + List associations) throws MappingException { + super( factory, entityReturn, associations ); } public String generateSql(String[] uniqueKey, int batchSize, LockMode lockMode) { diff --git a/hibernate-core/src/main/java/org/hibernate/loader/internal/JoinableAssociationImpl.java b/hibernate-core/src/main/java/org/hibernate/loader/internal/JoinableAssociationImpl.java index c853dba050..da8361304e 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/internal/JoinableAssociationImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/internal/JoinableAssociationImpl.java @@ -31,6 +31,7 @@ import org.hibernate.engine.FetchStyle; import org.hibernate.engine.internal.JoinHelper; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.loader.PropertyPath; +import org.hibernate.loader.plan.spi.CollectionFetch; import org.hibernate.loader.plan.spi.EntityFetch; import org.hibernate.loader.plan.spi.EntityReference; import org.hibernate.persister.collection.QueryableCollection; @@ -40,6 +41,7 @@ import org.hibernate.persister.entity.OuterJoinLoadable; import org.hibernate.sql.JoinFragment; import org.hibernate.sql.JoinType; import org.hibernate.type.AssociationType; +import org.hibernate.type.CollectionType; import org.hibernate.type.EntityType; /** @@ -57,6 +59,8 @@ public final class JoinableAssociationImpl { private final String[] lhsColumns; // belong to other persister private final String rhsAlias; private final String[] rhsColumns; + private final String currentEntitySuffix; + private final String currentCollectionSuffix; private final JoinType joinType; private final String on; private final Map enabledFilters; @@ -64,6 +68,7 @@ public final class JoinableAssociationImpl { public JoinableAssociationImpl( EntityFetch entityFetch, + String currentCollectionSuffix, String withClause, boolean hasRestriction, SessionFactoryImplementor factory, @@ -94,7 +99,52 @@ public final class JoinableAssociationImpl { joinType = JoinType.NONE; } this.joinable = joinableType.getAssociatedJoinable(factory); - this.rhsColumns = JoinHelper.getRHSColumnNames(joinableType, factory); + this.rhsColumns = JoinHelper.getRHSColumnNames( joinableType, factory ); + this.currentEntitySuffix = entityFetch.getEntityAliases().getSuffix(); + this.currentCollectionSuffix = currentCollectionSuffix; + this.on = joinableType.getOnCondition( rhsAlias, factory, enabledFilters ) + + ( withClause == null || withClause.trim().length() == 0 ? "" : " and ( " + withClause + " )" ); + this.hasRestriction = hasRestriction; + this.enabledFilters = enabledFilters; // needed later for many-to-many/filter application + } + + public JoinableAssociationImpl( + CollectionFetch collectionFetch, + String currentEntitySuffix, + String withClause, + boolean hasRestriction, + SessionFactoryImplementor factory, + Map enabledFilters) throws MappingException { + this.propertyPath = collectionFetch.getPropertyPath(); + final CollectionType collectionType = collectionFetch.getCollectionPersister().getCollectionType(); + this.joinableType = collectionType; + // TODO: this is not correct + final EntityPersister fetchSourcePersister = collectionFetch.getOwner().retrieveFetchSourcePersister(); + final int propertyNumber = fetchSourcePersister.getEntityMetamodel().getPropertyIndex( collectionFetch.getOwnerPropertyName() ); + + if ( EntityReference.class.isInstance( collectionFetch.getOwner() ) ) { + this.lhsAlias = ( (EntityReference) collectionFetch.getOwner() ).getSqlTableAlias(); + } + else { + throw new NotYetImplementedException( "Cannot determine LHS alias for a FetchOwner that is not an EntityReference." ); + } + final OuterJoinLoadable ownerPersister = (OuterJoinLoadable) collectionFetch.getOwner().retrieveFetchSourcePersister(); + this.lhsColumns = JoinHelper.getAliasedLHSColumnNames( + collectionType, lhsAlias, propertyNumber, ownerPersister, factory + ); + this.rhsAlias = collectionFetch.getAlias(); + + final boolean isNullable = ownerPersister.isSubclassPropertyNullable( propertyNumber ); + if ( collectionFetch.getFetchStrategy().getStyle() == FetchStyle.JOIN ) { + joinType = isNullable ? JoinType.LEFT_OUTER_JOIN : JoinType.INNER_JOIN; + } + else { + joinType = JoinType.NONE; + } + this.joinable = joinableType.getAssociatedJoinable(factory); + this.rhsColumns = JoinHelper.getRHSColumnNames( joinableType, factory ); + this.currentEntitySuffix = currentEntitySuffix; + this.currentCollectionSuffix = collectionFetch.getCollectionAliases().getSuffix(); this.on = joinableType.getOnCondition( rhsAlias, factory, enabledFilters ) + ( withClause == null || withClause.trim().length() == 0 ? "" : " and ( " + withClause + " )" ); this.hasRestriction = hasRestriction; @@ -117,6 +167,14 @@ public final class JoinableAssociationImpl { return rhsAlias; } + public String getCurrentEntitySuffix() { + return currentEntitySuffix; + } + + public String getCurrentCollectionSuffix() { + return currentCollectionSuffix; + } + private boolean isOneToOne() { if ( joinableType.isEntityType() ) { EntityType etype = (EntityType) joinableType; diff --git a/hibernate-core/src/main/java/org/hibernate/loader/internal/ResultSetProcessingContextImpl.java b/hibernate-core/src/main/java/org/hibernate/loader/internal/ResultSetProcessingContextImpl.java index 1163ef8125..a66e8acbc9 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/internal/ResultSetProcessingContextImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/internal/ResultSetProcessingContextImpl.java @@ -40,15 +40,18 @@ import org.hibernate.HibernateException; import org.hibernate.LockMode; import org.hibernate.StaleObjectStateException; import org.hibernate.WrongClassException; +import org.hibernate.collection.spi.PersistentCollection; import org.hibernate.engine.internal.TwoPhaseLoad; import org.hibernate.engine.spi.EntityKey; import org.hibernate.engine.spi.EntityUniqueKey; +import org.hibernate.engine.spi.PersistenceContext; import org.hibernate.engine.spi.QueryParameters; import org.hibernate.engine.spi.SessionImplementor; import org.hibernate.engine.spi.SubselectFetch; import org.hibernate.event.spi.EventSource; import org.hibernate.event.spi.PostLoadEvent; import org.hibernate.event.spi.PreLoadEvent; +import org.hibernate.loader.CollectionAliases; import org.hibernate.loader.EntityAliases; import org.hibernate.loader.plan.spi.CollectionFetch; import org.hibernate.loader.plan.spi.CollectionReturn; @@ -463,6 +466,113 @@ public class ResultSetProcessingContextImpl implements ResultSetProcessingContex } + public void readCollectionElements(final Object[] row) { + LoadPlanVisitor.visit( + loadPlan, + new LoadPlanVisitationStrategyAdapter() { + @Override + public void handleCollectionReturn(CollectionReturn rootCollectionReturn) { + readCollectionElement( + null, + null, + rootCollectionReturn.getCollectionPersister(), + rootCollectionReturn.getCollectionAliases(), + resultSet, + session + ); + } + + @Override + public void startingCollectionFetch(CollectionFetch collectionFetch) { + // TODO: determine which element is the owner. + final Object owner = row[ 0 ]; + readCollectionElement( + owner, + collectionFetch.getCollectionPersister().getCollectionType().getKeyOfOwner( owner, session ), + collectionFetch.getCollectionPersister(), + collectionFetch.getCollectionAliases(), + resultSet, + session + ); + } + + private void readCollectionElement( + final Object optionalOwner, + final Serializable optionalKey, + final CollectionPersister persister, + final CollectionAliases descriptor, + final ResultSet rs, + final SessionImplementor session) { + + try { + final PersistenceContext persistenceContext = session.getPersistenceContext(); + + final Serializable collectionRowKey = (Serializable) persister.readKey( + rs, + descriptor.getSuffixedKeyAliases(), + session + ); + + if ( collectionRowKey != null ) { + // we found a collection element in the result set + + if ( LOG.isDebugEnabled() ) { + LOG.debugf( "Found row of collection: %s", + MessageHelper.collectionInfoString( persister, collectionRowKey, session.getFactory() ) ); + } + + Object owner = optionalOwner; + if ( owner == null ) { + owner = persistenceContext.getCollectionOwner( collectionRowKey, persister ); + if ( owner == null ) { + //TODO: This is assertion is disabled because there is a bug that means the + // original owner of a transient, uninitialized collection is not known + // if the collection is re-referenced by a different object associated + // with the current Session + //throw new AssertionFailure("bug loading unowned collection"); + } + } + + PersistentCollection rowCollection = persistenceContext.getLoadContexts() + .getCollectionLoadContext( rs ) + .getLoadingCollection( persister, collectionRowKey ); + + if ( rowCollection != null ) { + rowCollection.readFrom( rs, persister, descriptor, owner ); + } + + } + else if ( optionalKey != null ) { + // we did not find a collection element in the result set, so we + // ensure that a collection is created with the owner's identifier, + // since what we have is an empty collection + + if ( LOG.isDebugEnabled() ) { + LOG.debugf( "Result set contains (possibly empty) collection: %s", + MessageHelper.collectionInfoString( persister, optionalKey, session.getFactory() ) ); + } + + persistenceContext.getLoadContexts() + .getCollectionLoadContext( rs ) + .getLoadingCollection( persister, optionalKey ); // handle empty collection + + } + + // else no collection element, but also no owner + } + catch ( SQLException sqle ) { + // TODO: would be nice to have the SQL string that failed... + throw session.getFactory().getSQLExceptionHelper().convert( + sqle, + "could not read next row of results" + ); + } + } + + } + ); + } + @Override public void registerHydratedEntity(EntityPersister persister, EntityKey entityKey, Object entityInstance) { if ( currentRowHydratedEntityRegistrationList == null ) { @@ -488,19 +598,18 @@ public class ResultSetProcessingContextImpl implements ResultSetProcessingContex // managing the map forms needed for subselect fetch generation ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - if ( ! hadSubselectFetches ) { - return; - } - if ( subselectLoadableEntityKeyMap == null ) { - subselectLoadableEntityKeyMap = new HashMap>(); - } - for ( HydratedEntityRegistration registration : currentRowHydratedEntityRegistrationList ) { - Set entityKeys = subselectLoadableEntityKeyMap.get( registration.persister ); - if ( entityKeys == null ) { - entityKeys = new HashSet(); - subselectLoadableEntityKeyMap.put( registration.persister, entityKeys ); + if ( hadSubselectFetches ) { + if ( subselectLoadableEntityKeyMap == null ) { + subselectLoadableEntityKeyMap = new HashMap>(); + } + for ( HydratedEntityRegistration registration : currentRowHydratedEntityRegistrationList ) { + Set entityKeys = subselectLoadableEntityKeyMap.get( registration.persister ); + if ( entityKeys == null ) { + entityKeys = new HashSet(); + subselectLoadableEntityKeyMap.put( registration.persister, entityKeys ); + } + entityKeys.add( registration.key ); } - entityKeys.add( registration.key ); } // release the currentRowHydratedEntityRegistrationList entries @@ -602,15 +711,15 @@ public class ResultSetProcessingContextImpl implements ResultSetProcessingContex new LoadPlanVisitationStrategyAdapter() { @Override public void handleCollectionReturn(CollectionReturn rootCollectionReturn) { - endLoadingArray( rootCollectionReturn.getCollectionPersister() ); + endLoadingCollection( rootCollectionReturn.getCollectionPersister() ); } @Override public void startingCollectionFetch(CollectionFetch collectionFetch) { - endLoadingArray( collectionFetch.getCollectionPersister() ); + endLoadingCollection( collectionFetch.getCollectionPersister() ); } - private void endLoadingArray(CollectionPersister persister) { + private void endLoadingCollection(CollectionPersister persister) { if ( ! persister.isArray() ) { session.getPersistenceContext() .getLoadContexts() diff --git a/hibernate-core/src/main/java/org/hibernate/loader/internal/ResultSetProcessorImpl.java b/hibernate-core/src/main/java/org/hibernate/loader/internal/ResultSetProcessorImpl.java index 479f12e6b9..e9176048e6 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/internal/ResultSetProcessorImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/internal/ResultSetProcessorImpl.java @@ -126,6 +126,7 @@ public class ResultSetProcessorImpl implements ResultSetProcessor { loadPlan.getReturns().get( 0 ).resolve( resultSet, context ); logicalRow = loadPlan.getReturns().get( 0 ).read( resultSet, context ); + context.readCollectionElements( new Object[] { logicalRow } ); } else { for ( Return rootReturn : loadPlan.getReturns() ) { @@ -141,6 +142,7 @@ public class ResultSetProcessorImpl implements ResultSetProcessor { ( (Object[]) logicalRow )[pos] = rootReturn.read( resultSet, context ); pos++; } + context.readCollectionElements( (Object[]) logicalRow ); } // todo : apply transformers here? diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan/internal/LoadPlanBuildingHelper.java b/hibernate-core/src/main/java/org/hibernate/loader/plan/internal/LoadPlanBuildingHelper.java index 35c8df1d63..0c1cc20723 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan/internal/LoadPlanBuildingHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan/internal/LoadPlanBuildingHelper.java @@ -34,6 +34,7 @@ import org.hibernate.loader.plan.spi.EntityFetch; import org.hibernate.loader.plan.spi.FetchOwner; import org.hibernate.loader.plan.spi.LoadPlanBuildingContext; import org.hibernate.persister.walking.spi.AssociationAttributeDefinition; +import org.hibernate.persister.walking.spi.CollectionDefinition; import org.hibernate.persister.walking.spi.CompositionDefinition; /** @@ -46,7 +47,11 @@ public class LoadPlanBuildingHelper { FetchStrategy fetchStrategy, LoadPlanBuildingContext loadPlanBuildingContext) { final CollectionAliases collectionAliases = loadPlanBuildingContext.resolveCollectionColumnAliases( attributeDefinition ); - final EntityAliases elementEntityAliases = loadPlanBuildingContext.resolveEntityColumnAliases( attributeDefinition ); + final CollectionDefinition collectionDefinition = attributeDefinition.toCollectionDefinition(); + final EntityAliases elementEntityAliases = + collectionDefinition.getElementDefinition().getType().isEntityType() ? + loadPlanBuildingContext.resolveEntityColumnAliases( attributeDefinition ) : + null; return new CollectionFetch( loadPlanBuildingContext.getSessionFactory(), diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/AbstractLoadPlanBuilderStrategy.java b/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/AbstractLoadPlanBuilderStrategy.java index 405b318dcc..a1162cbe33 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/AbstractLoadPlanBuilderStrategy.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/AbstractLoadPlanBuilderStrategy.java @@ -399,6 +399,7 @@ public abstract class AbstractLoadPlanBuilderStrategy implements LoadPlanBuilder final Fetch associationFetch; if ( attributeDefinition.isCollection() ) { associationFetch = fetchOwner.buildCollectionFetch( attributeDefinition, fetchStrategy, this ); + pushToCollectionStack( (CollectionReference) associationFetch ); } else { associationFetch = fetchOwner.buildEntityFetch( diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/CollectionFetch.java b/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/CollectionFetch.java index 89d82d4370..ab4e012ece 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/CollectionFetch.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/CollectionFetch.java @@ -62,6 +62,8 @@ public class CollectionFetch extends AbstractCollectionReference implements Fetc ); this.fetchOwner = fetchOwner; this.fetchStrategy = fetchStrategy; + + fetchOwner.addFetch( this ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/LoadPlanVisitor.java b/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/LoadPlanVisitor.java index 9a371aa011..304ace31ec 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/LoadPlanVisitor.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/LoadPlanVisitor.java @@ -82,13 +82,15 @@ public class LoadPlanVisitor { } private void visitFetches(FetchOwner fetchOwner) { - strategy.startingFetches( fetchOwner ); + if ( fetchOwner != null ) { + strategy.startingFetches( fetchOwner ); - for ( Fetch fetch : fetchOwner.getFetches() ) { - visitFetch( fetch ); + for ( Fetch fetch : fetchOwner.getFetches() ) { + visitFetch( fetch ); + } + + strategy.finishingFetches( fetchOwner ); } - - strategy.finishingFetches( fetchOwner ); } private void visitFetch(Fetch fetch) { diff --git a/hibernate-core/src/main/java/org/hibernate/persister/walking/spi/MetadataDrivenModelGraphVisitor.java b/hibernate-core/src/main/java/org/hibernate/persister/walking/spi/MetadataDrivenModelGraphVisitor.java index dd205aa9e8..f22d172bec 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/walking/spi/MetadataDrivenModelGraphVisitor.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/walking/spi/MetadataDrivenModelGraphVisitor.java @@ -199,7 +199,7 @@ public class MetadataDrivenModelGraphVisitor { if ( elementDefinition.getType().isComponentType() ) { visitCompositeDefinition( elementDefinition.toCompositeDefinition() ); } - else { + else if ( elementDefinition.getType().isEntityType() ) { visitEntityDefinition( elementDefinition.toEntityDefinition() ); } diff --git a/hibernate-core/src/test/java/org/hibernate/loader/AssociationResultSetProcessorTest.java b/hibernate-core/src/test/java/org/hibernate/loader/EntityAssociationResultSetProcessorTest.java similarity index 98% rename from hibernate-core/src/test/java/org/hibernate/loader/AssociationResultSetProcessorTest.java rename to hibernate-core/src/test/java/org/hibernate/loader/EntityAssociationResultSetProcessorTest.java index b41cf73572..f8eb3a0bcc 100644 --- a/hibernate-core/src/test/java/org/hibernate/loader/AssociationResultSetProcessorTest.java +++ b/hibernate-core/src/test/java/org/hibernate/loader/EntityAssociationResultSetProcessorTest.java @@ -60,9 +60,9 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; /** - * @author Steve Ebersole + * @author Gail Badner */ -public class AssociationResultSetProcessorTest extends BaseCoreFunctionalTestCase { +public class EntityAssociationResultSetProcessorTest extends BaseCoreFunctionalTestCase { @Override protected Class[] getAnnotatedClasses() { diff --git a/hibernate-core/src/test/java/org/hibernate/loader/EntityWithCollectionResultSetProcessorTest.java b/hibernate-core/src/test/java/org/hibernate/loader/EntityWithCollectionResultSetProcessorTest.java new file mode 100644 index 0000000000..d83804e815 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/loader/EntityWithCollectionResultSetProcessorTest.java @@ -0,0 +1,174 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2013, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.loader; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import javax.persistence.CascadeType; +import javax.persistence.ElementCollection; +import javax.persistence.Entity; +import javax.persistence.FetchType; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.ManyToOne; +import javax.persistence.OneToMany; + +import org.junit.Test; + +import org.hibernate.Hibernate; +import org.hibernate.Session; +import org.hibernate.engine.spi.LoadQueryInfluencers; +import org.hibernate.engine.spi.QueryParameters; +import org.hibernate.engine.spi.SessionImplementor; +import org.hibernate.internal.util.collections.IdentitySet; +import org.hibernate.jdbc.Work; +import org.hibernate.loader.internal.EntityLoadQueryBuilderImpl; +import org.hibernate.loader.internal.ResultSetProcessorImpl; +import org.hibernate.loader.plan.internal.SingleRootReturnLoadPlanBuilderStrategy; +import org.hibernate.loader.plan.spi.LoadPlan; +import org.hibernate.loader.plan.spi.LoadPlanBuilder; +import org.hibernate.loader.spi.NamedParameterContext; +import org.hibernate.persister.entity.EntityPersister; +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.hibernate.testing.junit4.ExtraAssertions; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +/** + * @author Gail Badner + */ +public class EntityWithCollectionResultSetProcessorTest extends BaseCoreFunctionalTestCase { + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { Person.class }; + } + + @Test + public void testEntityWithSet() throws Exception { + final EntityPersister entityPersister = sessionFactory().getEntityPersister( Person.class.getName() ); + + // create some test data + Session session = openSession(); + session.beginTransaction(); + Person person = new Person(); + person.id = 1; + person.name = "John Doe"; + person.nickNames.add( "Jack" ); + person.nickNames.add( "Johnny" ); + session.save( person ); + session.getTransaction().commit(); + session.close(); + + { + final SingleRootReturnLoadPlanBuilderStrategy strategy = new SingleRootReturnLoadPlanBuilderStrategy( + sessionFactory(), + LoadQueryInfluencers.NONE, + "abc", + 0 + ); + final LoadPlan plan = LoadPlanBuilder.buildRootEntityLoadPlan( strategy, entityPersister ); + final EntityLoadQueryBuilderImpl queryBuilder = new EntityLoadQueryBuilderImpl( + sessionFactory(), + LoadQueryInfluencers.NONE, + plan + ); + final String sql = queryBuilder.generateSql( 1 ); + + final ResultSetProcessorImpl resultSetProcessor = new ResultSetProcessorImpl( plan ); + final List results = new ArrayList(); + + final Session workSession = openSession(); + workSession.beginTransaction(); + workSession.doWork( + new Work() { + @Override + public void execute(Connection connection) throws SQLException { + PreparedStatement ps = connection.prepareStatement( sql ); + ps.setInt( 1, 1 ); + ResultSet resultSet = ps.executeQuery(); + results.addAll( + resultSetProcessor.extractResults( + resultSet, + (SessionImplementor) workSession, + new QueryParameters(), + new NamedParameterContext() { + @Override + public int[] getNamedParameterLocations(String name) { + return new int[0]; + } + }, + true, + false, + null, + null + ) + ); + resultSet.close(); + ps.close(); + } + } + ); + assertEquals( 2, results.size() ); + Object result1 = results.get( 0 ); + assertSame( result1, results.get( 1 ) ); + assertNotNull( result1 ); + + Person workPerson = ExtraAssertions.assertTyping( Person.class, result1 ); + assertEquals( 1, workPerson.id.intValue() ); + assertEquals( person.name, workPerson.name ); + assertTrue( Hibernate.isInitialized( workPerson.nickNames ) ); + assertEquals( 2, workPerson.nickNames.size() ); + assertEquals( person.nickNames, workPerson.nickNames ); + workSession.getTransaction().commit(); + workSession.close(); + } + + // clean up test data + session = openSession(); + session.beginTransaction(); + session.delete( person ); + session.getTransaction().commit(); + session.close(); + } + + @Entity( name = "Person" ) + public static class Person { + @Id + private Integer id; + private String name; + @ElementCollection( fetch = FetchType.EAGER ) + private Set nickNames = new HashSet(); + } +}