From 19da8f3e7ccc82a320a2f4dc5753c85306618106 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Mon, 17 Apr 2023 14:18:42 +0200 Subject: [PATCH] HHH-16468 Simplify embeddable key handling --- .../sqm/sql/BaseSqmToSqlAstConverter.java | 25 +++++----- .../internal/EmbeddableValuedExpression.java | 8 +-- .../AbstractEmbeddableInitializer.java | 49 ++++++++++--------- ...ggregatedIdentifierMappingInitializer.java | 20 +++++--- .../AggregateEmbeddableFetchInitializer.java | 14 ++++-- .../AggregateEmbeddableResultInitializer.java | 14 ++++-- 6 files changed, 78 insertions(+), 52 deletions(-) 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 aac15f504a..f77a05418a 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 @@ -5935,30 +5935,31 @@ public abstract class BaseSqmToSqlAstConverter extends Base if ( result instanceof SqlTupleContainer ) { return result; } + final NavigablePath baseNavigablePath; final Object offset; if ( lhs != expression.getLeftHandOperand() ) { - offset = ( (SqmPath) expression.getLeftHandOperand() ).get( + final SqmPath temporalPath = (SqmPath) expression.getLeftHandOperand(); + baseNavigablePath = temporalPath.getNavigablePath().getParent(); + offset = (temporalPath).get( AbstractTimeZoneStorageCompositeUserType.ZONE_OFFSET_NAME ).accept( this ); } else if ( rhs != expression.getRightHandOperand() ) { - offset = ( (SqmPath) expression.getRightHandOperand() ).get( + final SqmPath temporalPath = (SqmPath) expression.getRightHandOperand(); + baseNavigablePath = temporalPath.getNavigablePath().getParent(); + offset = ( temporalPath ).get( AbstractTimeZoneStorageCompositeUserType.ZONE_OFFSET_NAME ).accept( this ); } else { - offset = null; - } - if ( offset == null ) { return result; } - else { - final EmbeddableValuedModelPart valueMapping = (EmbeddableValuedModelPart) determineValueMapping( expression ); - return new EmbeddableValuedExpression<>( - valueMapping, - new SqlTuple( List.of( (Expression) result, (Expression) offset ), valueMapping ) - ); - } + final EmbeddableValuedModelPart valueMapping = (EmbeddableValuedModelPart) determineValueMapping( expression ); + return new EmbeddableValuedExpression<>( + baseNavigablePath, + valueMapping, + new SqlTuple( List.of( (Expression) result, (Expression) offset ), valueMapping ) + ); } finally { if ( operator == SUBTRACT ) { diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EmbeddableValuedExpression.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EmbeddableValuedExpression.java index 60eb6b86c5..d54b8c3deb 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EmbeddableValuedExpression.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EmbeddableValuedExpression.java @@ -19,7 +19,6 @@ import org.hibernate.spi.NavigablePath; import org.hibernate.sql.ast.SqlAstWalker; import org.hibernate.sql.ast.spi.SqlAstCreationState; import org.hibernate.sql.ast.spi.SqlExpressionResolver; -import org.hibernate.sql.ast.spi.SqlSelection; import org.hibernate.sql.ast.tree.expression.ColumnReference; import org.hibernate.sql.ast.tree.expression.Expression; import org.hibernate.sql.ast.tree.expression.SqlTuple; @@ -40,11 +39,14 @@ public class EmbeddableValuedExpression implements Expression, DomainResultPr private final EmbeddableValuedModelPart mapping; private final SqlTuple sqlExpression; - public EmbeddableValuedExpression(EmbeddableValuedModelPart mapping, SqlTuple sqlExpression) { + public EmbeddableValuedExpression( + NavigablePath baseNavigablePath, + EmbeddableValuedModelPart mapping, + SqlTuple sqlExpression) { assert mapping != null; assert sqlExpression != null; assert mapping.getEmbeddableTypeDescriptor().getNumberOfAttributeMappings() == sqlExpression.getExpressions().size(); - this.navigablePath = new NavigablePath( mapping.getPartName(), Long.toString( System.nanoTime() ) ); + this.navigablePath = baseNavigablePath.append( mapping.getPartName(), Long.toString( System.nanoTime() ) ); this.mapping = mapping; this.sqlExpression = sqlExpression; } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/AbstractEmbeddableInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/AbstractEmbeddableInitializer.java index 0b0f4d0af0..099c4d3c2e 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/AbstractEmbeddableInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/AbstractEmbeddableInitializer.java @@ -6,8 +6,6 @@ */ package org.hibernate.sql.results.graph.embeddable; -import java.util.List; - import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.metamodel.mapping.CompositeIdentifierMapping; import org.hibernate.metamodel.mapping.EmbeddableMappingType; @@ -19,6 +17,7 @@ import org.hibernate.metamodel.spi.ValueAccess; import org.hibernate.property.access.spi.PropertyAccess; import org.hibernate.proxy.HibernateProxy; import org.hibernate.proxy.LazyInitializer; +import org.hibernate.spi.EntityIdentifierNavigablePath; import org.hibernate.spi.NavigablePath; import org.hibernate.sql.results.graph.AbstractFetchParentAccess; import org.hibernate.sql.results.graph.AssemblerCreationState; @@ -32,7 +31,6 @@ import org.hibernate.sql.results.graph.entity.EntityInitializer; import org.hibernate.sql.results.internal.NullValueAssembler; import org.hibernate.sql.results.jdbc.spi.RowProcessingState; -import static org.hibernate.internal.util.collections.CollectionHelper.arrayList; import static org.hibernate.sql.results.graph.entity.internal.BatchEntityInsideEmbeddableSelectFetchInitializer.BATCH_PROPERTY; /** @@ -44,10 +42,11 @@ public abstract class AbstractEmbeddableInitializer extends AbstractFetchParentA private final NavigablePath navigablePath; private final EmbeddableValuedModelPart embedded; private final FetchParentAccess fetchParentAccess; + private final boolean isPartOfKey; private final boolean createEmptyCompositesEnabled; private final SessionFactoryImplementor sessionFactory; - private final List> assemblers; + protected final DomainResultAssembler[] assemblers; // per-row state private final Object[] rowState; @@ -66,23 +65,28 @@ public abstract class AbstractEmbeddableInitializer extends AbstractFetchParentA final EmbeddableMappingType embeddableTypeDescriptor = embedded.getEmbeddableTypeDescriptor(); final int size = embeddableTypeDescriptor.getNumberOfFetchables(); this.rowState = new Object[ size ]; - this.assemblers = arrayList( size ); + this.isPartOfKey = isPartOfKey( navigablePath ); // We never want to create empty composites for the FK target or PK, otherwise collections would break - createEmptyCompositesEnabled = !ForeignKeyDescriptor.PART_NAME.equals( navigablePath.getLocalName() ) - && !ForeignKeyDescriptor.TARGET_PART_NAME.equals( navigablePath.getLocalName() ) - && !EntityIdentifierMapping.ROLE_LOCAL_NAME.equals( navigablePath.getLocalName() ) - && embeddableTypeDescriptor.isCreateEmptyCompositesEnabled(); + this.createEmptyCompositesEnabled = !isPartOfKey && embeddableTypeDescriptor.isCreateEmptyCompositesEnabled(); - sessionFactory = creationState.getSqlAstCreationContext().getSessionFactory(); - initializeAssemblers( resultDescriptor, creationState, embeddableTypeDescriptor ); + this.sessionFactory = creationState.getSqlAstCreationContext().getSessionFactory(); + this.assemblers = createAssemblers( resultDescriptor, creationState, embeddableTypeDescriptor ); } - protected void initializeAssemblers( + private static boolean isPartOfKey(NavigablePath navigablePath) { + return ForeignKeyDescriptor.PART_NAME.equals( navigablePath.getLocalName() ) + || ForeignKeyDescriptor.TARGET_PART_NAME.equals( navigablePath.getLocalName() ) + || navigablePath instanceof EntityIdentifierNavigablePath + || navigablePath.getParent().getParent() != null && isPartOfKey( navigablePath.getParent() ); + } + + protected DomainResultAssembler[] createAssemblers( EmbeddableResultGraphNode resultDescriptor, AssemblerCreationState creationState, EmbeddableMappingType embeddableTypeDescriptor) { final int size = embeddableTypeDescriptor.getNumberOfFetchables(); + final DomainResultAssembler[] assemblers = new DomainResultAssembler[size]; for ( int i = 0; i < size; i++ ) { final Fetchable stateArrayContributor = embeddableTypeDescriptor.getFetchable( i ); final Fetch fetch = resultDescriptor.findFetch( stateArrayContributor ); @@ -91,8 +95,9 @@ public abstract class AbstractEmbeddableInitializer extends AbstractFetchParentA ? new NullValueAssembler<>( stateArrayContributor.getJavaType() ) : fetch.createAssembler( this, creationState ); - assemblers.add( stateAssembler ); + assemblers[i] = stateAssembler; } + return assemblers; } @Override @@ -220,6 +225,8 @@ public abstract class AbstractEmbeddableInitializer extends AbstractFetchParentA else { state = State.INJECTED; } + case INJECTED: + // Nothing to do } } @@ -253,15 +260,14 @@ public abstract class AbstractEmbeddableInitializer extends AbstractFetchParentA } private boolean isParentInstanceNull() { - if ( embedded instanceof CompositeIdentifierMapping ) { + if ( isPartOfKey ) { return false; } FetchParentAccess parentAccess = fetchParentAccess; while ( parentAccess != null && parentAccess.isEmbeddableInitializer() ) { - if ( parentAccess.getInitializedPart() instanceof CompositeIdentifierMapping ) { - return false; - } + assert !( parentAccess.getInitializedPart() instanceof CompositeIdentifierMapping ) + : "isPartOfKey should have been true in this case"; parentAccess = parentAccess.getFetchParentAccess(); } @@ -278,11 +284,8 @@ public abstract class AbstractEmbeddableInitializer extends AbstractFetchParentA private void extractRowState(RowProcessingState processingState) { boolean stateAllNull = true; - final boolean isKey = ForeignKeyDescriptor.PART_NAME.equals( navigablePath.getLocalName() ) - || ForeignKeyDescriptor.TARGET_PART_NAME.equals( navigablePath.getLocalName() ) - || EntityIdentifierMapping.ROLE_LOCAL_NAME.equals( embedded.getFetchableName() ); - for ( int i = 0; i < assemblers.size(); i++ ) { - final DomainResultAssembler assembler = assemblers.get( i ); + for ( int i = 0; i < assemblers.length; i++ ) { + final DomainResultAssembler assembler = assemblers[i]; final Object contributorValue = assembler.assemble( processingState, processingState.getJdbcValuesSourceProcessingState().getProcessingOptions() @@ -297,7 +300,7 @@ public abstract class AbstractEmbeddableInitializer extends AbstractFetchParentA if ( contributorValue != null ) { stateAllNull = false; } - else if ( isKey ) { + else if ( isPartOfKey ) { // If this is a foreign key and there is a null part, the whole thing has to be turned into null stateAllNull = true; break; diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/AbstractNonAggregatedIdentifierMappingInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/AbstractNonAggregatedIdentifierMappingInitializer.java index b2ad3de5b7..2d5ceddbc2 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/AbstractNonAggregatedIdentifierMappingInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/AbstractNonAggregatedIdentifierMappingInitializer.java @@ -51,7 +51,7 @@ public abstract class AbstractNonAggregatedIdentifierMappingInitializer extends private final FetchParentAccess fetchParentAccess; private final SessionFactoryImplementor sessionFactory; - private final List> assemblers; + private final DomainResultAssembler[] assemblers; private final boolean hasIdClass; @@ -77,28 +77,30 @@ public abstract class AbstractNonAggregatedIdentifierMappingInitializer extends final int size = virtualIdEmbeddable.getNumberOfFetchables(); this.virtualIdState = new Object[ size ]; this.idClassState = new Object[ size ]; - this.assemblers = arrayList( size ); this.sessionFactory = creationState.getSqlAstCreationContext().getSessionFactory(); - initializeAssemblers( resultDescriptor, creationState, virtualIdEmbeddable ); + this.assemblers = createAssemblers( this, resultDescriptor, creationState, virtualIdEmbeddable ); } - protected void initializeAssemblers( + protected static DomainResultAssembler[] createAssemblers( + FetchParentAccess parentAccess, EmbeddableResultGraphNode resultDescriptor, AssemblerCreationState creationState, EmbeddableMappingType embeddableTypeDescriptor) { final int size = embeddableTypeDescriptor.getNumberOfFetchables(); + final DomainResultAssembler[] assemblers = new DomainResultAssembler[size]; for ( int i = 0; i < size; i++ ) { final Fetchable stateArrayContributor = embeddableTypeDescriptor.getFetchable( i ); final Fetch fetch = resultDescriptor.findFetch( stateArrayContributor ); final DomainResultAssembler stateAssembler = fetch == null ? new NullValueAssembler<>( stateArrayContributor.getJavaType() ) - : fetch.createAssembler( this, creationState ); + : fetch.createAssembler( parentAccess, creationState ); - assemblers.add( stateAssembler ); + assemblers[i] = stateAssembler; } + return assemblers; } @Override @@ -209,13 +211,15 @@ public abstract class AbstractNonAggregatedIdentifierMappingInitializer extends state = State.INJECTED; } } + case INJECTED: + // Nothing to do } } private void extractRowState(RowProcessingState processingState) { state = State.NULL; - for ( int i = 0; i < assemblers.size(); i++ ) { - final DomainResultAssembler assembler = assemblers.get( i ); + for ( int i = 0; i < assemblers.length; i++ ) { + final DomainResultAssembler assembler = assemblers[i]; final Object contributorValue = assembler.assemble( processingState, processingState.getJdbcValuesSourceProcessingState().getProcessingOptions() diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/AggregateEmbeddableFetchInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/AggregateEmbeddableFetchInitializer.java index 919da16859..4d6e9bbe35 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/AggregateEmbeddableFetchInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/AggregateEmbeddableFetchInitializer.java @@ -9,6 +9,7 @@ package org.hibernate.sql.results.graph.embeddable.internal; import org.hibernate.metamodel.mapping.EmbeddableMappingType; import org.hibernate.sql.ast.spi.SqlSelection; import org.hibernate.sql.results.graph.AssemblerCreationState; +import org.hibernate.sql.results.graph.DomainResultAssembler; import org.hibernate.sql.results.graph.FetchParentAccess; import org.hibernate.sql.results.graph.embeddable.AbstractEmbeddableInitializer; import org.hibernate.sql.results.graph.embeddable.EmbeddableResultGraphNode; @@ -33,15 +34,22 @@ public class AggregateEmbeddableFetchInitializer extends AbstractEmbeddableIniti fetchParentAccess, structSelection ); - super.initializeAssemblers( resultDescriptor, creationState, resultDescriptor.getReferencedMappingType() ); + final DomainResultAssembler[] assemblers = super.createAssemblers( + resultDescriptor, + creationState, + resultDescriptor.getReferencedMappingType() + ); + System.arraycopy( assemblers, 0, this.assemblers, 0, assemblers.length ); } @Override - protected void initializeAssemblers( + protected DomainResultAssembler[] createAssemblers( EmbeddableResultGraphNode resultDescriptor, AssemblerCreationState creationState, EmbeddableMappingType embeddableTypeDescriptor) { - // No-op as we need to initialize assemblers in the constructor after the aggregateValuesArrayPositions is set + // Return just the assemblers array here without elements, + // as we initialize the array in the constructor after the aggregateValuesArrayPositions is set + return new DomainResultAssembler[embeddableTypeDescriptor.getNumberOfFetchables()]; } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/AggregateEmbeddableResultInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/AggregateEmbeddableResultInitializer.java index 5134e3860b..1cd15a1aac 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/AggregateEmbeddableResultInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/AggregateEmbeddableResultInitializer.java @@ -9,6 +9,7 @@ package org.hibernate.sql.results.graph.embeddable.internal; import org.hibernate.metamodel.mapping.EmbeddableMappingType; import org.hibernate.sql.ast.spi.SqlSelection; import org.hibernate.sql.results.graph.AssemblerCreationState; +import org.hibernate.sql.results.graph.DomainResultAssembler; import org.hibernate.sql.results.graph.FetchParentAccess; import org.hibernate.sql.results.graph.embeddable.AbstractEmbeddableInitializer; import org.hibernate.sql.results.graph.embeddable.EmbeddableResultGraphNode; @@ -33,15 +34,22 @@ public class AggregateEmbeddableResultInitializer extends AbstractEmbeddableInit fetchParentAccess, structSelection ); - super.initializeAssemblers( resultDescriptor, creationState, resultDescriptor.getReferencedMappingType() ); + final DomainResultAssembler[] assemblers = super.createAssemblers( + resultDescriptor, + creationState, + resultDescriptor.getReferencedMappingType() + ); + System.arraycopy( assemblers, 0, this.assemblers, 0, assemblers.length ); } @Override - protected void initializeAssemblers( + protected DomainResultAssembler[] createAssemblers( EmbeddableResultGraphNode resultDescriptor, AssemblerCreationState creationState, EmbeddableMappingType embeddableTypeDescriptor) { - // No-op as we need to initialize assemblers in the constructor after the aggregateValuesArrayPositions is set + // Return just the assemblers array here without elements, + // as we initialize the array in the constructor after the aggregateValuesArrayPositions is set + return new DomainResultAssembler[embeddableTypeDescriptor.getNumberOfFetchables()]; } @Override