HHH-17854 Avoid adding plural attribute restrictions multiple times

This commit is contained in:
Christian Beikov 2024-03-15 17:42:33 +01:00
parent 008090b60e
commit bf807f2694
4 changed files with 127 additions and 103 deletions

View File

@ -688,6 +688,8 @@ public class LoaderSelectBuilder {
SqlAstCreationState astCreationState) { SqlAstCreationState astCreationState) {
final NavigablePath parentNavigablePath = tableGroup.getNavigablePath().getParent(); final NavigablePath parentNavigablePath = tableGroup.getNavigablePath().getParent();
if ( parentNavigablePath == null ) { if ( parentNavigablePath == null ) {
// Only apply restrictions for root table groups,
// because for table group joins the restriction is applied via PluralAttributeMappingImpl.createTableGroupJoin
pluralAttributeMapping.applyBaseRestrictions( pluralAttributeMapping.applyBaseRestrictions(
querySpec::applyPredicate, querySpec::applyPredicate,
tableGroup, tableGroup,
@ -705,31 +707,6 @@ public class LoaderSelectBuilder {
astCreationState astCreationState
); );
} }
else {
final TableGroup parentTableGroup = astCreationState.getFromClauseAccess().getTableGroup(
parentNavigablePath );
final TableGroupJoin pluralTableGroupJoin = parentTableGroup.findTableGroupJoin( tableGroup );
assert pluralTableGroupJoin != null;
final TableGroupJoin joinForPredicate = TableGroupJoinHelper.determineJoinForPredicateApply( pluralTableGroupJoin );
pluralAttributeMapping.applyBaseRestrictions(
joinForPredicate::applyPredicate,
tableGroup,
true,
loadQueryInfluencers.getEnabledFilters(),
null,
astCreationState
);
pluralAttributeMapping.applyBaseManyToManyRestrictions(
joinForPredicate::applyPredicate,
tableGroup,
true,
loadQueryInfluencers.getEnabledFilters(),
null,
astCreationState
);
}
} }
private void applyFiltering( private void applyFiltering(

View File

@ -302,7 +302,7 @@ public abstract class AbstractCollectionPersister
if ( StringHelper.isNotEmpty( collectionBootDescriptor.getWhere() ) ) { if ( StringHelper.isNotEmpty( collectionBootDescriptor.getWhere() ) ) {
hasWhere = true; hasWhere = true;
sqlWhereString = "(" + collectionBootDescriptor.getWhere() + ") "; sqlWhereString = "(" + collectionBootDescriptor.getWhere() + ")";
sqlWhereStringTemplate = Template.renderWhereStringTemplate( sqlWhereStringTemplate = Template.renderWhereStringTemplate(
sqlWhereString, sqlWhereString,
dialect, dialect,

View File

@ -485,7 +485,6 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
*/ */
private Map<NavigablePath, Map.Entry<Integer, List<SqlSelection>>> trackedFetchSelectionsForGroup = Collections.emptyMap(); private Map<NavigablePath, Map.Entry<Integer, List<SqlSelection>>> trackedFetchSelectionsForGroup = Collections.emptyMap();
private final Map<NavigablePath, PredicateCollector> collectionFilterPredicates = new IdentityHashMap<>();
private List<Map.Entry<OrderByFragment, TableGroup>> orderByFragments; private List<Map.Entry<OrderByFragment, TableGroup>> orderByFragments;
private final SqlAliasBaseManager sqlAliasBaseManager = new SqlAliasBaseManager(); private final SqlAliasBaseManager sqlAliasBaseManager = new SqlAliasBaseManager();
@ -2088,7 +2087,6 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
); );
orderByFragments = null; orderByFragments = null;
} }
applyCollectionFilterPredicates( sqlQuerySpec );
} }
// Look for treated SqmFrom registrations that have uses of the untreated SqmFrom. // Look for treated SqmFrom registrations that have uses of the untreated SqmFrom.
@ -2181,32 +2179,6 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
return getFromClauseAccess().getTableGroup( navigablePath ); return getFromClauseAccess().getTableGroup( navigablePath );
} }
protected void applyCollectionFilterPredicates(QuerySpec sqlQuerySpec) {
if ( CollectionHelper.isNotEmpty( collectionFilterPredicates ) ) {
final FromClauseAccess fromClauseAccess = getFromClauseAccess();
OUTER:
for ( Map.Entry<NavigablePath, PredicateCollector> entry : collectionFilterPredicates.entrySet() ) {
final TableGroup parentTableGroup = fromClauseAccess.findTableGroup( entry.getKey().getParent() );
if ( parentTableGroup == null ) {
// Since we only keep a single map, this could return null for collections of subqueries
continue;
}
for ( TableGroupJoin tableGroupJoin : parentTableGroup.getTableGroupJoins() ) {
if ( tableGroupJoin.getJoinedGroup().getNavigablePath() == entry.getKey() ) {
tableGroupJoin.applyPredicate( entry.getValue().getPredicate() );
continue OUTER;
}
}
for ( TableGroupJoin tableGroupJoin : parentTableGroup.getNestedTableGroupJoins() ) {
if ( tableGroupJoin.getJoinedGroup().getNavigablePath() == entry.getKey() ) {
tableGroupJoin.applyPredicate( entry.getValue().getPredicate() );
continue OUTER;
}
}
}
}
}
@Override @Override
public SelectClause visitSelectClause(SqmSelectClause selectClause) { public SelectClause visitSelectClause(SqmSelectClause selectClause) {
currentClauseStack.push( Clause.SELECT ); currentClauseStack.push( Clause.SELECT );
@ -8310,49 +8282,13 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
@Override @Override
public ImmutableFetchList visitFetches(FetchParent fetchParent) { public ImmutableFetchList visitFetches(FetchParent fetchParent) {
if ( fetchParent instanceof EagerCollectionFetch ) { if ( fetchParent instanceof EagerCollectionFetch && currentQuerySpec().isRoot() ) {
final EagerCollectionFetch collectionFetch = (EagerCollectionFetch) fetchParent; final EagerCollectionFetch collectionFetch = (EagerCollectionFetch) fetchParent;
final PluralAttributeMapping pluralAttributeMapping = collectionFetch.getFetchedMapping(); final PluralAttributeMapping pluralAttributeMapping = collectionFetch.getFetchedMapping();
final NavigablePath fetchablePath = collectionFetch.getNavigablePath(); final NavigablePath fetchablePath = collectionFetch.getNavigablePath();
final TableGroup tableGroup = getFromClauseIndex().getTableGroup( fetchablePath ); final TableGroup tableGroup = getFromClauseIndex().getTableGroup( fetchablePath );
assert tableGroup.getModelPart() == pluralAttributeMapping;
// Base restrictions have already been applied if this is an explicit fetch applyOrdering( tableGroup, pluralAttributeMapping );
if ( getFromClauseIndex().findFetchedJoinByPath( fetchablePath ) == null ) {
final Restrictable restrictable = pluralAttributeMapping
.getCollectionDescriptor()
.getCollectionType()
.getAssociatedJoinable( getCreationContext().getSessionFactory() );
restrictable.applyBaseRestrictions(
(predicate) -> addCollectionFilterPredicate( tableGroup.getNavigablePath(), predicate ),
tableGroup,
true,
getLoadQueryInfluencers().getEnabledFilters(),
null,
this
);
}
pluralAttributeMapping.applyBaseManyToManyRestrictions(
(predicate) -> {
final TableGroup parentTableGroup = getFromClauseIndex().getTableGroup( collectionFetch.getFetchParent().getNavigablePath() );
final TableGroupJoin pluralTableGroupJoin = parentTableGroup.findTableGroupJoin( tableGroup );
assert pluralTableGroupJoin != null;
final TableGroupJoin joinForPredicate = TableGroupJoinHelper.determineJoinForPredicateApply( pluralTableGroupJoin );
joinForPredicate.applyPredicate( predicate );
},
tableGroup,
true,
getLoadQueryInfluencers().getEnabledFilters(),
null,
this
);
if ( currentQuerySpec().isRoot() ) {
assert tableGroup.getModelPart() == pluralAttributeMapping;
applyOrdering( tableGroup, pluralAttributeMapping );
}
} }
final FetchableContainer referencedMappingContainer = fetchParent.getReferencedMappingContainer(); final FetchableContainer referencedMappingContainer = fetchParent.getReferencedMappingContainer();
final int keySize = referencedMappingContainer.getNumberOfKeyFetchables(); final int keySize = referencedMappingContainer.getNumberOfKeyFetchables();
@ -8430,16 +8366,6 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
} }
} }
private void addCollectionFilterPredicate(NavigablePath navigablePath, Predicate predicate) {
final PredicateCollector existing = collectionFilterPredicates.get( navigablePath );
if ( existing != null ) {
existing.applyPredicate( predicate );
}
else {
collectionFilterPredicates.put( navigablePath, new PredicateCollector( predicate ) );
}
}
private void applyOrdering(TableGroup tableGroup, PluralAttributeMapping pluralAttributeMapping) { private void applyOrdering(TableGroup tableGroup, PluralAttributeMapping pluralAttributeMapping) {
if ( pluralAttributeMapping.getOrderByFragment() != null ) { if ( pluralAttributeMapping.getOrderByFragment() != null ) {
applyOrdering( tableGroup, pluralAttributeMapping.getOrderByFragment() ); applyOrdering( tableGroup, pluralAttributeMapping.getOrderByFragment() );

View File

@ -0,0 +1,121 @@
/*
* Hibernate, Relational Persistence for Idiomatic Java
*
* License: GNU Lesser General Public License (LGPL), version 2.1 or later.
* See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html.
*/
package org.hibernate.orm.test.mapping.where;
import java.time.LocalDateTime;
import java.util.HashSet;
import java.util.Set;
import org.hibernate.annotations.SQLRestriction;
import org.hibernate.graph.spi.RootGraphImplementor;
import org.hibernate.jpa.AvailableHints;
import org.hibernate.testing.jdbc.SQLStatementInspector;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.Jira;
import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.junit.jupiter.api.Test;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.Id;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.OneToMany;
import static org.assertj.core.api.Assertions.assertThat;
@DomainModel(annotatedClasses = { OneToManySQLRestrictionTests.Parent.class, OneToManySQLRestrictionTests.Child.class })
@SessionFactory(useCollectingStatementInspector = true)
@Jira("https://hibernate.atlassian.net/browse/HHH-17854")
public class OneToManySQLRestrictionTests {
@Test
public void testLoad(SessionFactoryScope scope) {
final SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();
statementInspector.clear();
scope.inTransaction( (session) -> {
session.find( Parent.class, 1 );
assertThat( statementInspector.getSqlQueries() ).hasSize( 1 );
assertThat( statementInspector.getSqlQueries().get( 0 ) )
.isEqualTo(
"select p1_0.id,cs1_0.parent_id,cs1_0.id,cs1_0.deleted_at " +
"from Parent p1_0 " +
"left join Child cs1_0 " +
"on p1_0.id=cs1_0.parent_id " +
"and (cs1_0.deleted_at IS NULL) " +
"where p1_0.id=?"
);
} );
}
@Test
public void testLoad2(SessionFactoryScope scope) {
final SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();
statementInspector.clear();
scope.inTransaction( (session) -> {
session.createQuery( "from Parent p join fetch p.childSet" ).getResultList();
assertThat( statementInspector.getSqlQueries() ).hasSize( 1 );
assertThat( statementInspector.getSqlQueries().get( 0 ) )
.isEqualTo(
"select p1_0.id,cs1_0.parent_id,cs1_0.id,cs1_0.deleted_at " +
"from Parent p1_0 " +
"join Child cs1_0 " +
"on p1_0.id=cs1_0.parent_id " +
"and (cs1_0.deleted_at IS NULL)"
);
} );
}
@Test
public void testLoad3(SessionFactoryScope scope) {
final SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();
statementInspector.clear();
scope.inTransaction( (session) -> {
RootGraphImplementor<Parent> entityGraph = session.createEntityGraph( Parent.class );
entityGraph.addAttributeNode( "childSet" );
session.createQuery( "from Parent p" )
.setHint( AvailableHints.HINT_SPEC_LOAD_GRAPH, entityGraph )
.getResultList();
assertThat( statementInspector.getSqlQueries() ).hasSize( 1 );
assertThat( statementInspector.getSqlQueries().get( 0 ) )
.isEqualTo(
"select p1_0.id,cs1_0.parent_id,cs1_0.id,cs1_0.deleted_at " +
"from Parent p1_0 " +
"left join Child cs1_0 " +
"on p1_0.id=cs1_0.parent_id " +
"and (cs1_0.deleted_at IS NULL)"
);
} );
}
@Entity(name = "Parent")
public static class Parent {
@Id
@GeneratedValue
private Long id;
@SQLRestriction("deleted_at IS NULL")
@OneToMany(mappedBy = "parent", fetch = FetchType.EAGER)
private Set<Child> childSet = new HashSet<>();
}
@Entity(name = "Child")
public static class Child {
@Id
@GeneratedValue
private Long id;
@ManyToOne
private Parent parent;
@Column(name = "deleted_at")
private LocalDateTime deletedAt;
}
}