HHH-16573 NPE with embeddable element collection with updateable = false

Author: dreab8

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ValuedModelPart.java

@@ -64,6 +64,18 @@ default void forEachInsertable(SelectableConsumer consumer) {
 		);
 	}
 
+	default void forEachNonFormula(SelectableConsumer consumer) {
+		ModelPart.super.forEachSelectable(
+				(selectionIndex, selectableMapping) -> {
+					if ( selectableMapping.isFormula() ) {
+						return;
+					}
+
+					consumer.accept( selectionIndex, selectableMapping );
+				}
+		);
+	}
+
 	default void forEachUpdatable(SelectableConsumer consumer) {
 		ModelPart.super.forEachSelectable(
 				(selectionIndex, selectableMapping) -> {

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

@@ -464,8 +464,32 @@ private RestrictedTableMutation<JdbcMutationOperation> generateUpdateRowAst(Muta
 		// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 		// SET
 
-		attribute.getElementDescriptor().forEachUpdatable( updateBuilder );
-
+		if ( hasIndex() ) {
+			/*
+				Given :
+
+				class MyEntity {
+					@ElementCollection(fetch = FetchType.LAZY)
+					@OrderColumn
+					private List<MyEmbeddable> myEmbeddables;
+				}
+
+				@Embeddable
+				public static class MyEmbeddable {
+					@Column(updatable = false)
+					private String embeddedProperty;
+				}
+
+			 	we cannot understand if the update is due to a change in the value embeddedProperty or because a
+			 	new element has been added to the list in an existing position (update) shifting the old value (insert).
+
+			 	For this reason we cannot take into consideration the `@Column(updatable = false)`
+			 */
+			attribute.getElementDescriptor().forEachNonFormula( updateBuilder );
+		}
+		else {
+			attribute.getElementDescriptor().forEachUpdatable( updateBuilder );
+		}
 		// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 		// WHERE
 
@@ -501,16 +525,28 @@ private void applyUpdateRowValues(
 				0,
 				jdbcValueBindings,
 				null,
-				(valueIndex, bindings, y, jdbcValue, jdbcValueMapping) -> {
-					if ( !jdbcValueMapping.isUpdateable() || jdbcValueMapping.isFormula() ) {
-						return;
-					}
-					bindings.bindValue(
-							jdbcValue,
-							jdbcValueMapping,
-							ParameterUsage.SET
-					);
-				},
+				hasIndex() ?
+						(valueIndex, bindings, y, jdbcValue, jdbcValueMapping) -> {
+							if ( jdbcValueMapping.isFormula() ) {
+								return;
+							}
+							bindings.bindValue(
+									jdbcValue,
+									jdbcValueMapping,
+									ParameterUsage.SET
+							);
+						}
+						:
+						(valueIndex, bindings, y, jdbcValue, jdbcValueMapping) -> {
+							if ( !jdbcValueMapping.isUpdateable() || jdbcValueMapping.isFormula() ) {
+								return;
+							}
+							bindings.bindValue(
+									jdbcValue,
+									jdbcValueMapping,
+									ParameterUsage.SET
+							);
+						},
 				session
 		);
 	}

hibernate-core/src/main/java/org/hibernate/persister/collection/mutation/UpdateRowsCoordinatorStandard.java

@@ -19,7 +19,10 @@
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.metamodel.mapping.PluralAttributeMapping;
 import org.hibernate.sql.model.MutationType;
+import org.hibernate.sql.model.internal.AbstractMutationOperationGroup;
+import org.hibernate.sql.model.internal.MutationOperationGroupNone;
 import org.hibernate.sql.model.internal.MutationOperationGroupSingle;
+import org.hibernate.sql.model.jdbc.JdbcMutationOperation;
 
 /**
  * UpdateRowsCoordinator implementation for cases with a separate collection table
@@ -31,7 +34,7 @@
 public class UpdateRowsCoordinatorStandard extends AbstractUpdateRowsCoordinator implements UpdateRowsCoordinator {
 	private final RowMutationOperations rowMutationOperations;
 
-	private MutationOperationGroupSingle operationGroup;
+	private AbstractMutationOperationGroup operationGroup;
 
 	public UpdateRowsCoordinatorStandard(
 			CollectionMutationTarget mutationTarget,
@@ -43,7 +46,7 @@ public UpdateRowsCoordinatorStandard(
 
 	@Override
 	protected int doUpdate(Object key, PersistentCollection<?> collection, SharedSessionContractImplementor session) {
-		final MutationOperationGroupSingle operationGroup = getOperationGroup();
+		final AbstractMutationOperationGroup operationGroup = getOperationGroup();
 
 		final MutationExecutorService mutationExecutorService = session
 				.getFactory()
@@ -115,42 +118,55 @@ private boolean processRow(
 			int entryPosition,
 			MutationExecutor mutationExecutor,
 			SharedSessionContractImplementor session) {
-		final PluralAttributeMapping attribute = getMutationTarget().getTargetPart();
-		if ( !collection.needsUpdating( entry, entryPosition, attribute ) ) {
-			return false;
-		}
+		if ( rowMutationOperations.getUpdateRowOperation() != null ) {
+			final PluralAttributeMapping attribute = getMutationTarget().getTargetPart();
+			if ( !collection.needsUpdating( entry, entryPosition, attribute ) ) {
+				return false;
+			}
 
-		rowMutationOperations.getUpdateRowValues().applyValues(
-				collection,
-				key,
-				entry,
-				entryPosition,
-				session,
-				mutationExecutor.getJdbcValueBindings()
-		);
+			rowMutationOperations.getUpdateRowValues().applyValues(
+					collection,
+					key,
+					entry,
+					entryPosition,
+					session,
+					mutationExecutor.getJdbcValueBindings()
+			);
 
-		rowMutationOperations.getUpdateRowRestrictions().applyRestrictions(
-				collection,
-				key,
-				entry,
-				entryPosition,
-				session,
-				mutationExecutor.getJdbcValueBindings()
-		);
+			rowMutationOperations.getUpdateRowRestrictions().applyRestrictions(
+					collection,
+					key,
+					entry,
+					entryPosition,
+					session,
+					mutationExecutor.getJdbcValueBindings()
+			);
 
-		mutationExecutor.execute( collection, null, null, null, session );
-		return true;
+			mutationExecutor.execute( collection, null, null, null, session );
+			return true;
+		}
+		else {
+			return false;
+		}
 	}
 
-	protected MutationOperationGroupSingle getOperationGroup() {
+	protected AbstractMutationOperationGroup getOperationGroup() {
 		if ( operationGroup == null ) {
-			operationGroup = new MutationOperationGroupSingle(
-					MutationType.UPDATE,
-					getMutationTarget(),
-					rowMutationOperations.getUpdateRowOperation()
-			);
+			final JdbcMutationOperation updateRowOperation = rowMutationOperations.getUpdateRowOperation();
+			if ( updateRowOperation == null ) {
+				operationGroup = new MutationOperationGroupNone(
+						MutationType.UPDATE,
+						getMutationTarget()
+				);
+			}
+			else {
+				operationGroup = new MutationOperationGroupSingle(
+						MutationType.UPDATE,
+						getMutationTarget(),
+						updateRowOperation
+				);
+			}
 		}
-
 		return operationGroup;
 	}