diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java index 462d52d890..8e46b9c1b8 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java @@ -708,18 +708,19 @@ public class LoaderSelectBuilder { else { final TableGroup parentTableGroup = astCreationState.getFromClauseAccess().getTableGroup( parentNavigablePath ); - TableGroupJoin pluralTableGroupJoin = null; - for ( TableGroupJoin nestedTableGroupJoin : parentTableGroup.getTableGroupJoins() ) { - if ( nestedTableGroupJoin.getNavigablePath() == tableGroup.getNavigablePath() ) { - pluralTableGroupJoin = nestedTableGroupJoin; - break; - } - } - + final TableGroupJoin pluralTableGroupJoin = parentTableGroup.findTableGroupJoin( tableGroup ); assert pluralTableGroupJoin != null; + final TableGroupJoin joinForPredicate; + if ( !tableGroup.getNestedTableGroupJoins().isEmpty() || tableGroup.getTableGroupJoins().isEmpty() ) { + joinForPredicate = pluralTableGroupJoin; + } + else { + joinForPredicate = tableGroup.getTableGroupJoins().get( tableGroup.getTableGroupJoins().size() - 1 ); + } + pluralAttributeMapping.applyBaseRestrictions( - pluralTableGroupJoin::applyPredicate, + joinForPredicate::applyPredicate, tableGroup, true, loadQueryInfluencers.getEnabledFilters(), @@ -727,7 +728,7 @@ public class LoaderSelectBuilder { astCreationState ); pluralAttributeMapping.applyBaseManyToManyRestrictions( - pluralTableGroupJoin::applyPredicate, + joinForPredicate::applyPredicate, tableGroup, true, loadQueryInfluencers.getEnabledFilters(), diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/EntityMappingType.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/EntityMappingType.java index a2395e5559..1fd559d944 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/EntityMappingType.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/EntityMappingType.java @@ -637,6 +637,11 @@ public interface EntityMappingType getEntityPersister().applyBaseRestrictions( predicateConsumer, tableGroup, useQualifier, enabledFilters, treatAsDeclarations, creationState ); } + @Override + default boolean hasWhereRestrictions() { + return getEntityPersister().hasWhereRestrictions(); + } + @Override default void applyWhereRestrictions( Consumer predicateConsumer, diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/PluralAttributeMapping.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/PluralAttributeMapping.java index 11f39d6cb9..78f6e6d1a2 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/PluralAttributeMapping.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/PluralAttributeMapping.java @@ -181,6 +181,11 @@ public interface PluralAttributeMapping getCollectionDescriptor().applyBaseManyToManyRestrictions( predicateConsumer, tableGroup, useQualifier, enabledFilters, treatAsDeclarations, creationState ); } + @Override + default boolean hasWhereRestrictions() { + return getCollectionDescriptor().hasWhereRestrictions(); + } + @Override default void applyWhereRestrictions( Consumer predicateConsumer, diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/WhereRestrictable.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/WhereRestrictable.java index a29ef38209..13e663c7b2 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/WhereRestrictable.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/WhereRestrictable.java @@ -19,6 +19,12 @@ import org.hibernate.sql.ast.tree.predicate.Predicate; * @see FilterRestrictable */ public interface WhereRestrictable { + + /** + * Does this restrictable have a where restriction? + */ + boolean hasWhereRestrictions(); + /** * Apply the {@link org.hibernate.annotations.Where} restrictions */ diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ManyToManyCollectionPart.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ManyToManyCollectionPart.java index 7374c743f1..d197318c02 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ManyToManyCollectionPart.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ManyToManyCollectionPart.java @@ -82,8 +82,6 @@ public class ManyToManyCollectionPart extends AbstractEntityCollectionPart imple LazyTableGroup.ParentTableGroupUseChecker { private ForeignKeyDescriptor foreignKey; private ValuedModelPart fkTargetModelPart; - private boolean[] isInsertable; - private boolean[] isUpdatable; public ManyToManyCollectionPart( Nature nature, diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java index 406a1d807e..456ac39e09 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java @@ -700,8 +700,7 @@ public class PluralAttributeMappingImpl boolean fetched, boolean addsPredicate, SqlAstCreationState creationState) { - final PredicateCollector predicateCollector = new PredicateCollector(); - + final PredicateCollector collectionPredicateCollector = new PredicateCollector(); final TableGroup tableGroup = createRootTableGroupJoin( navigablePath, lhs, @@ -709,9 +708,18 @@ public class PluralAttributeMappingImpl explicitSqlAliasBase, requestedJoinType, fetched, - predicateCollector::applyPredicate, + addsPredicate, + collectionPredicateCollector::applyPredicate, creationState ); + final PredicateCollector predicateCollector; + if ( tableGroup.getNestedTableGroupJoins().isEmpty() ) { + // No nested table group joins means that the predicate has to be pushed to the last join + predicateCollector = new PredicateCollector(); + } + else { + predicateCollector = collectionPredicateCollector; + } getCollectionDescriptor().applyBaseRestrictions( predicateCollector::applyPredicate, @@ -737,12 +745,23 @@ public class PluralAttributeMappingImpl creationState ); - return new TableGroupJoin( + final TableGroupJoin tableGroupJoin = new TableGroupJoin( navigablePath, determineSqlJoinType( lhs, requestedJoinType, fetched ), tableGroup, - predicateCollector.getPredicate() + collectionPredicateCollector.getPredicate() ); + if ( predicateCollector != collectionPredicateCollector ) { + final TableGroupJoin joinForPredicate; + if ( !tableGroup.getNestedTableGroupJoins().isEmpty() || tableGroup.getTableGroupJoins().isEmpty() ) { + joinForPredicate = tableGroupJoin; + } + else { + joinForPredicate = tableGroup.getTableGroupJoins().get( tableGroup.getTableGroupJoins().size() - 1 ); + } + joinForPredicate.applyPredicate( predicateCollector.getPredicate() ); + } + return tableGroupJoin; } private boolean hasSoftDelete() { @@ -827,6 +846,29 @@ public class PluralAttributeMappingImpl boolean fetched, Consumer predicateConsumer, SqlAstCreationState creationState) { + return createRootTableGroupJoin( + navigablePath, + lhs, + explicitSourceAlias, + explicitSqlAliasBase, + requestedJoinType, + fetched, + false, + predicateConsumer, + creationState + ); + } + + private TableGroup createRootTableGroupJoin( + NavigablePath navigablePath, + TableGroup lhs, + String explicitSourceAlias, + SqlAliasBase explicitSqlAliasBase, + SqlAstJoinType requestedJoinType, + boolean fetched, + boolean addsPredicate, + Consumer predicateConsumer, + SqlAstCreationState creationState) { final CollectionPersister collectionDescriptor = getCollectionDescriptor(); final SqlAstJoinType joinType = determineSqlJoinType( lhs, requestedJoinType, fetched ); final SqlAliasBase sqlAliasBase = creationState.getSqlAliasBaseGenerator().createSqlAliasBase( getSqlAliasStem() ); @@ -845,8 +887,10 @@ public class PluralAttributeMappingImpl else { tableGroup = createCollectionTableGroup( lhs.canUseInnerJoins() && joinType == SqlAstJoinType.INNER, + joinType, navigablePath, fetched, + addsPredicate, explicitSourceAlias, sqlAliasBase, creationState @@ -912,8 +956,10 @@ public class PluralAttributeMappingImpl private TableGroup createCollectionTableGroup( boolean canUseInnerJoins, + SqlAstJoinType joinType, NavigablePath navigablePath, boolean fetched, + boolean addsPredicate, String sourceAlias, SqlAliasBase explicitSqlAliasBase, SqlAstCreationState creationState) { @@ -944,6 +990,12 @@ public class PluralAttributeMappingImpl null, creationState.getCreationContext().getSessionFactory() ); + // For inner joins we never need join nesting + final boolean nestedJoin = joinType != SqlAstJoinType.INNER + // For outer joins we need nesting if there might be an on-condition that refers to the element table + && ( addsPredicate + || isAffectedByEnabledFilters( creationState.getLoadQueryInfluencers() ) + || collectionDescriptor.hasWhereRestrictions() ); if ( elementDescriptor instanceof TableGroupJoinProducer ) { final TableGroupJoin tableGroupJoin = ( (TableGroupJoinProducer) elementDescriptor ).createTableGroupJoin( @@ -951,12 +1003,12 @@ public class PluralAttributeMappingImpl tableGroup, null, sqlAliasBase, - SqlAstJoinType.INNER, + nestedJoin ? SqlAstJoinType.INNER : joinType, fetched, false, creationState ); - tableGroup.registerElementTableGroup( tableGroupJoin ); + tableGroup.registerElementTableGroup( tableGroupJoin, nestedJoin ); } if ( indexDescriptor instanceof TableGroupJoinProducer ) { @@ -965,12 +1017,12 @@ public class PluralAttributeMappingImpl tableGroup, null, sqlAliasBase, - SqlAstJoinType.INNER, + nestedJoin ? SqlAstJoinType.INNER : joinType, fetched, false, creationState ); - tableGroup.registerIndexTableGroup( tableGroupJoin ); + tableGroup.registerIndexTableGroup( tableGroupJoin, nestedJoin ); } return tableGroup; @@ -996,8 +1048,10 @@ public class PluralAttributeMappingImpl else { return createCollectionTableGroup( canUseInnerJoins, + SqlAstJoinType.INNER, navigablePath, false, + false, explicitSourceAlias, explicitSqlAliasBase, creationState diff --git a/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java index d675135993..f5b3c321e9 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java @@ -1149,6 +1149,11 @@ public abstract class AbstractCollectionPersister applyWhereRestrictions( predicateConsumer, tableGroup, useQualifier, creationState ); } + @Override + public boolean hasWhereRestrictions() { + return hasWhere() || manyToManyWhereTemplate != null; + } + @Override public void applyWhereRestrictions( Consumer predicateConsumer, diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java index 4b34215eb4..b98a99b209 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java @@ -3315,6 +3315,11 @@ public abstract class AbstractEntityPersister applyWhereRestrictions( predicateConsumer, tableGroup, useQualifier, creationState ); } + @Override + public boolean hasWhereRestrictions() { + return sqlWhereStringTemplate != null; + } + @Override public void applyWhereRestrictions( Consumer predicateConsumer, diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java index 3d73d3e62f..30914c8ebf 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java @@ -3350,6 +3350,14 @@ public abstract class BaseSqmToSqlAstConverter extends Base } } + final TableGroupJoin joinForPredicate; + if ( !joinedTableGroup.getNestedTableGroupJoins().isEmpty() || joinedTableGroup.getTableGroupJoins().isEmpty() ) { + joinForPredicate = joinedTableGroupJoin; + } + else { + joinForPredicate = joinedTableGroup.getTableGroupJoins().get( joinedTableGroup.getTableGroupJoins().size() - 1 ); + } + // add any additional join restrictions if ( sqmJoin.getJoinPredicate() != null ) { if ( sqmJoin.isFetched() ) { @@ -3358,13 +3366,21 @@ public abstract class BaseSqmToSqlAstConverter extends Base final SqmJoin oldJoin = currentlyProcessingJoin; currentlyProcessingJoin = sqmJoin; - joinedTableGroupJoin.applyPredicate( visitNestedTopLevelPredicate( sqmJoin.getJoinPredicate() ) ); + final Predicate predicate = visitNestedTopLevelPredicate( sqmJoin.getJoinPredicate() ); + // If translating the join predicate didn't initialize the table group, + // we can safely apply it on the collection table group instead + if ( joinForPredicate.getJoinedGroup().isInitialized() ) { + joinForPredicate.applyPredicate( predicate ); + } + else { + joinedTableGroupJoin.applyPredicate( predicate ); + } currentlyProcessingJoin = oldJoin; } // Since joins on treated paths will never cause table pruning, we need to add a join condition for the treat if ( sqmJoin.getLhs() instanceof SqmTreatedPath ) { final SqmTreatedPath treatedPath = (SqmTreatedPath) sqmJoin.getLhs(); - joinedTableGroupJoin.applyPredicate( + joinForPredicate.applyPredicate( createTreatTypeRestriction( treatedPath.getWrappedPath(), treatedPath.getTreatTarget() @@ -8171,16 +8187,17 @@ public abstract class BaseSqmToSqlAstConverter extends Base pluralAttributeMapping.applyBaseManyToManyRestrictions( (predicate) -> { final TableGroup parentTableGroup = getFromClauseIndex().getTableGroup( collectionFetch.getFetchParent().getNavigablePath() ); - TableGroupJoin pluralTableGroupJoin = null; - for ( TableGroupJoin nestedTableGroupJoin : parentTableGroup.getTableGroupJoins() ) { - if ( nestedTableGroupJoin.getNavigablePath() == fetchablePath ) { - pluralTableGroupJoin = nestedTableGroupJoin; - break; - } - } - + final TableGroupJoin pluralTableGroupJoin = parentTableGroup.findTableGroupJoin( tableGroup ); assert pluralTableGroupJoin != null; - pluralTableGroupJoin.applyPredicate( predicate ); + + final TableGroupJoin joinForPredicate; + if ( !tableGroup.getNestedTableGroupJoins().isEmpty() || tableGroup.getTableGroupJoins().isEmpty() ) { + joinForPredicate = pluralTableGroupJoin; + } + else { + joinForPredicate = tableGroup.getTableGroupJoins().get( tableGroup.getTableGroupJoins().size() - 1 ); + } + joinForPredicate.applyPredicate( predicate ); }, tableGroup, true, diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/CollectionTableGroup.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/CollectionTableGroup.java index 5283375878..acd3bbb876 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/CollectionTableGroup.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/CollectionTableGroup.java @@ -67,15 +67,33 @@ public class CollectionTableGroup extends StandardTableGroup implements PluralTa } public void registerIndexTableGroup(TableGroupJoin indexTableGroupJoin) { + registerIndexTableGroup( indexTableGroupJoin, true ); + } + + public void registerIndexTableGroup(TableGroupJoin indexTableGroupJoin, boolean nested) { assert this.indexTableGroup == null; this.indexTableGroup = indexTableGroupJoin.getJoinedGroup(); - addNestedTableGroupJoin( indexTableGroupJoin ); + if ( nested ) { + addNestedTableGroupJoin( indexTableGroupJoin ); + } + else { + addTableGroupJoin( indexTableGroupJoin ); + } } public void registerElementTableGroup(TableGroupJoin elementTableGroupJoin) { + registerElementTableGroup( elementTableGroupJoin, true ); + } + + public void registerElementTableGroup(TableGroupJoin elementTableGroupJoin, boolean nested) { assert this.elementTableGroup == null; this.elementTableGroup = elementTableGroupJoin.getJoinedGroup(); - addNestedTableGroupJoin( elementTableGroupJoin ); + if ( nested ) { + addNestedTableGroupJoin( elementTableGroupJoin ); + } + else { + addTableGroupJoin( elementTableGroupJoin ); + } } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ArrayInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ArrayInitializer.java index 207466ab4c..94a9a49072 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ArrayInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ArrayInitializer.java @@ -72,6 +72,11 @@ public class ArrayInitializer extends AbstractImmediateCollectionInitializer { throw new HibernateException( "Illegal null value for array index encountered while reading: " + getCollectionAttributeMapping().getNavigableRole() ); } + final Object element = elementAssembler.assemble( rowProcessingState ); + if ( element == null ) { + // If element is null, then NotFoundAction must be IGNORE + return; + } int index = indexValue; if ( indexBase != 0 ) { @@ -82,7 +87,7 @@ public class ArrayInitializer extends AbstractImmediateCollectionInitializer { loadingState.add( i, null ); } - loadingState.set( index, elementAssembler.assemble( rowProcessingState ) ); + loadingState.set( index, element ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/BagInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/BagInitializer.java index 73f564abd0..2d3ccd570a 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/BagInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/BagInitializer.java @@ -63,14 +63,24 @@ public class BagInitializer extends AbstractImmediateCollectionInitializer { List loadingState, RowProcessingState rowProcessingState) { if ( collectionIdAssembler != null ) { - final Object[] row = new Object[2]; - row[0] = collectionIdAssembler.assemble( rowProcessingState ); - row[1] = elementAssembler.assemble( rowProcessingState ); + final Object collectionId = collectionIdAssembler.assemble( rowProcessingState ); + if ( collectionId == null ) { + return; + } + final Object element = elementAssembler.assemble( rowProcessingState ); + if ( element == null ) { + // If element is null, then NotFoundAction must be IGNORE + return; + } - loadingState.add( row ); + loadingState.add( new Object[]{ collectionId, element } ); } else { - loadingState.add( elementAssembler.assemble( rowProcessingState ) ); + final Object element = elementAssembler.assemble( rowProcessingState ); + if ( element != null ) { + // If element is null, then NotFoundAction must be IGNORE + loadingState.add( element ); + } } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ListInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ListInitializer.java index 9ad6771ff2..2596663fe0 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ListInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ListInitializer.java @@ -74,6 +74,11 @@ public class ListInitializer extends AbstractImmediateCollectionInitializer { throw new HibernateException( "Illegal null value for list index encountered while reading: " + getCollectionAttributeMapping().getNavigableRole() ); } + final Object element = elementAssembler.assemble( rowProcessingState ); + if ( element == null ) { + // If element is null, then NotFoundAction must be IGNORE + return; + } int index = indexValue; if ( listIndexBase != 0 ) { @@ -84,7 +89,7 @@ public class ListInitializer extends AbstractImmediateCollectionInitializer { loadingState.add( i, null ); } - loadingState.set( index, elementAssembler.assemble( rowProcessingState ) ); + loadingState.set( index, element ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/MapInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/MapInitializer.java index 1047c1e582..294501896e 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/MapInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/MapInitializer.java @@ -9,10 +9,12 @@ package org.hibernate.sql.results.graph.collection.internal; import java.util.List; import org.hibernate.LockMode; +import org.hibernate.annotations.NotFoundAction; import org.hibernate.collection.spi.PersistentMap; import org.hibernate.engine.spi.CollectionKey; import org.hibernate.internal.log.LoggingHelper; import org.hibernate.metamodel.mapping.PluralAttributeMapping; +import org.hibernate.metamodel.mapping.internal.EntityCollectionPart; import org.hibernate.spi.NavigablePath; import org.hibernate.sql.results.graph.DomainResultAssembler; import org.hibernate.sql.results.graph.FetchParentAccess; @@ -61,12 +63,17 @@ public class MapInitializer extends AbstractImmediateCollectionInitializer { CollectionKey collectionKey, List loadingState, RowProcessingState rowProcessingState) { - loadingState.add( - new Object[] { - mapKeyAssembler.assemble( rowProcessingState ), - mapValueAssembler.assemble( rowProcessingState ) - } - ); + final Object key = mapKeyAssembler.assemble( rowProcessingState ); + if ( key == null ) { + // If element is null, then NotFoundAction must be IGNORE + return; + } + final Object value = mapValueAssembler.assemble( rowProcessingState ); + if ( value == null ) { + // If element is null, then NotFoundAction must be IGNORE + return; + } + loadingState.add( new Object[] { key, value } ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/SetInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/SetInitializer.java index 5f6c8067b7..72c88eafa8 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/SetInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/SetInitializer.java @@ -53,7 +53,12 @@ public class SetInitializer extends AbstractImmediateCollectionInitializer { CollectionKey collectionKey, List loadingState, RowProcessingState rowProcessingState) { - loadingState.add( elementAssembler.assemble( rowProcessingState ) ); + final Object element = elementAssembler.assemble( rowProcessingState ); + if ( element == null ) { + // If element is null, then NotFoundAction must be IGNORE + return; + } + loadingState.add( element ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchJoinedImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchJoinedImpl.java index 34e9272e47..1d9abfc444 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchJoinedImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchJoinedImpl.java @@ -65,7 +65,7 @@ public class EntityFetchJoinedImpl extends AbstractNonLazyEntityFetch { NavigablePath navigablePath, DomainResultCreationState creationState) { super( fetchParent, collectionPart, navigablePath ); - this.notFoundAction = null; + this.notFoundAction = collectionPart.getNotFoundAction(); this.keyResult = null; this.sourceAlias = tableGroup.getSourceAlias(); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/cfg/persister/GoofyPersisterClassProvider.java b/hibernate-core/src/test/java/org/hibernate/orm/test/cfg/persister/GoofyPersisterClassProvider.java index f8bc9f5326..94d76b7557 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/cfg/persister/GoofyPersisterClassProvider.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/cfg/persister/GoofyPersisterClassProvider.java @@ -1023,6 +1023,11 @@ public class GoofyPersisterClassProvider implements PersisterClassResolver { } + @Override + public boolean hasWhereRestrictions() { + return false; + } + @Override public void applyWhereRestrictions(Consumer predicateConsumer, TableGroup tableGroup, boolean useQualifier, SqlAstCreationState creationState) { diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/CriteriaEntityGraphTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/CriteriaEntityGraphTest.java index 42b8d1f3ed..7ac5b08dea 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/CriteriaEntityGraphTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/CriteriaEntityGraphTest.java @@ -280,10 +280,10 @@ public class CriteriaEntityGraphTest implements SessionFactoryScopeAware { // Check the from-clause assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses", tableGroup -> { if ( graphSemantic == GraphSemantic.LOAD ) { - assertThat( tableGroup.getTableGroupJoins(), isEmpty() ); - assertThat( tableGroup.getNestedTableGroupJoins(), hasSize( 1 ) ); + assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) ); + assertThat( tableGroup.getNestedTableGroupJoins(), isEmpty() ); - final TableGroup compositeTableGroup = tableGroup.getNestedTableGroupJoins() + final TableGroup compositeTableGroup = tableGroup.getTableGroupJoins() .iterator() .next() .getJoinedGroup(); @@ -295,10 +295,10 @@ public class CriteriaEntityGraphTest implements SessionFactoryScopeAware { assertThat( joinedGroup.isInitialized(), is( false ) ); } else { - assertThat( tableGroup.getTableGroupJoins(), isEmpty() ); - assertThat( tableGroup.getNestedTableGroupJoins(), hasSize( 1 ) ); + assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) ); + assertThat( tableGroup.getNestedTableGroupJoins(), isEmpty() ); - final TableGroup compositeTableGroup = CollectionUtils.getOnlyElement( tableGroup.getNestedTableGroupJoins() ).getJoinedGroup(); + final TableGroup compositeTableGroup = CollectionUtils.getOnlyElement( tableGroup.getTableGroupJoins() ).getJoinedGroup(); assertThat( compositeTableGroup, instanceOf( StandardVirtualTableGroup.class ) ); assertThat( compositeTableGroup.getNestedTableGroupJoins(), isEmpty() ); assertThat( compositeTableGroup.getTableGroupJoins(), hasSize( 1 ) ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/EntityGraphLoadPlanBuilderTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/EntityGraphLoadPlanBuilderTest.java index 275a1f554e..d1f91fa5ea 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/EntityGraphLoadPlanBuilderTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/EntityGraphLoadPlanBuilderTest.java @@ -248,10 +248,10 @@ public class EntityGraphLoadPlanBuilderTest implements SessionFactoryScopeAware // Check the from-clause assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses", tableGroup -> { if ( graphSemantic == GraphSemantic.LOAD ) { - assertThat( tableGroup.getTableGroupJoins(), isEmpty() ); - assertThat( tableGroup.getNestedTableGroupJoins(), hasSize( 1 ) ); + assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) ); + assertThat( tableGroup.getNestedTableGroupJoins(), isEmpty() ); - final TableGroup compositeTableGroup = CollectionUtils.getOnlyElement( tableGroup.getNestedTableGroupJoins() ) + final TableGroup compositeTableGroup = CollectionUtils.getOnlyElement( tableGroup.getTableGroupJoins() ) .getJoinedGroup(); assertThat( compositeTableGroup, instanceOf( StandardVirtualTableGroup.class ) ); assertThat( compositeTableGroup.getTableGroupJoins(), hasSize( 1 ) ); @@ -265,10 +265,10 @@ public class EntityGraphLoadPlanBuilderTest implements SessionFactoryScopeAware assertThat( countryTableGroup.getNestedTableGroupJoins(), isEmpty() ); } else { - assertThat( tableGroup.getTableGroupJoins(), isEmpty() ); - assertThat( tableGroup.getNestedTableGroupJoins(), hasSize( 1 ) ); + assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) ); + assertThat( tableGroup.getNestedTableGroupJoins(), isEmpty() ); - final TableGroup compositeTableGroup = CollectionUtils.getOnlyElement( tableGroup.getNestedTableGroupJoins() ).getJoinedGroup(); + final TableGroup compositeTableGroup = CollectionUtils.getOnlyElement( tableGroup.getTableGroupJoins() ).getJoinedGroup(); assertThat( compositeTableGroup, instanceOf( StandardVirtualTableGroup.class ) ); assertThat( compositeTableGroup.getNestedTableGroupJoins(), isEmpty() ); assertThat( compositeTableGroup.getTableGroupJoins(), hasSize( 1 ) ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/HqlEntityGraphTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/HqlEntityGraphTest.java index 4382e2abbc..3650402880 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/HqlEntityGraphTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/HqlEntityGraphTest.java @@ -278,10 +278,10 @@ public class HqlEntityGraphTest implements SessionFactoryScopeAware { // Check the from-clause assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses", tableGroup -> { if ( graphSemantic == GraphSemantic.LOAD ) { - assertThat( tableGroup.getTableGroupJoins(), isEmpty() ); - assertThat( tableGroup.getNestedTableGroupJoins(), hasSize( 1 ) ); + assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) ); + assertThat( tableGroup.getNestedTableGroupJoins(), isEmpty() ); - final TableGroup compositeTableGroup = tableGroup.getNestedTableGroupJoins() + final TableGroup compositeTableGroup = tableGroup.getTableGroupJoins() .iterator() .next() .getJoinedGroup(); @@ -293,10 +293,10 @@ public class HqlEntityGraphTest implements SessionFactoryScopeAware { assertThat( joinedGroup.isInitialized(), is( false ) ); } else { - assertThat( tableGroup.getTableGroupJoins(), isEmpty() ); - assertThat( tableGroup.getNestedTableGroupJoins(), hasSize( 1 ) ); + assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) ); + assertThat( tableGroup.getNestedTableGroupJoins(), isEmpty() ); - final TableGroup compositeTableGroup = CollectionUtils.getOnlyElement( tableGroup.getNestedTableGroupJoins() ).getJoinedGroup(); + final TableGroup compositeTableGroup = CollectionUtils.getOnlyElement( tableGroup.getTableGroupJoins() ).getJoinedGroup(); assertThat( compositeTableGroup, instanceOf( StandardVirtualTableGroup.class ) ); assertThat( compositeTableGroup.getNestedTableGroupJoins(), isEmpty() ); assertThat( compositeTableGroup.getTableGroupJoins(), hasSize( 1 ) ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/JoinTableOptimizationTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/JoinTableOptimizationTest.java index 766f9ca506..4fd3a75b75 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/JoinTableOptimizationTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/JoinTableOptimizationTest.java @@ -13,6 +13,7 @@ import org.hibernate.testing.jdbc.SQLStatementInspector; import org.hibernate.testing.orm.junit.DomainModel; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import jakarta.persistence.Entity; @@ -42,6 +43,86 @@ public class JoinTableOptimizationTest { ); } + @Test + public void testInnerJoin(SessionFactoryScope scope) { + SQLStatementInspector statementInspector = scope.getCollectingStatementInspector(); + statementInspector.clear(); + scope.inTransaction( + s -> { + s.createQuery( "select p.name from Document d join d.people p" ).list(); + statementInspector.assertExecutedCount( 1 ); + Assertions.assertEquals( + "select p1_1.name " + + "from Document d1_0 " + + "join people p1_0 on d1_0.id=p1_0.Document_id " + + "join Person p1_1 on p1_1.id=p1_0.people_id", + statementInspector.getSqlQueries().get( 0 ), + "Nested join was not optimized away" + ); + } + ); + } + + @Test + public void testLeftJoin(SessionFactoryScope scope) { + SQLStatementInspector statementInspector = scope.getCollectingStatementInspector(); + statementInspector.clear(); + scope.inTransaction( + s -> { + s.createQuery( "select p.name from Document d left join d.people p" ).list(); + statementInspector.assertExecutedCount( 1 ); + Assertions.assertEquals( + "select p1_1.name " + + "from Document d1_0 " + + "left join people p1_0 on d1_0.id=p1_0.Document_id " + + "left join Person p1_1 on p1_1.id=p1_0.people_id", + statementInspector.getSqlQueries().get( 0 ), + "Nested join was not optimized away" + ); + } + ); + } + + @Test + public void testInnerJoinCustomOnClause(SessionFactoryScope scope) { + SQLStatementInspector statementInspector = scope.getCollectingStatementInspector(); + statementInspector.clear(); + scope.inTransaction( + s -> { + s.createQuery( "select p.name from Document d join d.people p on p.id > 1" ).list(); + statementInspector.assertExecutedCount( 1 ); + Assertions.assertEquals( + "select p1_1.name " + + "from Document d1_0 " + + "join people p1_0 on d1_0.id=p1_0.Document_id and p1_0.people_id>1 " + + "join Person p1_1 on p1_1.id=p1_0.people_id", + statementInspector.getSqlQueries().get( 0 ), + "Nested join was not optimized away" + ); + } + ); + } + + @Test + public void testLeftJoinCustomOnClause(SessionFactoryScope scope) { + SQLStatementInspector statementInspector = scope.getCollectingStatementInspector(); + statementInspector.clear(); + scope.inTransaction( + s -> { + s.createQuery( "select p.name from Document d left join d.people p on p.id > 1" ).list(); + statementInspector.assertExecutedCount( 1 ); + Assertions.assertEquals( + "select p1_1.name " + + "from Document d1_0 " + + "left join (people p1_0 " + + "join Person p1_1 on p1_1.id=p1_0.people_id) on d1_0.id=p1_0.Document_id and p1_0.people_id>1", + statementInspector.getSqlQueries().get( 0 ), + "Nested join was wrongly optimized away" + ); + } + ); + } + @Entity(name = "Document") public static class Document { @Id diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/MapIssueTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/MapIssueTest.java index d81363a69c..c735fa79af 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/MapIssueTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/MapIssueTest.java @@ -57,7 +57,7 @@ public class MapIssueTest { statementInspector.clear(); scope.inTransaction( s -> { - s.createQuery( "select c from MapOwner as o left join o.contents c join c.relationship r where r.id is not null" ).list(); + s.createQuery( "select c from MapOwner as o join o.contents c join c.relationship r where r.id is not null" ).list(); statementInspector.assertExecutedCount( 1 ); // Assert 2 joins, collection table and collection element. No need to join the relationship because it is not nullable statementInspector.assertNumberOfJoins( 0, 2 ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/CompareEntityValuedPathsTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/CompareEntityValuedPathsTest.java index 5f44b8df6f..93789c0cc7 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/query/CompareEntityValuedPathsTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/CompareEntityValuedPathsTest.java @@ -249,7 +249,7 @@ public class CompareEntityValuedPathsTest { "select " + "1 " + "from PERSON_TABLE p1_0 " + - "join (children_uks cu1_0 join PERSON_TABLE cu1_1 on cu1_1.uk=cu1_0.child_uk) on p1_0.uk=cu1_0.owner_uk " + + "join children_uks cu1_0 on p1_0.uk=cu1_0.owner_uk join PERSON_TABLE cu1_1 on cu1_1.uk=cu1_0.child_uk " + "join PERSON_TABLE_PERSON_TABLE c1_0 on p1_0.id=c1_0.Person_id " + "where cu1_1.id=c1_0.children_id", statementInspector.getSqlQueries().get( 0 ) @@ -272,7 +272,7 @@ public class CompareEntityValuedPathsTest { "1 " + "from PERSON_TABLE p1_0 " + "join PERSON_TABLE_PERSON_TABLE c1_0 on p1_0.id=c1_0.Person_id " + - "join (children_uks cu1_0 join PERSON_TABLE cu1_1 on cu1_1.uk=cu1_0.child_uk) on p1_0.uk=cu1_0.owner_uk " + + "join children_uks cu1_0 on p1_0.uk=cu1_0.owner_uk join PERSON_TABLE cu1_1 on cu1_1.uk=cu1_0.child_uk " + "where c1_0.children_id=cu1_1.id", statementInspector.getSqlQueries().get( 0 ) ); @@ -293,7 +293,7 @@ public class CompareEntityValuedPathsTest { "select " + "1 " + "from PERSON_TABLE p1_0 " + - "join (children_uks cu1_0 join PERSON_TABLE cu1_1 on cu1_1.uk=cu1_0.child_uk) on p1_0.uk=cu1_0.owner_uk " + + "join children_uks cu1_0 on p1_0.uk=cu1_0.owner_uk join PERSON_TABLE cu1_1 on cu1_1.uk=cu1_0.child_uk " + "where cu1_1.id in (select c1_0.children_id from PERSON_TABLE_PERSON_TABLE c1_0 where p1_0.id=c1_0.Person_id)", statementInspector.getSqlQueries().get( 0 ) );