From 12aa8bd431b4c94e96b8ce133257ad8dca6dcbee Mon Sep 17 00:00:00 2001 From: Gavin Date: Sun, 27 Nov 2022 11:53:11 +0100 Subject: [PATCH] add some comments for the next poor soul who wrestles with unique constraints --- .../community/dialect/DB2LegacyDialect.java | 4 ++++ .../community/dialect/DB2iLegacyDialect.java | 3 +++ .../community/dialect/DB2zLegacyDialect.java | 3 +++ .../community/dialect/SQLServerLegacyDialect.java | 2 ++ .../dialect/unique/AlterTableUniqueDelegate.java | 13 ++++++------- .../unique/AlterTableUniqueIndexDelegate.java | 8 +++++--- .../dialect/unique/CreateTableUniqueDelegate.java | 3 +++ .../unique/SkipNullableUniqueDelegate.java | 2 +- .../hibernate/dialect/unique/UniqueDelegate.java | 15 +++++++++++++-- 9 files changed, 40 insertions(+), 13 deletions(-) diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DB2LegacyDialect.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DB2LegacyDialect.java index f8f6144297..c288694366 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DB2LegacyDialect.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DB2LegacyDialect.java @@ -212,7 +212,9 @@ public class DB2LegacyDialect extends Dialect { protected UniqueDelegate createUniqueDelegate() { return getDB2Version().isSameOrAfter(10,5) + //use 'create unique index ... exclude null keys' ? new AlterTableUniqueIndexDelegate( this ) + //ignore unique keys on nullable columns in earlier versions : new SkipNullableUniqueDelegate( this ); } @@ -495,6 +497,8 @@ public class DB2LegacyDialect extends Dialect { @Override public String getCreateIndexTail(boolean unique, List columns) { + // we only create unique indexes, as opposed to unique constraints, + // when the column is nullable, so safe to infer unique => nullable return unique ? " exclude null keys" : ""; } diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DB2iLegacyDialect.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DB2iLegacyDialect.java index 67efadca45..7aa24423b0 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DB2iLegacyDialect.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DB2iLegacyDialect.java @@ -73,8 +73,11 @@ public class DB2iLegacyDialect extends DB2LegacyDialect { @Override protected UniqueDelegate createUniqueDelegate() { + //TODO: when was 'create unique where not null index' really first introduced? return getVersion().isSameOrAfter(7, 1) + //use 'create unique where not null index' ? new AlterTableUniqueIndexDelegate(this) + //ignore unique keys on nullable columns in earlier versions : new SkipNullableUniqueDelegate(this); } diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DB2zLegacyDialect.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DB2zLegacyDialect.java index 9e35763f00..785120ee49 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DB2zLegacyDialect.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DB2zLegacyDialect.java @@ -88,8 +88,11 @@ public class DB2zLegacyDialect extends DB2LegacyDialect { @Override protected UniqueDelegate createUniqueDelegate() { + //TODO: when was 'create unique where not null index' really first introduced? return getVersion().isSameOrAfter(11) + //use 'create unique where not null index' ? new AlterTableUniqueIndexDelegate(this) + //ignore unique keys on nullable columns in earlier versions : new SkipNullableUniqueDelegate(this); } diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SQLServerLegacyDialect.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SQLServerLegacyDialect.java index 7eb77b92cc..79beabc10d 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SQLServerLegacyDialect.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SQLServerLegacyDialect.java @@ -120,7 +120,9 @@ public class SQLServerLegacyDialect extends AbstractTransactSQLDialect { private UniqueDelegate createUniqueDelgate(DatabaseVersion version) { return version.isSameOrAfter(10) + //use 'create unique nonclustered index ... where ...' ? new AlterTableUniqueIndexDelegate(this) + //ignore unique keys on nullable columns in versions before 2008 : new SkipNullableUniqueDelegate(this); } diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/unique/AlterTableUniqueDelegate.java b/hibernate-core/src/main/java/org/hibernate/dialect/unique/AlterTableUniqueDelegate.java index 14ad842578..bbd29fe7e4 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/unique/AlterTableUniqueDelegate.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/unique/AlterTableUniqueDelegate.java @@ -75,17 +75,16 @@ public class AlterTableUniqueDelegate implements UniqueDelegate { public String getAlterTableToDropUniqueKeyCommand(UniqueKey uniqueKey, Metadata metadata, SqlStringGenerationContext context) { final String tableName = context.format( uniqueKey.getTable().getQualifiedTableName() ); - - final StringBuilder buf = new StringBuilder( dialect.getAlterTableString(tableName) ); - buf.append( dialect.getDropUniqueKeyString() ); + final StringBuilder command = new StringBuilder( dialect.getAlterTableString(tableName) ); + command.append( dialect.getDropUniqueKeyString() ); if ( dialect.supportsIfExistsBeforeConstraintName() ) { - buf.append( "if exists " ); + command.append( "if exists " ); } - buf.append( dialect.quote( uniqueKey.getName() ) ); + command.append( dialect.quote( uniqueKey.getName() ) ); if ( dialect.supportsIfExistsAfterConstraintName() ) { - buf.append( " if exists" ); + command.append( " if exists" ); } - return buf.toString(); + return command.toString(); } } diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/unique/AlterTableUniqueIndexDelegate.java b/hibernate-core/src/main/java/org/hibernate/dialect/unique/AlterTableUniqueIndexDelegate.java index 4f64aafccc..b2d3ce899b 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/unique/AlterTableUniqueIndexDelegate.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/unique/AlterTableUniqueIndexDelegate.java @@ -12,15 +12,17 @@ import org.hibernate.dialect.Dialect; import org.hibernate.mapping.UniqueKey; import static org.hibernate.mapping.Index.buildSqlCreateIndexString; +import static org.hibernate.mapping.Index.buildSqlDropIndexString; /** * A {@link UniqueDelegate} which uses {@code create unique index} commands when necessary. * * * @author Brett Meyer @@ -53,7 +55,7 @@ public class AlterTableUniqueIndexDelegate extends AlterTableUniqueDelegate { public String getAlterTableToDropUniqueKeyCommand(UniqueKey uniqueKey, Metadata metadata, SqlStringGenerationContext context) { if ( uniqueKey.hasNullableColumn() ) { - return org.hibernate.mapping.Index.buildSqlDropIndexString( + return buildSqlDropIndexString( uniqueKey.getName(), context.format( uniqueKey.getTable().getQualifiedTableName() ) ); diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/unique/CreateTableUniqueDelegate.java b/hibernate-core/src/main/java/org/hibernate/dialect/unique/CreateTableUniqueDelegate.java index 40613519be..1ba561db7f 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/unique/CreateTableUniqueDelegate.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/unique/CreateTableUniqueDelegate.java @@ -24,6 +24,9 @@ import org.hibernate.mapping.UniqueKey; *
  • For unique keys with no explicit name, it results in {@code unique(x, y)} after the * column list. * + * Counterintuitively, this class extends {@link AlterTableUniqueDelegate}, since it falls back + * to using {@code alter table} for {@linkplain org.hibernate.tool.schema.spi.SchemaMigrator + * schema migration}. * * @author Gavin King */ diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/unique/SkipNullableUniqueDelegate.java b/hibernate-core/src/main/java/org/hibernate/dialect/unique/SkipNullableUniqueDelegate.java index 9c4ae613f7..5aaefa80ed 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/unique/SkipNullableUniqueDelegate.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/unique/SkipNullableUniqueDelegate.java @@ -19,7 +19,7 @@ import org.hibernate.mapping.UniqueKey; * Needed because unique constraints on nullable columns in Sybase always consider null values to be non-unique. * There is simply no way to create a unique constraint with the semantics we want on a nullable column in Sybase >:-( *

    - * You might argue that this was a bad decision because if the programmer explicitly specifies an {@code @UniqueKey}, + * You might argue that this behavior is bad because if the programmer explicitly specifies an {@code @UniqueKey}, * then we should damn well respect their wishes. But the simple answer is that the user should have also specified * {@code @Column(nullable=false)} if that is what they wanted. A unique key on a nullable column just really doesn't * make sense in Sybase, except, perhaps, in some incredibly corner cases. diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/unique/UniqueDelegate.java b/hibernate-core/src/main/java/org/hibernate/dialect/unique/UniqueDelegate.java index 4ffd5361b9..14dc57b477 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/unique/UniqueDelegate.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/unique/UniqueDelegate.java @@ -13,7 +13,7 @@ import org.hibernate.mapping.Table; import org.hibernate.mapping.UniqueKey; /** - * Dialect-level delegate in charge of applying unique constraints in DDL. Uniqueness can + * Dialect-level delegate responsible for applying unique constraints in DDL. Uniqueness can * be specified in any of three ways: *

      *
    1. @@ -30,7 +30,18 @@ import org.hibernate.mapping.UniqueKey; * Also, see {@link #getAlterTableToDropUniqueKeyCommand}. *
      2. *
    - * The first two options are generally preferred. + * The first two options are generally preferred, and so we use {@link CreateTableUniqueDelegate} + * where possible. However, for databases where unique constraints may not contain a nullable + * column, and unique indexes must be used instead, we use {@link AlterTableUniqueIndexDelegate}. + *

    + * Hibernate specifies that a unique constraint on a nullable column considers null values to be + * distinct. Some databases default to the opposite semantic, where null values are considered + * equal for the purpose of determining uniqueness. This is almost never useful, and is the + * opposite of what we want when we use a unique constraint on a foreign key to map an optional + * {@link org.hibernate.mapping.OneToOne} association. Therefore, our {@code UniqueDelegate}s must + * jump through hoops to emulate the sensible semantics specified by ANSI, Hibernate, and common + * sense, namely, that two null values are distinct. A particularly egregious offender is Sybase, + * where we must simply {@linkplain SkipNullableUniqueDelegate skip creating the unique constraint}. * * @author Brett Meyer */