Skip to content

Commit

Permalink
HHH-16624 Do not create subselects when there are fewer than 2 results
Browse files Browse the repository at this point in the history
  • Loading branch information
dreab8 committed Jun 6, 2023
1 parent 4c1d8a1 commit f8275f1
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,12 @@
import java.util.Set;

import org.hibernate.spi.NavigablePath;
import org.hibernate.sql.ast.tree.expression.JdbcParameter;
import org.hibernate.sql.ast.tree.from.TableGroup;
import org.hibernate.sql.ast.tree.select.QuerySpec;
import org.hibernate.sql.ast.tree.select.SelectStatement;
import org.hibernate.sql.exec.spi.JdbcParameterBindings;
import org.hibernate.sql.exec.spi.JdbcParametersList;
import org.hibernate.sql.results.graph.DomainResult;
import org.hibernate.sql.results.graph.entity.LoadingEntityEntry;
import org.hibernate.sql.results.graph.entity.internal.EntityResultInitializer;

/**
* Encapsulates details related to entities which contain sub-select-fetchable
Expand Down Expand Up @@ -151,8 +148,7 @@ private StandardRegistrationHandler(

public void addKey(EntityKey key, LoadingEntityEntry entry) {
if ( batchFetchQueue.getSession().getLoadQueryInfluencers()
.hasSubselectLoadableCollections( entry.getDescriptor() )
&& shouldAddSubselectFetch( entry ) ) {
.hasSubselectLoadableCollections( entry.getDescriptor() ) ) {
final SubselectFetch subselectFetch = subselectFetches.computeIfAbsent(
entry.getEntityInitializer().getNavigablePath(),
navigablePath -> new SubselectFetch(
Expand All @@ -169,23 +165,5 @@ && shouldAddSubselectFetch( entry ) ) {
batchFetchQueue.addSubselect( key, subselectFetch );
}
}

private boolean shouldAddSubselectFetch(LoadingEntityEntry entry) {
if ( entry.getEntityInitializer() instanceof EntityResultInitializer ) {
return true;
}
else {
final NavigablePath entityInitializerParent = entry.getEntityInitializer().getNavigablePath().getParent();

// We want to add only the collections of the loading entities
for ( DomainResult<?> domainResult : loadingSqlAst.getDomainResultDescriptors() ) {
if ( domainResult.getNavigablePath().equals( entityInitializerParent ) ) {
return true;
}
}

return false;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,22 @@ default EntityMappingType getRootEntityDescriptor() {
return null;
}

/**
*
* @param entityKey
* @param entry
*
* @deprecated use {@link #registerSubselect(EntityKey, LoadingEntityEntry)} instead.
*/
@Deprecated
default void registerLoadingEntityEntry(EntityKey entityKey, LoadingEntityEntry entry) {
// by default do nothing
}

default void registerSubselect(EntityKey entityKey, LoadingEntityEntry entry) {
registerLoadingEntityEntry( entityKey, entry );
}

/**
* Hook to allow delaying calls to {@link LogicalConnectionImplementor#afterStatement()}.
* Mainly used in the case of batching and multi-table mutations
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import org.hibernate.engine.spi.CollectionKey;
import org.hibernate.engine.spi.EntityKey;
import org.hibernate.engine.spi.SubselectFetch;
import org.hibernate.metamodel.mapping.EntityMappingType;
import org.hibernate.query.spi.QueryOptions;
import org.hibernate.query.spi.QueryParameterBindings;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ public InitializersList getInitializersList() {
@Override
public T readRow(RowProcessingState rowProcessingState, JdbcValuesSourceProcessingOptions options) {
LoadingLogger.LOGGER.trace( "StandardRowReader#readRow" );

coordinateInitializers( rowProcessingState );

final Object[] resultRow = new Object[ assemblerCount ];
Expand Down Expand Up @@ -112,6 +111,7 @@ private void coordinateInitializers(RowProcessingState rowProcessingState) {

@Override
public void finishUp(JdbcValuesSourceProcessingState processingState) {
processingState.registerSubselect();
initializers.endLoading( processingState.getExecutionContext() );
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import org.hibernate.event.spi.PostLoadEvent;
import org.hibernate.event.spi.PostLoadEventListener;
import org.hibernate.event.spi.PreLoadEvent;
import org.hibernate.internal.CoreLogging;
import org.hibernate.internal.CoreMessageLogger;
import org.hibernate.query.spi.QueryOptions;
import org.hibernate.sql.exec.spi.Callback;
import org.hibernate.sql.exec.spi.ExecutionContext;
Expand All @@ -36,6 +38,8 @@
*/
public class JdbcValuesSourceProcessingStateStandardImpl implements JdbcValuesSourceProcessingState {

private static final CoreMessageLogger LOG = CoreLogging.messageLogger( JdbcValuesSourceProcessingStateStandardImpl.class );

private final ExecutionContext executionContext;
private final JdbcValuesSourceProcessingOptions processingOptions;

Expand Down Expand Up @@ -97,7 +101,6 @@ public void registerLoadingEntity(
if ( loadingEntityMap == null ) {
loadingEntityMap = new HashMap<>();
}
executionContext.registerLoadingEntityEntry( entityKey, loadingEntry );
loadingEntityMap.put( entityKey, loadingEntry );
}

Expand Down Expand Up @@ -137,6 +140,22 @@ public LoadingCollectionEntry findLoadingCollectionLocally(CollectionKey key) {
return loadingCollectionMap.get( key );
}

@Override
public void registerSubselect() {
if ( loadingEntityMap != null && loadingEntityMap.size() > 1 ) {
loadingEntityMap.forEach(
(entityKey, loadingEntityEntry) ->
executionContext.registerSubselect( entityKey, loadingEntityEntry )
);
}
else {
LOG.tracef(
"Skipping create subselects because there are fewer than 2 results, so query by key is more efficient.",
getClass().getName()
);
}
}

@Override
public void registerLoadingCollection(CollectionKey key, LoadingCollectionEntry loadingCollectionEntry) {
if ( loadingCollectionMap == null ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,8 @@ void registerLoadingCollection(
CollectionKey collectionKey,
LoadingCollectionEntry loadingCollectionEntry);

default void registerSubselect() {
}

void finishUp();
}
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public void tesGetAgency(SessionFactoryScope scope) {
);

assertThat( executedQueries.get( 4 ).toLowerCase() ).isEqualTo(
"select u1_0.group_id,u1_1.user_id,a1_0.agency_id,a1_0.agency_txt,u1_1.user_name from group_user u1_0 join user_table u1_1 on u1_1.user_id=u1_0.user_id left join agency_table a1_0 on a1_0.agency_id=u1_1.agency_id where u1_0.group_id in (select g1_0.group_id from group_table g1_0 where g1_0.agency_id=?)"
"select u1_0.group_id,u1_1.user_id,a1_0.agency_id,a1_0.agency_txt,u1_1.user_name from group_user u1_0 join user_table u1_1 on u1_1.user_id=u1_0.user_id left join agency_table a1_0 on a1_0.agency_id=u1_1.agency_id where u1_0.group_id=?"
);

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public void tesGetAgency(SessionFactoryScope scope) {
);

assertThat( executedQueries.get( 3 ).toLowerCase() ).isEqualTo(
"select u1_0.group_id,u1_1.user_id,a1_0.agency_id,a1_0.agency_txt,u1_1.user_name from group_user u1_0 join user_table u1_1 on u1_1.user_id=u1_0.user_id left join agency_table a1_0 on a1_0.agency_id=u1_1.agency_id where u1_0.group_id in (select g1_0.group_id from group_table g1_0 where g1_0.agency_id=?)"
"select u1_0.group_id,u1_1.user_id,a1_0.agency_id,a1_0.agency_txt,u1_1.user_name from group_user u1_0 join user_table u1_1 on u1_1.user_id=u1_0.user_id left join agency_table a1_0 on a1_0.agency_id=u1_1.agency_id where u1_0.group_id=?"
);

assertThat( executedQueries.get( 4 ).toLowerCase() ).isEqualTo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public void testShouldJoin(SessionFactoryScope scope) {
assertThat( parent.getChildren() ).hasSize( 2 );
statementInspector.assertExecutedCount( 3 ); // 1 query for parent, 1 for grandparent, 1 for children
statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 1 );
statementInspector.assertNumberOfOccurrenceInQuery( 2, "join", 1 );
statementInspector.assertNumberOfOccurrenceInQuery( 2, "join", 0 );
} );
}

Expand Down

0 comments on commit f8275f1

Please sign in to comment.