cleanups to OneToManyPersister

Signed-off-by: Gavin King <[email protected]>

Author: gavinking

hibernate-core/src/main/java/org/hibernate/persister/collection/OneToManyPersister.java

@@ -51,8 +51,9 @@
 import org.hibernate.persister.collection.mutation.UpdateRowsCoordinatorNoOp;
 import org.hibernate.persister.collection.mutation.UpdateRowsCoordinatorOneToMany;
 import org.hibernate.persister.collection.mutation.UpdateRowsCoordinatorTablePerSubclass;
+import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.persister.entity.UnionSubclassEntityPersister;
-import org.hibernate.sql.ast.SqlAstTranslator;
+import org.hibernate.service.spi.ServiceRegistryImplementor;
 import org.hibernate.sql.ast.spi.SqlAstCreationState;
 import org.hibernate.sql.ast.tree.expression.ColumnReference;
 import org.hibernate.sql.ast.tree.from.TableGroup;
@@ -79,8 +80,8 @@
 import static org.hibernate.sql.model.ast.builder.TableUpdateBuilder.NULL;
 
 /**
- * A {@link CollectionPersister} for {@link jakarta.persistence.OneToMany one-to-one
- * associations}.
+ * A {@link CollectionPersister} for {@linkplain jakarta.persistence.OneToMany
+ * one-to-many associations}.
  *
  * @see BasicCollectionPersister
  *
@@ -407,101 +408,81 @@ private RowMutationOperations buildRowMutationOperations() {
 	private InsertRowsCoordinator buildInsertCoordinator() {
 		if ( isInverse() || !isRowInsertEnabled() ) {
 			if ( MODEL_MUTATION_LOGGER.isDebugEnabled() ) {
-				MODEL_MUTATION_LOGGER.debugf(
-						"Skipping collection (re)creation - %s",
-						getRolePath()
-				);
+				MODEL_MUTATION_LOGGER.debugf( "Skipping collection (re)creation - %s", getRolePath() );
 			}
 			return new InsertRowsCoordinatorNoOp( this );
 		}
-
-		if ( getElementPersisterInternal() != null && getElementPersisterInternal().hasSubclasses()
-				&& getElementPersisterInternal() instanceof UnionSubclassEntityPersister ) {
-			return new InsertRowsCoordinatorTablePerSubclass( this, rowMutationOperations, getFactory().getServiceRegistry() );
+		else {
+			final ServiceRegistryImplementor serviceRegistry = getFactory().getServiceRegistry();
+			final EntityPersister elementPersister = getElementPersisterInternal();
+			return elementPersister != null && elementPersister.hasSubclasses()
+						&& elementPersister instanceof UnionSubclassEntityPersister
+					? new InsertRowsCoordinatorTablePerSubclass( this, rowMutationOperations, serviceRegistry )
+					: new InsertRowsCoordinatorStandard( this, rowMutationOperations, serviceRegistry );
 		}
-		return new InsertRowsCoordinatorStandard( this, rowMutationOperations, getFactory().getServiceRegistry() );
 	}
 
 	private UpdateRowsCoordinator buildUpdateCoordinator() {
 		if ( !isRowDeleteEnabled() && !isRowInsertEnabled() ) {
 			if ( MODEL_MUTATION_LOGGER.isDebugEnabled() ) {
-				MODEL_MUTATION_LOGGER.debugf(
-						"Skipping collection row updates - %s",
-						getRolePath()
-				);
+				MODEL_MUTATION_LOGGER.debugf( "Skipping collection row updates - %s", getRolePath() );
 			}
 			return new UpdateRowsCoordinatorNoOp( this );
 		}
-
-		if ( getElementPersisterInternal() != null && getElementPersisterInternal().hasSubclasses()
-				&& getElementPersisterInternal() instanceof UnionSubclassEntityPersister ) {
-			return new UpdateRowsCoordinatorTablePerSubclass( this, rowMutationOperations, getFactory() );
+		else {
+			final EntityPersister elementPersister = getElementPersisterInternal();
+			return elementPersister != null && elementPersister.hasSubclasses()
+						&& elementPersister instanceof UnionSubclassEntityPersister
+					? new UpdateRowsCoordinatorTablePerSubclass( this, rowMutationOperations, getFactory() )
+					: new UpdateRowsCoordinatorOneToMany( this, rowMutationOperations, getFactory() );
 		}
-		return new UpdateRowsCoordinatorOneToMany( this, getRowMutationOperations(), getFactory() );
 	}
 
 	private DeleteRowsCoordinator buildDeleteCoordinator() {
 		if ( !needsRemove() ) {
 			if ( MODEL_MUTATION_LOGGER.isDebugEnabled() ) {
-				MODEL_MUTATION_LOGGER.debugf(
-						"Skipping collection row deletions - %s",
-						getRolePath()
-				);
+				MODEL_MUTATION_LOGGER.debugf( "Skipping collection row deletions - %s", getRolePath() );
 			}
 			return new DeleteRowsCoordinatorNoOp( this );
 		}
-
-
-		if ( getElementPersisterInternal() != null && getElementPersisterInternal().hasSubclasses()
-				&& getElementPersisterInternal() instanceof UnionSubclassEntityPersister ) {
-			return new DeleteRowsCoordinatorTablePerSubclass( this, rowMutationOperations, false, getFactory().getServiceRegistry() );
+		else {
+			final EntityPersister elementPersister = getElementPersisterInternal();
+			final ServiceRegistryImplementor serviceRegistry = getFactory().getServiceRegistry();
+			return elementPersister != null && elementPersister.hasSubclasses()
+						&& elementPersister instanceof UnionSubclassEntityPersister
+					// never delete by index for one-to-many
+					? new DeleteRowsCoordinatorTablePerSubclass( this, rowMutationOperations, false, serviceRegistry )
+					: new DeleteRowsCoordinatorStandard( this, rowMutationOperations, false, serviceRegistry );
 		}
-		return new DeleteRowsCoordinatorStandard(
-				this,
-				rowMutationOperations,
-				// never delete by index for one-to-many
-				false,
-				getFactory().getServiceRegistry()
-		);
 	}
 
 	private RemoveCoordinator buildDeleteAllCoordinator() {
 		if ( ! needsRemove() ) {
 			if ( MODEL_MUTATION_LOGGER.isDebugEnabled() ) {
-				MODEL_MUTATION_LOGGER.debugf(
-						"Skipping collection removals - %s",
-						getRolePath()
-				);
+				MODEL_MUTATION_LOGGER.debugf( "Skipping collection removals - %s", getRolePath() );
 			}
 			return new RemoveCoordinatorNoOp( this );
 		}
-
-		if ( getElementPersisterInternal() != null && getElementPersisterInternal().hasSubclasses()
-				&& getElementPersisterInternal() instanceof UnionSubclassEntityPersister ) {
-			return new RemoveCoordinatorTablePerSubclass( this, this::buildDeleteAllOperation, getFactory().getServiceRegistry() );
+		else {
+			final ServiceRegistryImplementor serviceRegistry = getFactory().getServiceRegistry();
+			final EntityPersister elementPersister = getElementPersisterInternal();
+			return elementPersister != null && elementPersister.hasSubclasses()
+						&& elementPersister instanceof UnionSubclassEntityPersister
+					? new RemoveCoordinatorTablePerSubclass( this, this::buildDeleteAllOperation, serviceRegistry )
+					: new RemoveCoordinatorStandard( this, this::buildDeleteAllOperation, serviceRegistry );
 		}
-		return new RemoveCoordinatorStandard( this, this::buildDeleteAllOperation, getFactory().getServiceRegistry() );
 	}
 
 	private JdbcMutationOperation generateDeleteRowOperation(MutatingTableReference tableReference) {
-		final RestrictedTableMutation<JdbcMutationOperation> sqlAst = generateDeleteRowAst( tableReference );
-
-		final SqlAstTranslator<JdbcMutationOperation> translator = getFactory().getJdbcServices()
-				.getDialect()
-				.getSqlAstTranslatorFactory()
-				.buildModelMutationTranslator( sqlAst, getFactory() );
-
-		return translator.translate( null, MutationQueryOptions.INSTANCE );
+		return getFactory().getJdbcServices().getDialect().getSqlAstTranslatorFactory()
+				.buildModelMutationTranslator( generateDeleteRowAst( tableReference ), getFactory() )
+				.translate( null, MutationQueryOptions.INSTANCE );
 	}
 
 	public RestrictedTableMutation<JdbcMutationOperation> generateDeleteRowAst(MutatingTableReference tableReference) {
 		// note that custom sql delete row details are handled by CollectionRowUpdateBuilder
-		final CollectionRowDeleteByUpdateSetNullBuilder<MutationOperation> updateBuilder = new CollectionRowDeleteByUpdateSetNullBuilder<>(
-				this,
-				tableReference,
-				getFactory(),
-				sqlWhereString
-		);
+		final CollectionRowDeleteByUpdateSetNullBuilder<MutationOperation> updateBuilder =
+				new CollectionRowDeleteByUpdateSetNullBuilder<>( this, tableReference, getFactory(), sqlWhereString );
 
 		// for each key column -
 		// 		1) set the value to null
@@ -510,42 +491,36 @@ public RestrictedTableMutation<JdbcMutationOperation> generateDeleteRowAst(Mutat
 		final int keyTypeCount = keyDescriptor.getJdbcTypeCount();
 		for ( int i = 0; i < keyTypeCount; i++ ) {
 			final SelectableMapping selectable = keyDescriptor.getSelectable( i );
-			if ( selectable.isFormula() ) {
-				continue;
-			}
-
-			if ( selectable.isUpdateable() ) {
-				// set null
-				updateBuilder.addValueColumn(
-						selectable.getSelectionExpression(),
-						NULL,
-						selectable.getJdbcMapping(),
-						selectable.isLob()
-				);
+			if ( !selectable.isFormula() ) {
+				if ( selectable.isUpdateable() ) {
+					// set null
+					updateBuilder.addValueColumn(
+							selectable.getSelectionExpression(),
+							NULL,
+							selectable.getJdbcMapping(),
+							selectable.isLob()
+					);
+				}
+				// restrict
+				updateBuilder.addKeyRestrictionLeniently( selectable );
 			}
-
-			// restrict
-			updateBuilder.addKeyRestrictionLeniently( selectable );
 		}
 
 		// set the value for each index column to null
 		if ( hasIndex() && !indexContainsFormula ) {
 			final CollectionPart indexDescriptor = getAttributeMapping().getIndexDescriptor();
 			assert indexDescriptor != null;
-
 			final int indexTypeCount = indexDescriptor.getJdbcTypeCount();
 			for ( int i = 0; i < indexTypeCount; i++ ) {
 				final SelectableMapping selectable = indexDescriptor.getSelectable( i );
-				if ( !selectable.isUpdateable() ) {
-					continue;
+				if ( selectable.isUpdateable() ) {
+					updateBuilder.addValueColumn(
+							selectable.getSelectionExpression(),
+							NULL,
+							selectable.getJdbcMapping(),
+							selectable.isLob()
+					);
 				}
-
-				updateBuilder.addValueColumn(
-						selectable.getSelectionExpression(),
-						NULL,
-						selectable.getJdbcMapping(),
-						selectable.isLob()
-				);
 			}
 		}
 
@@ -593,12 +568,8 @@ private JdbcMutationOperation generateInsertRowOperation(MutatingTableReference
 	}
 
 	private TableUpdate<JdbcMutationOperation> buildTableUpdate(MutatingTableReference tableReference) {
-		final TableUpdateBuilderStandard<JdbcMutationOperation> updateBuilder = new TableUpdateBuilderStandard<>(
-				this,
-				tableReference,
-				getFactory(),
-				sqlWhereString
-		);
+		final TableUpdateBuilderStandard<JdbcMutationOperation> updateBuilder =
+				new TableUpdateBuilderStandard<>( this, tableReference, getFactory(), sqlWhereString );
 
 		final PluralAttributeMapping attributeMapping = getAttributeMapping();
 		attributeMapping.getKeyDescriptor().getKeyPart().forEachSelectable( updateBuilder );
@@ -667,12 +638,8 @@ private void applyInsertRowValues(
 
 	private JdbcMutationOperation generateWriteIndexOperation(MutatingTableReference tableReference) {
 		// note that custom sql update details are handled by TableUpdateBuilderStandard
-		final TableUpdateBuilderStandard<JdbcMutationOperation> updateBuilder = new TableUpdateBuilderStandard<>(
-				this,
-				tableReference,
-				getFactory(),
-				sqlWhereString
-		);
+		final TableUpdateBuilderStandard<JdbcMutationOperation> updateBuilder =
+				new TableUpdateBuilderStandard<>( this, tableReference, getFactory(), sqlWhereString );
 
 		final OneToManyCollectionPart elementDescriptor = (OneToManyCollectionPart) getAttributeMapping().getElementDescriptor();
 		updateBuilder.addKeyRestrictionsLeniently( elementDescriptor.getAssociatedEntityMappingType().getIdentifierMapping() );
@@ -698,10 +665,8 @@ private void applyWriteIndexValues(
 			int entryPosition,
 			SharedSessionContractImplementor session,
 			JdbcValueBindings jdbcValueBindings) {
-		final Object index = collection.getIndex( entry, entryPosition, this );
-
 		getAttributeMapping().getIndexDescriptor().decompose(
-				index,
+				collection.getIndex( entry, entryPosition, this ),
 				0,
 				jdbcValueBindings,
 				null,