HHH-7841 - Redesign Loader

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<JoinableAssociationImpl> associations,
+			List<JoinableAssociationImpl> associations) {
-			List<String> suffixes) {
+		super( factory, associations );
-		super( factory, associations, suffixes );
 		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(

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<JoinableAssociationImpl> associations;
-	private final List<String> suffixes;
-
-	private String[] collectionSuffixes;
 
 	protected AbstractLoadQueryImpl(
 			SessionFactoryImplementor factory,
-			List<JoinableAssociationImpl> associations,
+			List<JoinableAssociationImpl> associations) {
-			List<String> suffixes) {
 		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<JoinableAssociationImpl> associations)
+	protected static String orderBy(List<JoinableAssociationImpl> 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<batchSize; i++ ) in.addValue("?");
+			for ( int i = 0; i < batchSize; i++ ) {
+				in.addValue( "?" );
+			}
 			return new StringBuilder( in.toFragmentString() );
 		}
 		else {
@@ -207,33 +184,23 @@ public abstract class AbstractLoadQueryImpl {
 		}
 		else {
 			StringBuilder buf = new StringBuilder( associations.size() * 100 );
-			int entityAliasCount=0;
-			int collectionAliasCount=0;
 			for ( int i=0; i<associations.size(); i++ ) {
 				JoinableAssociationImpl join = associations.get(i);
 				JoinableAssociationImpl next = (i == associations.size() - 1)
 				        ? null
 				        : associations.get( i + 1 );
 				final Joinable joinable = join.getJoinable();
-				final String entitySuffix = ( suffixes == null || entityAliasCount >= 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,
+						associations.get( i ).getCurrentEntitySuffix(),
-				        collectionSuffix,
+						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();
 		}

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<JoinableAssociationImpl> associations;
-	private final List<String> 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,
+				associations
-				suffixes
 		);
 		return loadQuery.generateSql( uniqueKey, batchSize, getRootEntityReturn().getLockMode() );
 	}
@@ -85,7 +88,8 @@ public class EntityLoadQueryBuilderImpl implements LoadQueryBuilder {
 	}
 	private class LocalVisitationStrategy extends LoadPlanVisitationStrategyAdapter {
 		private final List<JoinableAssociationImpl> associations = new ArrayList<JoinableAssociationImpl>();
-		private final List<String> suffixes = new ArrayList<String>();
+		private Deque<EntityAliases> entityAliasStack = new ArrayDeque<EntityAliases>();
+		private Deque<CollectionAliases> collectionAliasStack = new ArrayDeque<CollectionAliases>();
 
 		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 <T> void pushToStack(Deque<T> stack, T value) {
+			stack.push( value );
+		}
+
+		private <T> void popFromStack(Deque<T> 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
+						)
+				);
+			}
 		}
 	}
 }

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<JoinableAssociationImpl> associations,
+			List<JoinableAssociationImpl> associations) throws MappingException {
-			List<String> suffixes) throws MappingException {
+		super( factory, entityReturn, associations );
-		super( factory, entityReturn, associations, suffixes );
 	}
 
 	public String generateSql(String[] uniqueKey, int batchSize, LockMode lockMode) {

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;

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 ) {
+		if ( hadSubselectFetches ) {
-			return;
+			if ( subselectLoadableEntityKeyMap == null ) {
-		}
+				subselectLoadableEntityKeyMap = new HashMap<EntityPersister, Set<EntityKey>>();
-		if ( subselectLoadableEntityKeyMap == null ) {
+			}
-			subselectLoadableEntityKeyMap = new HashMap<EntityPersister, Set<EntityKey>>();
+			for ( HydratedEntityRegistration registration : currentRowHydratedEntityRegistrationList ) {
-		}
+				Set<EntityKey> entityKeys = subselectLoadableEntityKeyMap.get( registration.persister );
-		for ( HydratedEntityRegistration registration : currentRowHydratedEntityRegistrationList ) {
+				if ( entityKeys == null ) {
-			Set<EntityKey> entityKeys = subselectLoadableEntityKeyMap.get( registration.persister );
+					entityKeys = new HashSet<EntityKey>();
-			if ( entityKeys == null ) {
+					subselectLoadableEntityKeyMap.put( registration.persister, entityKeys );
-				entityKeys = new HashSet<EntityKey>();
+				}
-				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()

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?

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(),

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(

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

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() ) {
+			for ( Fetch fetch : fetchOwner.getFetches() ) {
-			visitFetch( fetch );
+				visitFetch( fetch );
+			}
+
+			strategy.finishingFetches( fetchOwner );
 		}
-
-		strategy.finishingFetches( fetchOwner );
 	}
 
 	private void visitFetch(Fetch fetch) {

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

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() {

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<String> nickNames = new HashSet<String>();
+	}
+}