HHH-17837 Render target-side key for explicit plural joins when needed
Also, change how we determine whether we need to use the target-side to only the strictly needed cases (non-optimizable joins, `group by` or `order by` clauses)
This commit is contained in:
parent
b2f1725520
commit
9ba0dd7af0
|
@ -6,6 +6,8 @@
|
|||
*/
|
||||
package org.hibernate.metamodel.mapping;
|
||||
|
||||
import java.util.Set;
|
||||
|
||||
import org.hibernate.sql.ast.tree.from.TableGroupJoinProducer;
|
||||
|
||||
/**
|
||||
|
@ -21,6 +23,8 @@ public interface EntityAssociationMapping extends ModelPart, Association, TableG
|
|||
|
||||
EntityMappingType getAssociatedEntityMappingType();
|
||||
|
||||
Set<String> getTargetKeyPropertyNames();
|
||||
|
||||
/**
|
||||
* The model sub-part relative to the associated entity type that is the target
|
||||
* of this association's foreign-key
|
||||
|
|
|
@ -60,7 +60,7 @@ public abstract class AbstractEntityCollectionPart implements EntityCollectionPa
|
|||
private final EntityMappingType associatedEntityTypeDescriptor;
|
||||
private final NotFoundAction notFoundAction;
|
||||
|
||||
private final Set<String> targetKeyPropertyNames;
|
||||
protected final Set<String> targetKeyPropertyNames;
|
||||
|
||||
public AbstractEntityCollectionPart(
|
||||
Nature nature,
|
||||
|
@ -110,10 +110,6 @@ public abstract class AbstractEntityCollectionPart implements EntityCollectionPa
|
|||
return getAssociatedEntityMappingType();
|
||||
}
|
||||
|
||||
protected Set<String> getTargetKeyPropertyNames() {
|
||||
return targetKeyPropertyNames;
|
||||
}
|
||||
|
||||
@Override
|
||||
public NavigableRole getNavigableRole() {
|
||||
return navigableRole;
|
||||
|
|
|
@ -7,6 +7,7 @@
|
|||
package org.hibernate.metamodel.mapping.internal;
|
||||
|
||||
import java.util.Locale;
|
||||
import java.util.Set;
|
||||
import java.util.function.Consumer;
|
||||
|
||||
import org.hibernate.annotations.NotFoundAction;
|
||||
|
@ -135,6 +136,11 @@ public class ManyToManyCollectionPart extends AbstractEntityCollectionPart imple
|
|||
return super.findSubPart( name, targetType );
|
||||
}
|
||||
|
||||
@Override
|
||||
public Set<String> getTargetKeyPropertyNames() {
|
||||
return targetKeyPropertyNames;
|
||||
}
|
||||
|
||||
@Override
|
||||
public <X, Y> int breakDownJdbcValues(
|
||||
Object domainValue,
|
||||
|
|
|
@ -899,6 +899,7 @@ public class ToOneAttributeMapping
|
|||
return targetKeyPropertyName;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Set<String> getTargetKeyPropertyNames() {
|
||||
return targetKeyPropertyNames;
|
||||
}
|
||||
|
|
|
@ -25,13 +25,14 @@ import org.hibernate.internal.util.collections.CollectionHelper;
|
|||
import org.hibernate.metamodel.MappingMetamodel;
|
||||
import org.hibernate.metamodel.mapping.BasicValuedMapping;
|
||||
import org.hibernate.metamodel.mapping.Bindable;
|
||||
import org.hibernate.metamodel.mapping.CollectionPart;
|
||||
import org.hibernate.metamodel.mapping.EntityAssociationMapping;
|
||||
import org.hibernate.metamodel.mapping.EntityIdentifierMapping;
|
||||
import org.hibernate.metamodel.mapping.EntityMappingType;
|
||||
import org.hibernate.metamodel.mapping.ForeignKeyDescriptor;
|
||||
import org.hibernate.metamodel.mapping.JdbcMapping;
|
||||
import org.hibernate.metamodel.mapping.ManagedMappingType;
|
||||
import org.hibernate.metamodel.mapping.MappingModelExpressible;
|
||||
import org.hibernate.metamodel.mapping.ModelPart;
|
||||
import org.hibernate.metamodel.mapping.ModelPartContainer;
|
||||
import org.hibernate.metamodel.mapping.PluralAttributeMapping;
|
||||
import org.hibernate.query.IllegalQueryOperationException;
|
||||
|
@ -46,6 +47,7 @@ import org.hibernate.query.sqm.SqmPathSource;
|
|||
import org.hibernate.query.sqm.SqmQuerySource;
|
||||
import org.hibernate.query.sqm.spi.JdbcParameterBySqmParameterAccess;
|
||||
import org.hibernate.query.sqm.spi.SqmParameterMappingModelResolutionAccess;
|
||||
import org.hibernate.query.sqm.sql.SqmToSqlAstConverter;
|
||||
import org.hibernate.query.sqm.tree.SqmDmlStatement;
|
||||
import org.hibernate.query.sqm.tree.SqmJoinType;
|
||||
import org.hibernate.query.sqm.tree.SqmStatement;
|
||||
|
@ -60,11 +62,13 @@ import org.hibernate.query.sqm.tree.from.SqmFrom;
|
|||
import org.hibernate.query.sqm.tree.from.SqmJoin;
|
||||
import org.hibernate.query.sqm.tree.from.SqmQualifiedJoin;
|
||||
import org.hibernate.query.sqm.tree.from.SqmRoot;
|
||||
import org.hibernate.query.sqm.tree.select.SqmQueryPart;
|
||||
import org.hibernate.query.sqm.tree.select.SqmSelectStatement;
|
||||
import org.hibernate.query.sqm.tree.select.SqmSelectableNode;
|
||||
import org.hibernate.query.sqm.tree.select.SqmSelection;
|
||||
import org.hibernate.query.sqm.tree.select.SqmSortSpecification;
|
||||
import org.hibernate.spi.NavigablePath;
|
||||
import org.hibernate.sql.ast.Clause;
|
||||
import org.hibernate.sql.ast.SqlTreeCreationException;
|
||||
import org.hibernate.sql.ast.tree.expression.JdbcParameter;
|
||||
import org.hibernate.sql.ast.tree.from.TableGroup;
|
||||
|
@ -79,6 +83,7 @@ import org.hibernate.type.internal.BasicTypeImpl;
|
|||
import org.hibernate.type.internal.ConvertedBasicTypeImpl;
|
||||
import org.hibernate.type.spi.TypeConfiguration;
|
||||
|
||||
import static org.hibernate.internal.util.NullnessUtil.castNonNull;
|
||||
import static org.hibernate.query.sqm.tree.jpa.ParameterCollector.collectParameters;
|
||||
|
||||
/**
|
||||
|
@ -132,13 +137,56 @@ public class SqmUtil {
|
|||
}
|
||||
|
||||
/**
|
||||
* Utility that returns {@code true} if the specified {@link SqmPath sqmPath} should be
|
||||
* dereferenced using the target table mapping, i.e. when the path's lhs is an explicit join.
|
||||
* Utility that returns the entity association target's mapping type if the specified {@code sqmPath} should
|
||||
* be dereferenced using the target table, i.e. when the path's lhs is an explicit join that is used in the
|
||||
* group by clause, or defaults to the provided {@code modelPartContainer} otherwise.
|
||||
*/
|
||||
public static boolean needsTargetTableMapping(SqmPath<?> sqmPath, ModelPartContainer modelPartContainer) {
|
||||
return modelPartContainer.getPartMappingType() != modelPartContainer
|
||||
&& sqmPath.getLhs() instanceof SqmFrom<?, ?>
|
||||
&& modelPartContainer.getPartMappingType() instanceof ManagedMappingType;
|
||||
public static ModelPartContainer getTargetMappingIfNeeded(
|
||||
SqmPath<?> sqmPath,
|
||||
ModelPartContainer modelPartContainer,
|
||||
SqmToSqlAstConverter sqlAstCreationState) {
|
||||
final SqmQueryPart<?> queryPart = sqlAstCreationState.getCurrentSqmQueryPart();
|
||||
if ( queryPart != null ) {
|
||||
// We only need to do this for queries
|
||||
final Clause clause = sqlAstCreationState.getCurrentClauseStack().getCurrent();
|
||||
if ( clause != Clause.FROM && modelPartContainer.getPartMappingType() != modelPartContainer && sqmPath.getLhs() instanceof SqmFrom<?, ?> ) {
|
||||
final ModelPart modelPart;
|
||||
if ( modelPartContainer instanceof PluralAttributeMapping ) {
|
||||
modelPart = getCollectionPart(
|
||||
(PluralAttributeMapping) modelPartContainer,
|
||||
castNonNull( sqmPath.getNavigablePath().getParent() )
|
||||
);
|
||||
}
|
||||
else {
|
||||
modelPart = modelPartContainer;
|
||||
}
|
||||
if ( modelPart instanceof EntityAssociationMapping ) {
|
||||
final EntityAssociationMapping association = (EntityAssociationMapping) modelPart;
|
||||
// If the path is one of the association's target key properties,
|
||||
// we need to render the target side if in group/order by
|
||||
if ( association.getTargetKeyPropertyNames().contains( sqmPath.getReferencedPathSource().getPathName() )
|
||||
&& ( clause == Clause.GROUP || clause == Clause.ORDER
|
||||
|| !isFkOptimizationAllowed( sqmPath.getLhs() )
|
||||
|| queryPart.getFirstQuerySpec().groupByClauseContains( sqmPath.getNavigablePath() ) ) ) {
|
||||
return association.getAssociatedEntityMappingType();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return modelPartContainer;
|
||||
}
|
||||
|
||||
private static CollectionPart getCollectionPart(PluralAttributeMapping attribute, NavigablePath path) {
|
||||
final CollectionPart.Nature nature = CollectionPart.Nature.fromNameExact( path.getLocalName() );
|
||||
if ( nature != null ) {
|
||||
switch ( nature ) {
|
||||
case ELEMENT:
|
||||
return attribute.getElementDescriptor();
|
||||
case INDEX:
|
||||
return attribute.getIndexDescriptor();
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -13,7 +13,6 @@ import java.util.function.Consumer;
|
|||
import org.hibernate.metamodel.MappingMetamodel;
|
||||
import org.hibernate.metamodel.mapping.BasicValuedModelPart;
|
||||
import org.hibernate.metamodel.mapping.EntityMappingType;
|
||||
import org.hibernate.metamodel.mapping.ManagedMappingType;
|
||||
import org.hibernate.metamodel.mapping.MappingType;
|
||||
import org.hibernate.metamodel.mapping.ModelPart;
|
||||
import org.hibernate.metamodel.mapping.ModelPartContainer;
|
||||
|
@ -35,7 +34,7 @@ import org.hibernate.sql.ast.tree.from.TableReference;
|
|||
import org.hibernate.sql.ast.tree.update.Assignable;
|
||||
|
||||
import static org.hibernate.internal.util.NullnessUtil.castNonNull;
|
||||
import static org.hibernate.query.sqm.internal.SqmUtil.needsTargetTableMapping;
|
||||
import static org.hibernate.query.sqm.internal.SqmUtil.getTargetMappingIfNeeded;
|
||||
|
||||
/**
|
||||
* @author Steve Ebersole
|
||||
|
@ -82,20 +81,12 @@ public class BasicValuedPathInterpretation<T> extends AbstractSqmPathInterpretat
|
|||
}
|
||||
}
|
||||
|
||||
final ModelPart modelPart;
|
||||
if ( needsTargetTableMapping( sqmPath, modelPartContainer ) ) {
|
||||
// We have to make sure we render the column of the target table
|
||||
modelPart = ( (ManagedMappingType) modelPartContainer.getPartMappingType() ).findSubPart(
|
||||
sqmPath.getReferencedPathSource().getPathName(),
|
||||
treatTarget
|
||||
);
|
||||
}
|
||||
else {
|
||||
modelPart = modelPartContainer.findSubPart(
|
||||
sqmPath.getReferencedPathSource().getPathName(),
|
||||
treatTarget
|
||||
);
|
||||
}
|
||||
// Use the target type to find the sub part if needed, otherwise just use the container
|
||||
final ModelPart modelPart = getTargetMappingIfNeeded(
|
||||
sqmPath,
|
||||
modelPartContainer,
|
||||
sqlAstCreationState
|
||||
).findSubPart( sqmPath.getReferencedPathSource().getPathName(), treatTarget );
|
||||
|
||||
if ( modelPart == null ) {
|
||||
if ( jpaQueryComplianceEnabled ) {
|
||||
|
|
|
@ -13,7 +13,6 @@ import java.util.function.Consumer;
|
|||
import org.hibernate.metamodel.MappingMetamodel;
|
||||
import org.hibernate.metamodel.mapping.EmbeddableValuedModelPart;
|
||||
import org.hibernate.metamodel.mapping.EntityMappingType;
|
||||
import org.hibernate.metamodel.mapping.ManagedMappingType;
|
||||
import org.hibernate.metamodel.mapping.ModelPartContainer;
|
||||
import org.hibernate.metamodel.model.domain.EntityDomainType;
|
||||
import org.hibernate.query.sqm.sql.SqmToSqlAstConverter;
|
||||
|
@ -29,7 +28,7 @@ import org.hibernate.sql.ast.tree.expression.SqlTupleContainer;
|
|||
import org.hibernate.sql.ast.tree.from.TableGroup;
|
||||
import org.hibernate.sql.ast.tree.update.Assignable;
|
||||
|
||||
import static org.hibernate.query.sqm.internal.SqmUtil.needsTargetTableMapping;
|
||||
import static org.hibernate.query.sqm.internal.SqmUtil.getTargetMappingIfNeeded;
|
||||
|
||||
/**
|
||||
* @author Steve Ebersole
|
||||
|
@ -65,20 +64,12 @@ public class EmbeddableValuedPathInterpretation<T> extends AbstractSqmPathInterp
|
|||
}
|
||||
|
||||
final ModelPartContainer modelPartContainer = tableGroup.getModelPart();
|
||||
final EmbeddableValuedModelPart mapping;
|
||||
if ( needsTargetTableMapping( sqmPath, modelPartContainer ) ) {
|
||||
// We have to make sure we render the column of the target table
|
||||
mapping = (EmbeddableValuedModelPart) ( (ManagedMappingType) modelPartContainer.getPartMappingType() ).findSubPart(
|
||||
sqmPath.getReferencedPathSource().getPathName(),
|
||||
treatTarget
|
||||
);
|
||||
}
|
||||
else {
|
||||
mapping = (EmbeddableValuedModelPart) modelPartContainer.findSubPart(
|
||||
sqmPath.getReferencedPathSource().getPathName(),
|
||||
treatTarget
|
||||
);
|
||||
}
|
||||
// Use the target type to find the sub part if needed, otherwise just use the container
|
||||
final EmbeddableValuedModelPart mapping = (EmbeddableValuedModelPart) getTargetMappingIfNeeded(
|
||||
sqmPath,
|
||||
modelPartContainer,
|
||||
sqlAstCreationState
|
||||
).findSubPart( sqmPath.getReferencedPathSource().getPathName(), treatTarget );
|
||||
|
||||
return new EmbeddableValuedPathInterpretation<>(
|
||||
mapping.toSqlExpression(
|
||||
|
|
|
@ -162,7 +162,9 @@ public class EntityValuedPathInterpretation<T> extends AbstractSqmPathInterpreta
|
|||
// we try to make use of it and the FK model part if possible based on the inferred mapping
|
||||
if ( mapping instanceof EntityAssociationMapping ) {
|
||||
final EntityAssociationMapping associationMapping = (EntityAssociationMapping) mapping;
|
||||
final ModelPart keyTargetMatchPart = associationMapping.getKeyTargetMatchPart();
|
||||
final ModelPart keyTargetMatchPart = associationMapping.getForeignKeyDescriptor().getPart(
|
||||
associationMapping.getSideNature()
|
||||
);
|
||||
|
||||
if ( associationMapping.isFkOptimizationAllowed() ) {
|
||||
final boolean forceUsingForeignKeyAssociationSidePart;
|
||||
|
|
|
@ -628,8 +628,8 @@ public class SqmQuerySpec<T> extends SqmQueryPart<T>
|
|||
}
|
||||
|
||||
public boolean groupByClauseContains(NavigablePath path) {
|
||||
for ( SqmExpression<?> expression : groupByClauseExpressions ) {
|
||||
if ( expression instanceof SqmPath && ( (SqmPath<?>) expression ).getNavigablePath() == path ) {
|
||||
for ( final SqmExpression<?> expression : groupByClauseExpressions ) {
|
||||
if ( expression instanceof SqmPath && ( (SqmPath<?>) expression ).getNavigablePath().isParentOrEqual( path ) ) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -52,15 +52,15 @@ public class MapIssueTest {
|
|||
}
|
||||
|
||||
@Test
|
||||
public void testMapKeyJoinIsIncluded(SessionFactoryScope scope) {
|
||||
public void testMapKeyJoinIsOmitted(SessionFactoryScope scope) {
|
||||
SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();
|
||||
statementInspector.clear();
|
||||
scope.inTransaction(
|
||||
s -> {
|
||||
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 3 joins, collection table, collection element and relationship
|
||||
statementInspector.assertNumberOfJoins( 0, 3 );
|
||||
// Assert 2 joins, collection table and collection element. No need to join the relationship because it is not nullable
|
||||
statementInspector.assertNumberOfJoins( 0, 2 );
|
||||
}
|
||||
);
|
||||
}
|
||||
|
|
|
@ -152,8 +152,9 @@ public class EntityWithBidirectionalAssociationsOneOfWhichIsAJoinTableTest {
|
|||
.getSingleResult();
|
||||
|
||||
statementInspector.assertExecutedCount( 2 );
|
||||
// The join to the target table PARENT for Male#parent is added since it's explicitly joined in HQL
|
||||
statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 2 );
|
||||
// The join to the target table PARENT for Male#parent is avoided,
|
||||
// because the FK in the collection table is not-null and data from the target table is not needed
|
||||
statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 1 );
|
||||
statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 3 );
|
||||
assertThat( son.getParent(), CoreMatchers.notNullValue() );
|
||||
|
||||
|
|
Loading…
Reference in New Issue