From 985467bcbabc084a1c2f4a41ac64eca90e353487 Mon Sep 17 00:00:00 2001 From: Andrea Boriero Date: Wed, 22 Jun 2022 15:27:58 +0200 Subject: [PATCH] HHH-15342 Inappropriate variation of HQL left join to SQL inner join --- .../internal/ToOneAttributeMapping.java | 44 ++++++++++++++++++- .../sqm/sql/BaseSqmToSqlAstConverter.java | 3 +- .../sql/ast/tree/from/TableGroupJoin.java | 10 +++++ .../internal/EntityFetchSelectImpl.java | 2 - 4 files changed, 54 insertions(+), 5 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ToOneAttributeMapping.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ToOneAttributeMapping.java index 850f2c7168..284e06ebc8 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ToOneAttributeMapping.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ToOneAttributeMapping.java @@ -1196,9 +1196,33 @@ public class ToOneAttributeMapping || fetchParent.getNavigablePath() instanceof TreatedNavigablePath && parentNavigablePath.equals( fetchParent.getNavigablePath().getRealParent() ); + /* + In case of @NotFound we are going to add fetch for the `fetchablePath` only if there is not already a `TableGroupJoin`. - if ( (hasNotFoundAction() - || ( fetchTiming == FetchTiming.IMMEDIATE && selected )) ) { + e.g. given : + public static class EntityA { + ... + + @ManyToOne(fetch = FetchType.LAZY) + @NotFound(action = NotFoundAction.IGNORE) + private EntityB entityB; + } + + @Entity(name = "EntityB") + public static class EntityB { + ... + + private String name; + } + + and the HQL query : + + `Select a From EntityA a Left Join a.entityB b Where ( b.name IS NOT NULL )` + + 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( fetchablePath, fetchParent, @@ -1308,6 +1332,15 @@ 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; if ( fetchParent instanceof EntityResultJoinedSubclassImpl @@ -1321,6 +1354,10 @@ public class ToOneAttributeMapping false, creationState.getSqlAstCreationState() ); + if ( hasNotFoundAction() ) { + tableGroupJoin.setImplicit(); + } + parentTableGroup.addTableGroupJoin( tableGroupJoin ); tableGroup = tableGroupJoin.getJoinedGroup(); fromClauseAccess.registerTableGroup( fetchablePath, tableGroup ); @@ -1338,6 +1375,9 @@ public class ToOneAttributeMapping false, creationState.getSqlAstCreationState() ); + if ( hasNotFoundAction() ) { + tableGroupJoin.setImplicit(); + } parentTableGroup.addTableGroupJoin( tableGroupJoin ); return tableGroupJoin.getJoinedGroup(); } 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 8350bd99aa..df2b52e42a 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 @@ -3209,7 +3209,7 @@ public abstract class BaseSqmToSqlAstConverter extends Base final NavigablePath parentParentPath = parentPath.getParentPath().getNavigablePath(); final TableGroup parentParentTableGroup = fromClauseIndex.findTableGroup( parentParentPath ); parentParentTableGroup.visitTableGroupJoins( (join) -> { - if ( join.getNavigablePath().equals( parentPath.getNavigablePath() ) ) { + if ( join.getNavigablePath().equals( parentPath.getNavigablePath() ) && join.isImplicit() ) { join.setJoinType( SqlAstJoinType.INNER ); } } ); @@ -3312,6 +3312,7 @@ public abstract class BaseSqmToSqlAstConverter 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 diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableGroupJoin.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableGroupJoin.java index ad5d404b3e..f57d7eba7a 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableGroupJoin.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableGroupJoin.java @@ -27,6 +27,8 @@ public class TableGroupJoin implements TableJoin, DomainResultProducer { private SqlAstJoinType joinType; private Predicate predicate; + private boolean implicit; + public TableGroupJoin( NavigablePath navigablePath, SqlAstJoinType joinType, @@ -91,6 +93,14 @@ public class TableGroupJoin implements TableJoin, DomainResultProducer { return navigablePath; } + public void setImplicit(){ + this.implicit = true; + } + + public boolean isImplicit(){ + return implicit; + } + @Override public DomainResult createDomainResult( String resultVariable, diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchSelectImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchSelectImpl.java index 73f9ca6327..4dc1bab3c5 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchSelectImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchSelectImpl.java @@ -36,8 +36,6 @@ public class EntityFetchSelectImpl extends AbstractNonJoinedEntityFetch { @SuppressWarnings("unused") DomainResultCreationState creationState) { super( navigablePath, fetchedAttribute, fetchParent ); - assert fetchedAttribute.getNotFoundAction() == null; - this.keyResult = keyResult; this.selectByUniqueKey = selectByUniqueKey; }