HHH-17132 Cleanse non-explicit unique keys with matching columns

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

@@ -170,6 +170,7 @@ class IndexBinder {
 		if ( unique && !hasFormula ) {
 			final String keyName = getImplicitNamingStrategy().determineUniqueKeyName( source ).render( getDialect() );
 			final UniqueKey uniqueKey = table.getOrCreateUniqueKey( keyName );
+			uniqueKey.setExplicit( true );
 			uniqueKey.setNameExplicit( nameExplicit );
 			for ( int i = 0; i < columns.length; i++ ) {
 				uniqueKey.addColumn( (Column) columns[i], orderings != null ? orderings[i] : null );

hibernate-core/src/main/java/org/hibernate/dialect/unique/CreateTableUniqueDelegate.java

@@ -63,7 +63,7 @@ public class CreateTableUniqueDelegate extends AlterTableUniqueDelegate {
 				// named unique constraint, then the name gets lost. Unfortunately the
 				// signature of getColumnDefinitionUniquenessFragment() doesn't let me
 				// detect this case. (But that would be easy to fix!)
-				if ( !isSingleColumnUnique( uniqueKey ) ) {
+				if ( !isSingleColumnUnique( table, uniqueKey ) ) {
 					appendUniqueConstraint( fragment, uniqueKey );
 				}
 			}
@@ -79,9 +79,11 @@ public class CreateTableUniqueDelegate extends AlterTableUniqueDelegate {
 		fragment.append( uniqueConstraintSql(uniqueKey) );
 	}
 
-	private static boolean isSingleColumnUnique(UniqueKey uniqueKey) {
+	private static boolean isSingleColumnUnique(Table table, UniqueKey uniqueKey) {
 		return uniqueKey.getColumns().size() == 1
-			&& uniqueKey.getColumn(0).isUnique();
+				// Since columns are created on demand in IndexBinder.createColumn,
+				// we also have to check if the "real" column is unique to be safe
+				&& ( uniqueKey.getColumn( 0 ).isUnique() || table.getColumn( uniqueKey.getColumn( 0 ) ).isUnique() );
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/mapping/Table.java

@@ -377,21 +377,24 @@ public class Table implements Serializable, ContributableDatabaseObject {
 					final UniqueKey uniqueKey = uniqueKeyEntry.getValue();
 					boolean removeIt = false;
 
-					// condition 1 : check against other unique keys
-					for ( UniqueKey otherUniqueKey : uniqueKeys.values() ) {
-						// make sure it's not the same unique key
-						if ( uniqueKeyEntry.getValue() == otherUniqueKey ) {
-							continue;
-						}
-						if ( otherUniqueKey.getColumns().containsAll( uniqueKey.getColumns() )
-								&& uniqueKey.getColumns().containsAll( otherUniqueKey.getColumns() ) ) {
-							removeIt = true;
-							break;
+					// Never remove explicit unique keys based on column matching
+					if ( !uniqueKey.isExplicit() ) {
+						// condition 1 : check against other unique keys
+						for ( UniqueKey otherUniqueKey : uniqueKeys.values() ) {
+							// make sure it's not the same unique key
+							if ( uniqueKeyEntry.getValue() == otherUniqueKey ) {
+								continue;
+							}
+							if ( otherUniqueKey.getColumns().containsAll( uniqueKey.getColumns() )
+									&& uniqueKey.getColumns().containsAll( otherUniqueKey.getColumns() ) ) {
+								removeIt = true;
+								break;
+							}
 						}
 					}
 
 					// condition 2 : check against pk
-					if ( isSameAsPrimaryKeyColumns( uniqueKeyEntry.getValue() ) ) {
+					if ( !removeIt && isSameAsPrimaryKeyColumns( uniqueKeyEntry.getValue() ) ) {
 						removeIt = true;
 					}
 

hibernate-core/src/main/java/org/hibernate/mapping/UniqueKey.java

@@ -23,6 +23,7 @@ import static org.hibernate.internal.util.StringHelper.isNotEmpty;
 public class UniqueKey extends Constraint {
 	private final Map<Column, String> columnOrderMap = new HashMap<>();
 	private boolean nameExplicit; // true when the constraint name was explicitly specified by @UniqueConstraint annotation
+	private boolean explicit; // true when the constraint was explicitly specified by @UniqueConstraint annotation
 
 	@Override @Deprecated(since="6.2", forRemoval = true)
 	public String sqlConstraintString(
@@ -63,6 +64,14 @@ public class UniqueKey extends Constraint {
 		this.nameExplicit = nameExplicit;
 	}
 
+	public boolean isExplicit() {
+		return explicit;
+	}
+
+	public void setExplicit(boolean explicit) {
+		this.explicit = explicit;
+	}
+
 	public boolean hasNullableColumn() {
 		for ( Column column : getColumns() ) {
 			final Column tableColumn = getTable().getColumn( column );

hibernate-core/src/test/java/org/hibernate/orm/test/annotations/naturalid/NaturalIdUniqueConstraintNameTest.java

@@ -23,7 +23,7 @@ public class NaturalIdUniqueConstraintNameTest extends BaseCoreFunctionalTestCas
 
 	@Override
 	protected Class<?>[] getAnnotatedClasses() {
-		return new Class<?>[] { City.class };
+		return new Class<?>[] { City1.class, City2.class };
 	}
 
 	@Test
@@ -32,7 +32,7 @@ public class NaturalIdUniqueConstraintNameTest extends BaseCoreFunctionalTestCas
 				.addAnnotatedClasses( getAnnotatedClasses() )
 				.buildMetadata();
 
-		Map<String, UniqueKey> uniqueKeys = metadata.getEntityBinding( City.class.getName() )
+		Map<String, UniqueKey> uniqueKeys = metadata.getEntityBinding( City1.class.getName() )
 				.getTable()
 				.getUniqueKeys();
 
@@ -44,9 +44,65 @@ public class NaturalIdUniqueConstraintNameTest extends BaseCoreFunctionalTestCas
 		assertEquals( "UK_zipCode_city", uniqueKey.getName() );
 	}
 
-	@Entity(name = "City")
+	@Test
+	public void testNaturalIdUsesExplicitColumns() {
+		Metadata metadata = new MetadataSources( serviceRegistry() )
+				.addAnnotatedClasses( getAnnotatedClasses() )
+				.buildMetadata();
+
+		Map<String, UniqueKey> uniqueKeys = metadata.getEntityBinding( City2.class.getName() )
+				.getTable()
+				.getUniqueKeys();
+
+		// The unique key should not be duplicated for NaturalID + UniqueConstraint.
+		assertEquals( 1, uniqueKeys.size() );
+
+		// The unique key should use the name specified in UniqueConstraint.
+		UniqueKey uniqueKey = uniqueKeys.values().iterator().next();
+		assertEquals( "zipCode", uniqueKey.getColumns().get( 0 ).getName() );
+		assertEquals( "city", uniqueKey.getColumns().get( 1 ).getName() );
+	}
+
+	@Entity(name = "City1")
 	@Table(uniqueConstraints = @UniqueConstraint(name = "UK_zipCode_city", columnNames = { "zipCode", "city" }))
-	public static class City {
+	public static class City1 {
+
+		@Id
+		private Long id;
+
+		@NaturalId
+		private String zipCode;
+		@NaturalId
+		private String city;
+
+		public Long getId() {
+			return id;
+		}
+
+		public void setId(Long id) {
+			this.id = id;
+		}
+
+		public String getZipCode() {
+			return zipCode;
+		}
+
+		public void setZipCode(String zipCode) {
+			this.zipCode = zipCode;
+		}
+
+		public String getCity() {
+			return city;
+		}
+
+		public void setCity(String city) {
+			this.city = city;
+		}
+	}
+
+	@Entity(name = "City2")
+	@Table(uniqueConstraints = @UniqueConstraint(columnNames = { "zipCode", "city" }))
+	public static class City2 {
 
 		@Id
 		private Long id;