Skip to content

Commit

Permalink
HHH-13059 Partially revert HHH-12594
Browse files Browse the repository at this point in the history
The first commit was on the safe side, we decided to go the extra mile
and that was a mistake as we missed all the consequences.

The new issue is about having a shared ReaderCollector: we add the info
there for each batch which leads to collecting the elements several
times.

This reverts commit a19fc84.

HHH-13059 : Correct Javadoc
  • Loading branch information
gsmet authored and gbadner committed Jan 16, 2019
1 parent 27ddc8e commit e6286e0
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 96 deletions.
Expand Up @@ -40,7 +40,6 @@ public abstract class AbstractLoadQueryDetails implements LoadQueryDetails {
private final String[] keyColumnNames; private final String[] keyColumnNames;
private final Return rootReturn; private final Return rootReturn;
private final LoadQueryJoinAndFetchProcessor queryProcessor; private final LoadQueryJoinAndFetchProcessor queryProcessor;
private final QueryBuildingParameters buildingParameters;
private String sqlStatement; private String sqlStatement;
private ResultSetProcessor resultSetProcessor; private ResultSetProcessor resultSetProcessor;


Expand All @@ -59,30 +58,7 @@ protected AbstractLoadQueryDetails(
this.keyColumnNames = keyColumnNames; this.keyColumnNames = keyColumnNames;
this.rootReturn = rootReturn; this.rootReturn = rootReturn;
this.loadPlan = loadPlan; this.loadPlan = loadPlan;
this.queryProcessor = new LoadQueryJoinAndFetchProcessor( this.queryProcessor = new LoadQueryJoinAndFetchProcessor( aliasResolutionContext, buildingParameters, factory );
aliasResolutionContext, buildingParameters.getQueryInfluencers(), factory
);
this.buildingParameters = buildingParameters;
}

/**
* Constructs an AbstractLoadQueryDetails object from an initial object and new building parameters,
* with the guarantee that only batch size changed between the initial parameters and the new ones.
*
* @param initialLoadQueryDetails The initial object to be copied
* @param buildingParameters The new building parameters, with only the batch size being different
* from the parameters used in the initial object.
*/
protected AbstractLoadQueryDetails(
AbstractLoadQueryDetails initialLoadQueryDetails,
QueryBuildingParameters buildingParameters) {
this.keyColumnNames = initialLoadQueryDetails.keyColumnNames;
this.rootReturn = initialLoadQueryDetails.rootReturn;
this.loadPlan = initialLoadQueryDetails.loadPlan;
this.queryProcessor = new LoadQueryJoinAndFetchProcessor(
initialLoadQueryDetails.queryProcessor, buildingParameters.getQueryInfluencers()
);
this.buildingParameters = buildingParameters;
} }


protected QuerySpace getQuerySpace(String querySpaceUid) { protected QuerySpace getQuerySpace(String querySpaceUid) {
Expand All @@ -108,7 +84,7 @@ protected final AliasResolutionContext getAliasResolutionContext() {
} }


protected final QueryBuildingParameters getQueryBuildingParameters() { protected final QueryBuildingParameters getQueryBuildingParameters() {
return buildingParameters; return queryProcessor.getQueryBuildingParameters();
} }


protected final SessionFactoryImplementor getSessionFactory() { protected final SessionFactoryImplementor getSessionFactory() {
Expand Down
Expand Up @@ -85,24 +85,17 @@ protected EntityLoadQueryDetails(
generate(); generate();
} }


/**
* Constructs an EntityLoadQueryDetails object from an initial object and new building parameters,
* with the guarantee that only batch size changed between the initial parameters and the new ones.
*
* @param initialEntityLoadQueryDetails The initial object to be copied
* @param buildingParameters The new building parameters, with only the batch size being different
* from the parameters used in the initial object.
*/
protected EntityLoadQueryDetails( protected EntityLoadQueryDetails(
EntityLoadQueryDetails initialEntityLoadQueryDetails, EntityLoadQueryDetails initialEntityLoadQueryDetails,
QueryBuildingParameters buildingParameters) { QueryBuildingParameters buildingParameters) {
super( this(
initialEntityLoadQueryDetails, initialEntityLoadQueryDetails.getLoadPlan(),
buildingParameters initialEntityLoadQueryDetails.getKeyColumnNames(),
new AliasResolutionContextImpl( initialEntityLoadQueryDetails.getSessionFactory() ),
(EntityReturn) initialEntityLoadQueryDetails.getRootReturn(),
buildingParameters,
initialEntityLoadQueryDetails.getSessionFactory()
); );
this.entityReferenceAliases = initialEntityLoadQueryDetails.entityReferenceAliases;
this.readerCollector = initialEntityLoadQueryDetails.readerCollector;
generate();
} }


public boolean hasCollectionInitializers() { public boolean hasCollectionInitializers() {
Expand Down
Expand Up @@ -13,14 +13,14 @@
import org.hibernate.AssertionFailure; import org.hibernate.AssertionFailure;
import org.hibernate.engine.FetchStyle; import org.hibernate.engine.FetchStyle;
import org.hibernate.engine.FetchTiming; import org.hibernate.engine.FetchTiming;
import org.hibernate.engine.spi.LoadQueryInfluencers;
import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.internal.CoreLogging; import org.hibernate.internal.CoreLogging;
import org.hibernate.internal.util.StringHelper; import org.hibernate.internal.util.StringHelper;
import org.hibernate.loader.plan.exec.process.internal.CollectionReferenceInitializerImpl; import org.hibernate.loader.plan.exec.process.internal.CollectionReferenceInitializerImpl;
import org.hibernate.loader.plan.exec.process.internal.EntityReferenceInitializerImpl; import org.hibernate.loader.plan.exec.process.internal.EntityReferenceInitializerImpl;
import org.hibernate.loader.plan.exec.process.spi.ReaderCollector; import org.hibernate.loader.plan.exec.process.spi.ReaderCollector;
import org.hibernate.loader.plan.exec.query.internal.SelectStatementBuilder; import org.hibernate.loader.plan.exec.query.internal.SelectStatementBuilder;
import org.hibernate.loader.plan.exec.query.spi.QueryBuildingParameters;
import org.hibernate.loader.plan.exec.spi.AliasResolutionContext; import org.hibernate.loader.plan.exec.spi.AliasResolutionContext;
import org.hibernate.loader.plan.exec.spi.CollectionReferenceAliases; import org.hibernate.loader.plan.exec.spi.CollectionReferenceAliases;
import org.hibernate.loader.plan.exec.spi.EntityReferenceAliases; import org.hibernate.loader.plan.exec.spi.EntityReferenceAliases;
Expand Down Expand Up @@ -66,51 +66,34 @@
public class LoadQueryJoinAndFetchProcessor { public class LoadQueryJoinAndFetchProcessor {
private static final Logger LOG = CoreLogging.logger( LoadQueryJoinAndFetchProcessor.class ); private static final Logger LOG = CoreLogging.logger( LoadQueryJoinAndFetchProcessor.class );


private final AliasResolutionContext aliasResolutionContext; private final AliasResolutionContextImpl aliasResolutionContext;
private final AliasResolutionContextImpl mutableAliasResolutionContext; private final QueryBuildingParameters buildingParameters;
private final LoadQueryInfluencers queryInfluencers;
private final SessionFactoryImplementor factory; private final SessionFactoryImplementor factory;


/** /**
* Instantiates a LoadQueryJoinAndFetchProcessor with the given information * Instantiates a LoadQueryJoinAndFetchProcessor with the given information
* *
* @param mutableAliasResolutionContext * @param aliasResolutionContext
* @param queryInfluencers * @param buildingParameters
* @param factory * @param factory
*/ */
public LoadQueryJoinAndFetchProcessor( public LoadQueryJoinAndFetchProcessor(
AliasResolutionContextImpl mutableAliasResolutionContext, AliasResolutionContextImpl aliasResolutionContext,
LoadQueryInfluencers queryInfluencers, QueryBuildingParameters buildingParameters,
SessionFactoryImplementor factory) { SessionFactoryImplementor factory) {
this.aliasResolutionContext = mutableAliasResolutionContext; this.aliasResolutionContext = aliasResolutionContext;
this.mutableAliasResolutionContext = mutableAliasResolutionContext; this.buildingParameters = buildingParameters;
this.queryInfluencers = queryInfluencers;
this.factory = factory; this.factory = factory;
} }


/**
* Instantiates a LoadQueryJoinAndFetchProcessor from an initial object and new query influencers.
*
* Aliases are considered already contributed to the initial objects alias resolution context
* and won't be added again.
*
* @param initialLoadQueryJoinAndFetchProcessor The initial object to be copied
* @param queryInfluencers The new query influencers
*/
public LoadQueryJoinAndFetchProcessor(
LoadQueryJoinAndFetchProcessor initialLoadQueryJoinAndFetchProcessor,
LoadQueryInfluencers queryInfluencers) {
this.aliasResolutionContext = initialLoadQueryJoinAndFetchProcessor.aliasResolutionContext;
// Do not change the alias resolution context, it should be pre-populated
this.mutableAliasResolutionContext = null;
this.queryInfluencers = queryInfluencers;
this.factory = initialLoadQueryJoinAndFetchProcessor.factory;
}

public AliasResolutionContext getAliasResolutionContext() { public AliasResolutionContext getAliasResolutionContext() {
return aliasResolutionContext; return aliasResolutionContext;
} }


public QueryBuildingParameters getQueryBuildingParameters() {
return buildingParameters;
}

public SessionFactoryImplementor getSessionFactory() { public SessionFactoryImplementor getSessionFactory() {
return factory; return factory;
} }
Expand Down Expand Up @@ -182,21 +165,16 @@ private void handleCompositeJoin(Join join, JoinFragment joinFragment) {
); );
} }


if ( mutableAliasResolutionContext != null ) { aliasResolutionContext.registerCompositeQuerySpaceUidResolution( rightHandSideUid, leftHandSideTableAlias );
mutableAliasResolutionContext.registerCompositeQuerySpaceUidResolution(
rightHandSideUid,
leftHandSideTableAlias
);
}
} }


private void renderEntityJoin(Join join, JoinFragment joinFragment) { private void renderEntityJoin(Join join, JoinFragment joinFragment) {
final EntityQuerySpace rightHandSide = (EntityQuerySpace) join.getRightHandSide(); final EntityQuerySpace rightHandSide = (EntityQuerySpace) join.getRightHandSide();


// see if there is already aliases registered for this entity query space (collection joins) // see if there is already aliases registered for this entity query space (collection joins)
EntityReferenceAliases aliases = aliasResolutionContext.resolveEntityReferenceAliases( rightHandSide.getUid() ); EntityReferenceAliases aliases = aliasResolutionContext.resolveEntityReferenceAliases( rightHandSide.getUid() );
if ( aliases == null && mutableAliasResolutionContext != null ) { if ( aliases == null ) {
mutableAliasResolutionContext.generateEntityReferenceAliases( aliasResolutionContext.generateEntityReferenceAliases(
rightHandSide.getUid(), rightHandSide.getUid(),
rightHandSide.getEntityPersister() rightHandSide.getEntityPersister()
); );
Expand Down Expand Up @@ -227,10 +205,10 @@ private String resolveAdditionalJoinCondition(String rhsTableAlias, String withC
// calls to the Joinable.filterFragment() method where the Joinable is either the entity or // calls to the Joinable.filterFragment() method where the Joinable is either the entity or
// collection persister // collection persister
final String filter = associationType!=null? final String filter = associationType!=null?
associationType.getOnCondition( rhsTableAlias, factory, queryInfluencers.getEnabledFilters() ): associationType.getOnCondition( rhsTableAlias, factory, buildingParameters.getQueryInfluencers().getEnabledFilters() ):
joinable.filterFragment( joinable.filterFragment(
rhsTableAlias, rhsTableAlias,
queryInfluencers.getEnabledFilters() buildingParameters.getQueryInfluencers().getEnabledFilters()
); );


if ( StringHelper.isEmpty( withClause ) && StringHelper.isEmpty( filter ) ) { if ( StringHelper.isEmpty( withClause ) && StringHelper.isEmpty( filter ) ) {
Expand Down Expand Up @@ -317,19 +295,7 @@ else if ( !StringHelper.isEmpty( joinConditions ) ) {


private void renderCollectionJoin(Join join, JoinFragment joinFragment) { private void renderCollectionJoin(Join join, JoinFragment joinFragment) {
final CollectionQuerySpace rightHandSide = (CollectionQuerySpace) join.getRightHandSide(); final CollectionQuerySpace rightHandSide = (CollectionQuerySpace) join.getRightHandSide();
if ( mutableAliasResolutionContext != null ) {
registerCollectionJoinAliases( mutableAliasResolutionContext, rightHandSide );
}
addJoins(
join,
joinFragment,
(Joinable) rightHandSide.getCollectionPersister(),
null
);
}


private void registerCollectionJoinAliases(AliasResolutionContextImpl mutableAliasResolutionContext,
CollectionQuerySpace rightHandSide) {
// The SQL join to the "collection table" needs to be rendered. // The SQL join to the "collection table" needs to be rendered.
// //
// In the case of a basic collection, that's the only join needed. // In the case of a basic collection, that's the only join needed.
Expand Down Expand Up @@ -385,14 +351,14 @@ private void registerCollectionJoinAliases(AliasResolutionContextImpl mutableAli
) )
); );
} }
mutableAliasResolutionContext.generateCollectionReferenceAliases( aliasResolutionContext.generateCollectionReferenceAliases(
rightHandSide.getUid(), rightHandSide.getUid(),
rightHandSide.getCollectionPersister(), rightHandSide.getCollectionPersister(),
collectionElementJoin.getRightHandSide().getUid() collectionElementJoin.getRightHandSide().getUid()
); );
} }
else { else {
mutableAliasResolutionContext.generateCollectionReferenceAliases( aliasResolutionContext.generateCollectionReferenceAliases(
rightHandSide.getUid(), rightHandSide.getUid(),
rightHandSide.getCollectionPersister(), rightHandSide.getCollectionPersister(),
null null
Expand All @@ -412,11 +378,17 @@ private void registerCollectionJoinAliases(AliasResolutionContextImpl mutableAli
) )
); );
} }
mutableAliasResolutionContext.generateEntityReferenceAliases( aliasResolutionContext.generateEntityReferenceAliases(
collectionIndexJoin.getRightHandSide().getUid(), collectionIndexJoin.getRightHandSide().getUid(),
rightHandSide.getCollectionPersister().getIndexDefinition().toEntityDefinition().getEntityPersister() rightHandSide.getCollectionPersister().getIndexDefinition().toEntityDefinition().getEntityPersister()
); );
} }
addJoins(
join,
joinFragment,
(Joinable) rightHandSide.getCollectionPersister(),
null
);
} }


private void renderManyToManyJoin( private void renderManyToManyJoin(
Expand Down Expand Up @@ -447,7 +419,7 @@ private void renderManyToManyJoin(
final CollectionPersister persister = leftHandSide.getCollectionPersister(); final CollectionPersister persister = leftHandSide.getCollectionPersister();
manyToManyFilter = persister.getManyToManyFilterFragment( manyToManyFilter = persister.getManyToManyFilterFragment(
entityTableAlias, entityTableAlias,
queryInfluencers.getEnabledFilters() buildingParameters.getQueryInfluencers().getEnabledFilters()
); );
} }
else { else {
Expand Down

0 comments on commit e6286e0

Please sign in to comment.