HHH-15342 Inappropriate variation of HQL left join to SQL inner join

This commit is contained in:
Christian Beikov 2022-06-27 16:41:36 +02:00 committed by Andrea Boriero
parent 985467bcba
commit 1da894318c
5 changed files with 85 additions and 68 deletions

View File

@ -559,8 +559,13 @@ public class PluralAttributeMappingImpl
SqlAstCreationContext creationContext) {
final SqlAstJoinType joinType;
if ( requestedJoinType == null ) {
if ( fetched ) {
joinType = getDefaultSqlAstJoinType( lhs );
}
else {
joinType = SqlAstJoinType.INNER;
}
}
else {
joinType = requestedJoinType;
}

View File

@ -100,8 +100,6 @@ import org.hibernate.type.EmbeddedComponentType;
import org.hibernate.type.EntityType;
import org.hibernate.type.Type;
import static java.util.Objects.requireNonNullElse;
/**
* @author Steve Ebersole
*/
@ -1221,9 +1219,8 @@ public class ToOneAttributeMapping
having the left join we don't want to add an extra implicit join that will be translated into an SQL inner join (see HHH-15342)
*/
if ( ( ( hasNotFoundAction() && doesNotExistATableGroupJoin( parentTableGroup, fetchablePath ) )
|| ( fetchTiming == FetchTiming.IMMEDIATE && selected ) ) ) {
final TableGroup tableGroup = determineTableGroup(
if ( fetchTiming == FetchTiming.IMMEDIATE && selected || hasNotFoundAction() ) {
final TableGroup tableGroup = determineTableGroupForFetch(
fetchablePath,
fetchParent,
parentTableGroup,
@ -1332,59 +1329,67 @@ public class ToOneAttributeMapping
);
}
private boolean doesNotExistATableGroupJoin(TableGroup parentTableGroup, NavigablePath fetchablePath) {
for ( TableGroupJoin tableGroupJoin : parentTableGroup.getTableGroupJoins() ) {
if ( tableGroupJoin.getNavigablePath().pathsMatch( fetchablePath ) ) {
return false;
}
}
return true;
}
private TableGroup determineTableGroup(NavigablePath fetchablePath, FetchParent fetchParent, TableGroup parentTableGroup, String resultVariable, FromClauseAccess fromClauseAccess, DomainResultCreationState creationState) {
final TableGroup tableGroup;
private TableGroup determineTableGroupForFetch(
NavigablePath fetchablePath,
FetchParent fetchParent,
TableGroup parentTableGroup,
String resultVariable,
FromClauseAccess fromClauseAccess,
DomainResultCreationState creationState) {
final SqlAstJoinType joinType;
if ( fetchParent instanceof EntityResultJoinedSubclassImpl
&& ( (EntityPersister) fetchParent.getReferencedModePart() ).findDeclaredAttributeMapping( getPartName() ) == null ) {
final TableGroupJoin tableGroupJoin = createTableGroupJoin(
fetchablePath,
parentTableGroup,
resultVariable,
getJoinType( fetchablePath, parentTableGroup ),
true,
false,
creationState.getSqlAstCreationState()
);
if ( hasNotFoundAction() ) {
tableGroupJoin.setImplicit();
}
parentTableGroup.addTableGroupJoin( tableGroupJoin );
tableGroup = tableGroupJoin.getJoinedGroup();
fromClauseAccess.registerTableGroup( fetchablePath, tableGroup );
joinType = getJoinTypeForFetch( fetchablePath, parentTableGroup );
}
else {
tableGroup = fromClauseAccess.resolveTableGroup(
joinType = null;
}
return fromClauseAccess.resolveTableGroup(
fetchablePath,
np -> {
// Try to reuse an existing join if possible,
// and note that we prefer reusing an inner over a left join,
// because a left join might stay uninitialized if unused
TableGroup leftJoined = null;
for ( TableGroupJoin tableGroupJoin : parentTableGroup.getTableGroupJoins() ) {
switch ( tableGroupJoin.getJoinType() ) {
case INNER:
// If this is an inner joins, it's fine if the paths match
// Since this inner join would filter the parent row anyway,
// it makes no sense to add another left join for this association
if ( tableGroupJoin.getNavigablePath().pathsMatch( np ) ) {
return tableGroupJoin.getJoinedGroup();
}
break;
case LEFT:
// For an existing left join on the other hand which is row preserving,
// it is important to check if the predicate has user defined bits in it
// and only if it doesn't, we can reuse the join
if ( tableGroupJoin.getNavigablePath().pathsMatch( np )
&& isSimpleJoinPredicate( tableGroupJoin.getPredicate() ) ) {
leftJoined = tableGroupJoin.getJoinedGroup();
}
}
}
if ( leftJoined != null ) {
return leftJoined;
}
final TableGroupJoin tableGroupJoin = createTableGroupJoin(
fetchablePath,
parentTableGroup,
resultVariable,
getDefaultSqlAstJoinType( parentTableGroup ),
joinType,
true,
false,
creationState.getSqlAstCreationState()
);
if ( hasNotFoundAction() ) {
tableGroupJoin.setImplicit();
}
parentTableGroup.addTableGroupJoin( tableGroupJoin );
return tableGroupJoin.getJoinedGroup();
}
);
}
return tableGroup;
}
private boolean isSelectByUniqueKey(ForeignKeyDescriptor.Nature side) {
if ( side == ForeignKeyDescriptor.Nature.KEY ) {
@ -1531,7 +1536,9 @@ public class ToOneAttributeMapping
@Override
public boolean isSimpleJoinPredicate(Predicate predicate) {
return foreignKeyDescriptor.isSimpleJoinPredicate( predicate );
// Since the table group is lazy, the initial predicate is null,
// but if we get null here, we can safely assume this will be a simple join predicate
return predicate == null || foreignKeyDescriptor.isSimpleJoinPredicate( predicate );
}
@Override
@ -1555,7 +1562,18 @@ public class ToOneAttributeMapping
// This is vital for the map key property check that comes next
assert !( lhs instanceof PluralTableGroup );
final SqlAstJoinType joinType = requireNonNullElse( requestedJoinType, SqlAstJoinType.INNER );
final SqlAstJoinType joinType;
if ( requestedJoinType == null ) {
if ( fetched ) {
joinType = getDefaultSqlAstJoinType( lhs );
}
else {
joinType = SqlAstJoinType.INNER;
}
}
else {
joinType = requestedJoinType;
}
// If a parent is a collection part, there is no custom predicate and the join is INNER or LEFT
// we check if this attribute is the map key property to reuse the existing index table group
@ -1800,13 +1818,13 @@ public class ToOneAttributeMapping
}
}
private SqlAstJoinType getJoinType(NavigablePath navigablePath, TableGroup tableGroup) {
private SqlAstJoinType getJoinTypeForFetch(NavigablePath navigablePath, TableGroup tableGroup) {
for ( TableGroupJoin tableGroupJoin : tableGroup.getTableGroupJoins() ) {
if ( tableGroupJoin.getNavigablePath().equals( navigablePath ) ) {
return tableGroupJoin.getJoinType();
}
}
return getDefaultSqlAstJoinType( tableGroup );
return null;
}
public TableGroup createTableGroupInternal(

View File

@ -3312,7 +3312,6 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
false,
this
);
tableGroupJoin.setImplicit();
// Implicit joins in the ON clause of attribute joins need to be added as nested table group joins
// We don't have to do that for entity joins etc. as these do not have an inherent dependency on the lhs.
// We can just add the implicit join before the currently processing join
@ -6987,7 +6986,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
fetchablePath,
lhs,
alias,
tableGroupJoinProducer.getDefaultSqlAstJoinType( lhs ),
null,
true,
false,
BaseSqmToSqlAstConverter.this

View File

@ -7,6 +7,7 @@
package org.hibernate.query.sqm.sql.internal;
import org.hibernate.NotYetImplementedFor6Exception;
import org.hibernate.metamodel.mapping.CollectionPart;
import org.hibernate.metamodel.mapping.PluralAttributeMapping;
import org.hibernate.spi.NavigablePath;
import org.hibernate.query.sqm.sql.SqmToSqlAstConverter;
@ -60,7 +61,7 @@ public class PluralValuedSimplePathInterpretation<T> extends AbstractSqmPathInte
// This is only invoked when a plural attribute is a top level select, order by or group by item
// in which case we have to produce results for the element
return ( (PluralAttributeMapping) getExpressionType() ).getElementDescriptor().createDomainResult(
getNavigablePath(),
getNavigablePath().append( CollectionPart.Nature.ELEMENT.getName() ),
getTableGroup(),
resultVariable,
creationState

View File

@ -27,8 +27,6 @@ public class TableGroupJoin implements TableJoin, DomainResultProducer {
private SqlAstJoinType joinType;
private Predicate predicate;
private boolean implicit;
public TableGroupJoin(
NavigablePath navigablePath,
SqlAstJoinType joinType,
@ -93,12 +91,8 @@ public class TableGroupJoin implements TableJoin, DomainResultProducer {
return navigablePath;
}
public void setImplicit(){
this.implicit = true;
}
public boolean isImplicit() {
return implicit;
return !navigablePath.isAliased();
}
@Override