HHH-17334 Simplify indexed element collection update and assert element column mutability

hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionBinder.java

@@ -2179,6 +2179,7 @@ public abstract class CollectionBinder {
 		}
 
 		checkFilterConditions( collection );
+		checkConsistentColumnMutability( collection );
 	}
 
 	private void handleElementCollection(XClass elementType, String hqlOrderBy) {
@@ -2666,6 +2667,34 @@ public abstract class CollectionBinder {
 		}
 	}
 
+	private static void checkConsistentColumnMutability(Collection collection) {
+		checkConsistentColumnMutability( collection.getRole(), collection.getKey() );
+		checkConsistentColumnMutability( collection.getRole(), collection.getElement() );
+	}
+
+	private static void checkConsistentColumnMutability(String collectionRole, Value value) {
+		Boolean readOnly = null;
+		for ( int i = 0; i < value.getColumnSpan(); i++ ) {
+			final boolean insertable = value.isColumnInsertable( i );
+			if ( insertable != value.isColumnUpdateable( i ) ) {
+				throw new AnnotationException(
+						"Join column '" + value.getColumns().get( i ).getName() + "' on collection property '"
+								+ collectionRole + "' must be defined with the same insertable and updatable attributes"
+				);
+			}
+			if ( readOnly == null ) {
+				readOnly = insertable;
+			}
+			else if ( readOnly != insertable && !value.getColumns().get( i ).isFormula() ) {
+				// We also assert that all join columns have the same mutability (except formulas)
+				throw new AnnotationException(
+						"All join columns on collection '" + collectionRole + "' should have" +
+								" the same insertable and updatable setting"
+				);
+			}
+		}
+	}
+
 	private void bindCollectionSecondPass(PersistentClass targetEntity, AnnotatedJoinColumns joinColumns) {
 
 		if ( !isUnownedCollection() ) {

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

@@ -491,32 +491,8 @@ public class BasicCollectionPersister extends AbstractCollectionPersister {
 		// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 		// SET
 
-		if ( hasIndex() ) {
-			/*
-				Given :
+		attribute.getElementDescriptor().forEachUpdatable( updateBuilder );
 
-				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
 
@@ -552,28 +528,16 @@ public class BasicCollectionPersister extends AbstractCollectionPersister {
 				0,
 				jdbcValueBindings,
 				null,
-				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
-							);
-						},
+				(valueIndex, bindings, y, jdbcValue, jdbcValueMapping) -> {
+					if ( !jdbcValueMapping.isUpdateable() || jdbcValueMapping.isFormula() ) {
+						return;
+					}
+					bindings.bindValue(
+							jdbcValue,
+							jdbcValueMapping,
+							ParameterUsage.SET
+					);
+				},
 				session
 		);
 	}

hibernate-core/src/test/java/org/hibernate/orm/test/embeddable/EmbeddableAsElementCollectionWithUpdatableFalseTest.java

@@ -8,6 +8,7 @@ import org.hibernate.testing.orm.junit.JiraKey;
 import org.hibernate.testing.orm.junit.SessionFactory;
 import org.hibernate.testing.orm.junit.SessionFactoryScope;
 import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import jakarta.persistence.Column;
@@ -26,6 +27,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 })
 @SessionFactory
 @JiraKey("HHH-16573")
+@Disabled("We now assert that collection columns have the same insertable / updatable attributes, see HHH-17334")
 public class EmbeddableAsElementCollectionWithUpdatableFalseTest {
 
 	@AfterEach

hibernate-core/src/test/java/org/hibernate/orm/test/embeddable/EmbeddableAsOrderedElementCollectionWithUpdatableFalseTest.java

@@ -8,6 +8,7 @@ import org.hibernate.testing.orm.junit.JiraKey;
 import org.hibernate.testing.orm.junit.SessionFactory;
 import org.hibernate.testing.orm.junit.SessionFactoryScope;
 import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import jakarta.persistence.Column;
@@ -27,6 +28,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 })
 @SessionFactory
 @JiraKey("HHH-16573")
+@Disabled("We now assert that collection columns have the same insertable / updatable attributes, see HHH-17334")
 public class EmbeddableAsOrderedElementCollectionWithUpdatableFalseTest {
 
 	@AfterEach