HHH-16370 Using MapKey on ManyToMany leads to wrong insert SQL

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

@@ -507,7 +507,7 @@ public class MapBinder extends CollectionBinder {
 
 	private static void addSelectable(SimpleValue targetValue, Selectable selectable) {
 		if ( selectable instanceof Column ) {
-			targetValue.addColumn( ( (Column) selectable).clone() );
+			targetValue.addColumn( ( (Column) selectable).clone(), false, false  );
 		}
 		else if ( selectable instanceof Formula ) {
 			targetValue.addFormula( new Formula( ( (Formula) selectable).getFormula() ) );

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ManyToManyCollectionPart.java

@@ -21,6 +21,7 @@ import org.hibernate.mapping.Map;
 import org.hibernate.mapping.SimpleValue;
 import org.hibernate.mapping.Value;
 import org.hibernate.metamodel.mapping.AssociationKey;
+import org.hibernate.metamodel.mapping.AttributeMapping;
 import org.hibernate.metamodel.mapping.BasicEntityIdentifierMapping;
 import org.hibernate.metamodel.mapping.BasicValuedModelPart;
 import org.hibernate.metamodel.mapping.CompositeIdentifierMapping;
@@ -29,6 +30,7 @@ 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.ManagedMappingType;
 import org.hibernate.metamodel.mapping.ModelPart;
 import org.hibernate.metamodel.mapping.ModelPartContainer;
 import org.hibernate.metamodel.mapping.PluralAttributeMapping;
@@ -79,6 +81,8 @@ public class ManyToManyCollectionPart extends AbstractEntityCollectionPart imple
 		LazyTableGroup.ParentTableGroupUseChecker {
 	private ForeignKeyDescriptor foreignKey;
 	private ValuedModelPart fkTargetModelPart;
+	private boolean[] isInsertable;
+	private boolean[] isUpdatable;
 
 	public ManyToManyCollectionPart(
 			Nature nature,
@@ -122,9 +126,6 @@ public class ManyToManyCollectionPart extends AbstractEntityCollectionPart imple
 		// This is not possible for one-to-many associations because we need to create the target table group eagerly,
 		// to preserve the cardinality. Also, the OneToManyTableGroup has no reference to the parent table group
 		if ( getTargetKeyPropertyNames().contains( name ) ) {
-			if ( fkTargetModelPart instanceof ToOneAttributeMapping ) {
-				return ( (ToOneAttributeMapping) fkTargetModelPart ).findSubPart( name, targetType );
-			}
 			final ModelPart keyPart = foreignKey.getKeyPart();
 			if ( keyPart instanceof EmbeddableValuedModelPart && keyPart instanceof VirtualModelPart ) {
 				return ( (ModelPartContainer) keyPart ).findSubPart( name, targetType );
@@ -161,6 +162,32 @@ public class ManyToManyCollectionPart extends AbstractEntityCollectionPart imple
 		return getJdbcTypeCount();
 	}
 
+	@Override
+	public void forEachInsertable(SelectableConsumer consumer) {
+		forEachSelectable(
+				(selectionIndex, selectableMapping) -> {
+					if ( !foreignKey.getKeyPart().getSelectable( selectionIndex ).isInsertable() ) {
+						return;
+					}
+
+					consumer.accept( selectionIndex, selectableMapping );
+				}
+		);
+	}
+
+	@Override
+	public void forEachUpdatable(SelectableConsumer consumer) {
+		forEachSelectable(
+				(selectionIndex, selectableMapping) -> {
+					if ( !foreignKey.getKeyPart().getSelectable( selectionIndex ).isUpdateable() ) {
+						return;
+					}
+
+					consumer.accept( selectionIndex, selectableMapping );
+				}
+		);
+	}
+
 	@Override
 	public <X, Y> int decompose(
 			Object domainValue,
@@ -216,6 +243,9 @@ public class ManyToManyCollectionPart extends AbstractEntityCollectionPart imple
 
 	@Override
 	public ModelPart getKeyTargetMatchPart() {
+		if ( fkTargetModelPart instanceof ToOneAttributeMapping ) {
+			return foreignKey.getKeyPart();
+		}
 		return fkTargetModelPart;
 	}
 
@@ -418,8 +448,9 @@ public class ManyToManyCollectionPart extends AbstractEntityCollectionPart imple
 		}
 
 		if ( getNature() == Nature.ELEMENT ) {
+			final Value element = bootCollectionDescriptor.getElement();
 			foreignKey = createForeignKeyDescriptor(
-					bootCollectionDescriptor.getElement(),
+					element,
 					(EntityType) collectionDescriptor.getElementType(),
 					fkTargetModelPart,
 					creationProcess,
@@ -428,8 +459,9 @@ public class ManyToManyCollectionPart extends AbstractEntityCollectionPart imple
 		}
 		else {
 			assert bootCollectionDescriptor.isIndexed();
+			final Value index = ( (IndexedCollection) bootCollectionDescriptor ).getIndex();
 			foreignKey = createForeignKeyDescriptor(
-					( (IndexedCollection) bootCollectionDescriptor ).getIndex(),
+					index,
 					(EntityType) collectionDescriptor.getIndexType(),
 					fkTargetModelPart,
 					creationProcess,
@@ -565,7 +597,11 @@ public class ManyToManyCollectionPart extends AbstractEntityCollectionPart imple
 			if ( toOneAttributeMapping.getForeignKeyDescriptor() == null ) {
 				throw new IllegalStateException( "Not yet ready: " + toOneAttributeMapping );
 			}
-			return toOneAttributeMapping.getForeignKeyDescriptor();
+			return determineForeignKey(
+					toOneAttributeMapping.getForeignKeyDescriptor(),
+					fkBootDescriptorSource,
+					creationProcess
+			);
 		}
 
 		if ( fkTargetModelPart instanceof ManyToManyCollectionPart ) {
@@ -574,7 +610,11 @@ public class ManyToManyCollectionPart extends AbstractEntityCollectionPart imple
 			if ( targetModelPart.getForeignKeyDescriptor() == null ) {
 				throw new IllegalStateException( "Not yet ready: " + targetModelPart );
 			}
-			return targetModelPart.getForeignKeyDescriptor();
+			return determineForeignKey(
+					targetModelPart.getForeignKeyDescriptor(),
+					fkBootDescriptorSource,
+					creationProcess
+			);
 		}
 
 		final String collectionTableName = ( (CollectionMutationTarget) getCollectionDescriptor() ).getCollectionTableMapping().getTableName();
@@ -609,6 +649,46 @@ public class ManyToManyCollectionPart extends AbstractEntityCollectionPart imple
 		);
 	}
 
+	private ForeignKeyDescriptor determineForeignKey(
+			ForeignKeyDescriptor foreignKeyDescriptor,
+			Value fkBootDescriptorSource,
+			MappingModelCreationProcess creationProcess) {
+		final int selectableCount = foreignKeyDescriptor.getJdbcTypeCount();
+		final ValuedModelPart keyPart = foreignKeyDescriptor.getKeyPart();
+		for ( int i = 0; i < selectableCount; i++ ) {
+			if ( keyPart.getSelectable( i ).isInsertable() != fkBootDescriptorSource.isColumnInsertable( i )
+					|| keyPart.getSelectable( i ).isUpdateable() != fkBootDescriptorSource.isColumnUpdateable( i ) ) {
+				final AttributeMapping attributeMapping = keyPart.asAttributeMapping();
+				final ManagedMappingType declaringType;
+				if ( attributeMapping == null ) {
+					declaringType = null;
+				}
+				else {
+					declaringType = attributeMapping.getDeclaringType();
+				}
+				final SelectableMappings selectableMappings = SelectableMappingsImpl.from(
+						keyPart.getContainingTableExpression(),
+						fkBootDescriptorSource,
+						getPropertyOrder( fkBootDescriptorSource, creationProcess ),
+						creationProcess.getCreationContext().getMetadata(),
+						creationProcess.getCreationContext().getTypeConfiguration(),
+						fkBootDescriptorSource.getColumnInsertability(),
+						fkBootDescriptorSource.getColumnUpdateability(),
+						creationProcess.getCreationContext().getDialect(),
+						creationProcess.getSqmFunctionRegistry()
+				);
+				return foreignKeyDescriptor.withKeySelectionMapping(
+						declaringType,
+						this,
+						selectableMappings::getSelectable,
+						creationProcess
+				);
+			}
+		}
+
+		return foreignKeyDescriptor;
+	}
+
 	private SimpleForeignKeyDescriptor createSimpleForeignKeyDescriptor(
 			Value fkBootDescriptorSource,
 			EntityType entityType,
@@ -618,7 +698,7 @@ public class ManyToManyCollectionPart extends AbstractEntityCollectionPart imple
 			BasicValuedModelPart basicFkTargetPart) {
 		final boolean columnInsertable;
 		final boolean columnUpdateable;
-		if ( getNature() == Nature.ELEMENT ) {
+		if ( getNature() == Nature.ELEMENT && !fkBootDescriptorSource.getSelectables().get( 0 ).isFormula() ) {
 			// Replicate behavior of AbstractCollectionPersister#elementColumnIsSettable
 			columnInsertable = true;
 			columnUpdateable = true;
@@ -627,34 +707,26 @@ public class ManyToManyCollectionPart extends AbstractEntityCollectionPart imple
 			columnInsertable = fkBootDescriptorSource.isColumnInsertable( 0 );
 			columnUpdateable = fkBootDescriptorSource.isColumnUpdateable( 0 );
 		}
+		final SimpleValue fkValue = (SimpleValue) fkBootDescriptorSource;
 		final SelectableMapping keySelectableMapping = SelectableMappingImpl.from(
 				fkKeyTableName,
-				fkBootDescriptorSource.getSelectables().get(0),
+				fkBootDescriptorSource.getSelectables().get( 0 ),
 				basicFkTargetPart.getJdbcMapping(),
 				creationProcess.getCreationContext().getTypeConfiguration(),
 				columnInsertable,
 				columnUpdateable,
-				((SimpleValue) fkBootDescriptorSource).isPartitionKey(),
+				fkValue.isPartitionKey(),
 				dialect,
 				creationProcess.getSqmFunctionRegistry()
 		);
 
-		final boolean hasConstraint;
-		if ( fkBootDescriptorSource instanceof SimpleValue ) {
-			hasConstraint = ( (SimpleValue) fkBootDescriptorSource ).isConstrained();
-		}
-		else {
-			// We assume there is a constraint if the key is not nullable
-			hasConstraint = !fkBootDescriptorSource.isNullable();
-		}
-
 		// here we build a ModelPart that represents the many-to-many table key referring to the element table
 		return new SimpleForeignKeyDescriptor(
 				getAssociatedEntityMappingType(),
 				keySelectableMapping,
 				basicFkTargetPart,
 				entityType.isReferenceToPrimaryKey(),
-				hasConstraint
+				fkValue.isConstrained()
 		);
 	}
 }

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/SimpleForeignKeyDescriptor.java

@@ -209,14 +209,15 @@ public class SimpleForeignKeyDescriptor implements ForeignKeyDescriptor, BasicVa
 			TableGroupProducer declaringTableGroupProducer,
 			IntFunction<SelectableMapping> selectableMappingAccess,
 			MappingModelCreationProcess creationProcess) {
+		final SelectableMapping selectableMapping = selectableMappingAccess.apply( 0 );
 		return new SimpleForeignKeyDescriptor(
 				declaringType,
 				keySide.getModelPart(),
 				( (PropertyBasedMapping) keySide.getModelPart() ).getPropertyAccess(),
-				selectableMappingAccess.apply( 0 ),
+				selectableMapping,
 				targetSide.getModelPart(),
-				keySide.getModelPart().isInsertable(),
+				selectableMapping.isInsertable(),
-				keySide.getModelPart().isUpdateable(),
+				selectableMapping.isUpdateable(),
 				refersToPrimaryKey,
 				hasConstraint
 		);

hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EntityValuedPathInterpretation.java

@@ -12,7 +12,6 @@ import java.util.List;
 import java.util.function.Consumer;
 
 import org.hibernate.metamodel.mapping.BasicValuedModelPart;
-import org.hibernate.metamodel.mapping.EmbeddableValuedModelPart;
 import org.hibernate.metamodel.mapping.EntityAssociationMapping;
 import org.hibernate.metamodel.mapping.EntityDiscriminatorMapping;
 import org.hibernate.metamodel.mapping.EntityIdentifierMapping;
@@ -37,16 +36,12 @@ import org.hibernate.query.sqm.tree.select.SqmSelection;
 import org.hibernate.spi.NavigablePath;
 import org.hibernate.sql.ast.Clause;
 import org.hibernate.sql.ast.SqlAstWalker;
-import org.hibernate.sql.ast.spi.FromClauseAccess;
 import org.hibernate.sql.ast.spi.SqlAstCreationState;
-import org.hibernate.sql.ast.spi.SqlAstProcessingState;
-import org.hibernate.sql.ast.spi.SqlAstQueryPartProcessingState;
 import org.hibernate.sql.ast.spi.SqlExpressionResolver;
 import org.hibernate.sql.ast.tree.expression.ColumnReference;
 import org.hibernate.sql.ast.tree.expression.Expression;
 import org.hibernate.sql.ast.tree.expression.SqlTuple;
 import org.hibernate.sql.ast.tree.expression.SqlTupleContainer;
-import org.hibernate.sql.ast.tree.from.CorrelatedTableGroup;
 import org.hibernate.sql.ast.tree.from.TableGroup;
 import org.hibernate.sql.ast.tree.from.TableReference;
 import org.hibernate.sql.ast.tree.update.Assignable;
@@ -163,14 +158,7 @@ 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 associationKeyTargetMatchPart = associationMapping.getKeyTargetMatchPart();
+			final ModelPart keyTargetMatchPart = associationMapping.getKeyTargetMatchPart();
-			final ModelPart keyTargetMatchPart;
-			if ( associationKeyTargetMatchPart instanceof ToOneAttributeMapping ) {
-				keyTargetMatchPart = ( (ToOneAttributeMapping) associationKeyTargetMatchPart ).getKeyTargetMatchPart();
-			}
-			else {
-				keyTargetMatchPart = associationKeyTargetMatchPart;
-			}
 
 			if ( associationMapping.isFkOptimizationAllowed() ) {
 				final boolean forceUsingForeignKeyAssociationSidePart;
@@ -197,13 +185,8 @@ public class EntityValuedPathInterpretation<T> extends AbstractSqmPathInterpreta
 					forceUsingForeignKeyAssociationSidePart = false;
 				}
 				if ( forceUsingForeignKeyAssociationSidePart ) {
-					if ( associationKeyTargetMatchPart instanceof ToOneAttributeMapping ) {
-						resultModelPart = keyTargetMatchPart;
-					}
-					else {
 					resultModelPart = associationMapping.getForeignKeyDescriptor()
 							.getPart( associationMapping.getSideNature() );
-					}
 					resultTableGroup = sqlAstCreationState.getFromClauseAccess()
 							.findTableGroup( tableGroup.getNavigablePath().getParent() );
 				}