Skip to content

Commit 4df8388

Browse files
gavinkinggavinking
committed
HHH-19479 automatically make referenced column unique
this was happening for multi-column references, but not for single columns
1 parent 5f7d562 commit 4df8388

File tree

5 files changed

+79
-47
lines changed

5 files changed

+79
-47
lines changed

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,12 @@ private static void checkColumnInSameTable(
224224
* considered the target of the association. This method adds
225225
* the property holding the synthetic component to the target
226226
* entity {@link PersistentClass} by side effect.
227+
* <p>
228+
* This method automatically marks the reference column unique,
229+
* or creates a unique key on the referenced columns. It's not
230+
* really clear that we should do this. Perhaps we should just
231+
* validate that they are unique and error if not, like in
232+
* {@code TableBinder.checkReferenceToUniqueKey()}.
227233
*/
228234
private static Property referencedProperty(
229235
PersistentClass ownerEntity,
@@ -238,7 +244,10 @@ private static Property referencedProperty(
238244
&& ownerEntity == columnOwner
239245
&& !( properties.get(0).getValue() instanceof ToOne ) ) {
240246
// no need to make a synthetic property
241-
return properties.get(0);
247+
final Property property = properties.get( 0 );
248+
// mark it unique
249+
property.getValue().createUniqueKey( context );
250+
return property;
242251
}
243252
else {
244253
// Create a synthetic Property whose Value is a synthetic

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import java.util.StringTokenizer;
1111

1212
import org.hibernate.AnnotationException;
13+
import org.hibernate.AssertionFailure;
1314
import org.hibernate.boot.model.naming.Identifier;
1415
import org.hibernate.boot.model.naming.ImplicitIndexNameSource;
1516
import org.hibernate.boot.model.naming.ImplicitNamingStrategy;
@@ -175,7 +176,12 @@ private void createIndexOrUniqueKey(
175176
uniqueKey.setNameExplicit( nameExplicit );
176177
uniqueKey.setOptions( options );
177178
for ( int i = 0; i < columns.length; i++ ) {
178-
uniqueKey.addColumn( (Column) columns[i], orderings != null ? orderings[i] : null );
179+
if ( columns[i] instanceof Column column) {
180+
uniqueKey.addColumn( column, orderings != null ? orderings[i] : null );
181+
}
182+
else {
183+
throw new AssertionFailure( "Not a column" );
184+
}
179185
}
180186
}
181187
else {

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

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.hibernate.mapping.SortableValue;
3535
import org.hibernate.mapping.Table;
3636
import org.hibernate.mapping.ToOne;
37+
import org.hibernate.mapping.UniqueKey;
3738
import org.hibernate.mapping.Value;
3839

3940
import jakarta.persistence.Index;
@@ -752,7 +753,35 @@ else if ( value instanceof DependantValue ) {
752753
+ referencedEntity.getEntityName() + "." + referencedPropertyName );
753754
}
754755
linkJoinColumnWithValueOverridingNameIfImplicit( referencedEntity, synthProp.getValue(), joinColumns, value );
755-
( (SortableValue) value).sortProperties();
756+
checkReferenceToUniqueKey( value, synthProp );
757+
( (SortableValue) value ).sortProperties();
758+
}
759+
760+
// This code is good but unnecessary, because BinderHelper.referencedProperty()
761+
// automatically marks the referenced property unique (but is this actually good?)
762+
private static void checkReferenceToUniqueKey(SimpleValue value, Property synthProp) {
763+
final Table table = synthProp.getValue().getTable();
764+
final List<Column> columns = synthProp.getValue().getConstraintColumns();
765+
if ( columns.size() == 1 ) {
766+
final Column column = columns.get( 0 );
767+
assert column.isUnique();
768+
// if ( !column.isUnique() ) {
769+
// throw new MappingException( "Referenced column '" + column.getName()
770+
// + "' in table '" + table.getName() + "' is not unique"
771+
// + " ('@JoinColumn' must reference a unique key)" );
772+
// }
773+
}
774+
else {
775+
final UniqueKey uniqueKey = new UniqueKey( table );
776+
for ( Column column : columns ) {
777+
uniqueKey.addColumn( column );
778+
}
779+
assert table.isRedundantUniqueKey( uniqueKey );
780+
// if ( !table.isRedundantUniqueKey( uniqueKey ) ) {
781+
// throw new MappingException( "Referenced columns in table '" + table.getName() + "' are not unique"
782+
// + " ('@JoinColumn's must reference a unique key" );
783+
// }
784+
}
756785
}
757786

758787
private static void bindImplicitColumns(

‎hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ public ForeignKey createForeignKeyOfEntity(String entityName) {
354354
@Override
355355
public void createUniqueKey(MetadataBuildingContext context) {
356356
if ( hasFormula() ) {
357-
throw new MappingException( "unique key constraint involves formulas" );
357+
throw new MappingException( "Unique key constraint involves formulas" );
358358
}
359359
getTable().createUniqueKey( getConstraintColumns(), context );
360360
}

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

Lines changed: 31 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ public void addColumn(Column column) {
297297

298298
@Internal
299299
public void columnRenamed(Column column) {
300-
for ( Map.Entry<String, Column> entry : columns.entrySet() ) {
300+
for ( var entry : columns.entrySet() ) {
301301
if ( entry.getValue() == column ) {
302302
columns.remove( entry.getKey() );
303303
columns.put( column.getCanonicalName(), column );
@@ -330,12 +330,10 @@ public Map<String, UniqueKey> getUniqueKeys() {
330330
private int sizeOfUniqueKeyMapOnLastCleanse;
331331

332332
private void cleanseUniqueKeyMapIfNeeded() {
333-
if ( uniqueKeys.size() == sizeOfUniqueKeyMapOnLastCleanse ) {
334-
// nothing to do
335-
return;
333+
if ( uniqueKeys.size() != sizeOfUniqueKeyMapOnLastCleanse ) {
334+
cleanseUniqueKeyMap();
335+
sizeOfUniqueKeyMapOnLastCleanse = uniqueKeys.size();
336336
}
337-
cleanseUniqueKeyMap();
338-
sizeOfUniqueKeyMapOnLastCleanse = uniqueKeys.size();
339337
}
340338

341339
private void cleanseUniqueKeyMap() {
@@ -351,57 +349,47 @@ private void cleanseUniqueKeyMap() {
351349
if ( !uniqueKeys.isEmpty() ) {
352350
if ( uniqueKeys.size() == 1 ) {
353351
// we have to worry about condition 2 above, but not condition 1
354-
final Map.Entry<String,UniqueKey> uniqueKeyEntry = uniqueKeys.entrySet().iterator().next();
352+
final var uniqueKeyEntry = uniqueKeys.entrySet().iterator().next();
355353
if ( isSameAsPrimaryKeyColumns( uniqueKeyEntry.getValue() ) ) {
356354
uniqueKeys.remove( uniqueKeyEntry.getKey() );
357355
}
358356
}
359357
else {
360358
// we have to check both conditions 1 and 2
361-
final Iterator<Map.Entry<String,UniqueKey>> uniqueKeyEntries = uniqueKeys.entrySet().iterator();
362-
while ( uniqueKeyEntries.hasNext() ) {
363-
final Map.Entry<String,UniqueKey> uniqueKeyEntry = uniqueKeyEntries.next();
364-
final UniqueKey uniqueKey = uniqueKeyEntry.getValue();
365-
boolean removeIt = false;
366-
367-
// Never remove explicit unique keys based on column matching
368-
if ( !uniqueKey.isExplicit() ) {
369-
// condition 1 : check against other unique keys
370-
for ( UniqueKey otherUniqueKey : uniqueKeys.values() ) {
371-
// make sure it's not the same unique key
372-
if ( uniqueKeyEntry.getValue() == otherUniqueKey ) {
373-
continue;
374-
}
375-
if ( otherUniqueKey.getColumns().containsAll( uniqueKey.getColumns() )
376-
&& uniqueKey.getColumns().containsAll( otherUniqueKey.getColumns() ) ) {
377-
removeIt = true;
378-
break;
379-
}
380-
}
381-
}
359+
//uniqueKeys.remove( uniqueKeyEntry.getKey() );
360+
uniqueKeys.entrySet().removeIf( entry -> isRedundantUniqueKey( entry.getValue() ) );
361+
}
362+
}
363+
}
382364

383-
// condition 2 : check against pk
384-
if ( !removeIt && isSameAsPrimaryKeyColumns( uniqueKeyEntry.getValue() ) ) {
385-
primaryKey.setOrderingUniqueKey(uniqueKeyEntry.getValue());
386-
removeIt = true;
387-
}
365+
public boolean isRedundantUniqueKey(UniqueKey uniqueKey) {
388366

389-
if ( removeIt ) {
390-
//uniqueKeys.remove( uniqueKeyEntry.getKey() );
391-
uniqueKeyEntries.remove();
392-
}
367+
// Never remove explicit unique keys based on column matching
368+
if ( !uniqueKey.isExplicit() ) {
369+
// condition 1 : check against other unique keys
370+
for ( UniqueKey otherUniqueKey : uniqueKeys.values() ) {
371+
// make sure it's not the same unique key
372+
if ( uniqueKey != otherUniqueKey
373+
&& otherUniqueKey.getColumns().containsAll( uniqueKey.getColumns() )
374+
&& uniqueKey.getColumns().containsAll( otherUniqueKey.getColumns() ) ) {
375+
return true;
393376
}
394377
}
395378
}
379+
380+
// condition 2 : check against pk
381+
if ( isSameAsPrimaryKeyColumns( uniqueKey ) ) {
382+
primaryKey.setOrderingUniqueKey( uniqueKey );
383+
return true;
384+
}
385+
386+
return false;
396387
}
397388

398389
private boolean isSameAsPrimaryKeyColumns(UniqueKey uniqueKey) {
399-
if ( primaryKey == null || primaryKey.getColumns().isEmpty() ) {
400-
// happens for many-to-many tables
401-
return false;
402-
}
403-
return primaryKey.getColumns().containsAll( uniqueKey.getColumns() )
404-
&& primaryKey.getColumns().size() == uniqueKey.getColumns().size();
390+
return primaryKey != null && !primaryKey.getColumns().isEmpty() // happens for many-to-many tables
391+
&& primaryKey.getColumns().size() == uniqueKey.getColumns().size()
392+
&& primaryKey.getColumns().containsAll( uniqueKey.getColumns() );
405393
}
406394

407395
@Override

0 commit comments

Comments
 (0)