feat: EXPOSED-85 Add support for changing default value in SQL Server (…

…JetBrains#1812)

* feat: Add support for changing default value in SQL Server

When creating a column with a DEFAULT value in SQL Server, a DEFAULT constraint is created under the hood. To modify the default value of a column, the constraint has to be dropped first. To do that, the name of the constraint is needed. To be able to obtain its name easily, the way the DEFAULT constraint is created was modified to give it an explicit name with the format DF_TableName_ColumnName. So when adding a new DEFAULT value to a column or modifying an existing one, it's necessary to drop the existing DEFAULT constraint first, and then add a new one.

* fix: Change the way modifyColumn constructs its statements because the previous version was not including all necessary modifications

Every alter statement will be a separate item in the list returned by the modifyColumn function. The statement to alter the default value of a column will combine both the DROP CONSTRAINT and ADD CONSTRAINT statements in one String item, separated by a semicolon. The statement to alter the type and nullability will be part of one String item since that syntax is allowed in SQL Server, but it seems like it's not allowed for adding and dropping constraints.

exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Column.kt

@@ -3,6 +3,7 @@ package org.jetbrains.exposed.sql
 import org.jetbrains.exposed.exceptions.throwUnsupportedException
 import org.jetbrains.exposed.sql.transactions.TransactionManager
 import org.jetbrains.exposed.sql.vendors.H2Dialect
+import org.jetbrains.exposed.sql.vendors.SQLServerDialect
 import org.jetbrains.exposed.sql.vendors.SQLiteDialect
 import org.jetbrains.exposed.sql.vendors.currentDialect
 import org.jetbrains.exposed.sql.vendors.inProperCase
@@ -112,7 +113,15 @@ class Column<T>(
                 }
                 exposedLogger.error("${currentDialect.name} ${tr.db.version} doesn't support expression '$expressionSQL' as default value.$clientDefault")
             } else {
-                append(" DEFAULT $expressionSQL")
+                if (currentDialect is SQLServerDialect) {
+                    // Create a DEFAULT constraint with an explicit name to facilitate removing it later if needed
+                    val tableName = column.table.tableName
+                    val columnName = column.name
+                    val constraintName = "DF_${tableName}_$columnName"
+                    append(" CONSTRAINT $constraintName DEFAULT $expressionSQL")
+                } else {
+                    append(" DEFAULT $expressionSQL")
+                }
             }
         }
 

exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/SQLServerDialect.kt

@@ -251,9 +251,55 @@ open class SQLServerDialect : VendorDialect(dialectName, SQLServerDataTypeProvid
         return columnDefault !in nonAcceptableDefaults
     }
 
-    // EXPOSED-85 Fix changing default value on column in SQL Server as it requires to drop/create constraint
-    override fun modifyColumn(column: Column<*>, columnDiff: ColumnDiff): List<String> =
-        super.modifyColumn(column, columnDiff).map { it.replace("MODIFY COLUMN", "ALTER COLUMN") }
+    override fun modifyColumn(column: Column<*>, columnDiff: ColumnDiff): List<String> {
+        val transaction = TransactionManager.current()
+
+        val alterTablePart = "ALTER TABLE ${transaction.identity(column.table)} "
+
+        val statements = mutableListOf<String>()
+
+        statements.add(
+            buildString {
+                append(alterTablePart + "ALTER COLUMN ${transaction.identity(column)} ${column.columnType.sqlType()}")
+
+                if (columnDiff.nullability) {
+                    val defaultValue = column.dbDefaultValue
+                    val isPKColumn = column.table.primaryKey?.columns?.contains(column) == true
+
+                    if (column.columnType.nullable ||
+                        (defaultValue != null && column.defaultValueFun == null && ! currentDialect.isAllowedAsColumnDefault(defaultValue))
+                    ) {
+                        append(" NULL")
+                    } else if (!isPKColumn) {
+                        append(" NOT NULL")
+                    }
+                }
+            }
+        )
+
+        if (columnDiff.defaults) {
+            val tableName = column.table.tableName
+            val columnName = column.name
+            val constraintName = "DF_${tableName}_$columnName"
+
+            val dropConstraint = "DROP CONSTRAINT IF EXISTS $constraintName"
+
+            statements.add(
+                buildString {
+                    column.dbDefaultValue?.let {
+                        append(alterTablePart + dropConstraint)
+                        append("; ")
+                        append(
+                            alterTablePart +
+                                "ADD CONSTRAINT $constraintName DEFAULT ${SQLServerDataTypeProvider.processForDefaultValue(it)} for ${transaction.identity(column)}"
+                        )
+                    } ?: append(alterTablePart + dropConstraint)
+                }
+            )
+        }
+
+        return statements
+    }
 
     override fun createDatabase(name: String): String = "CREATE DATABASE ${name.inProperCase()}"
 
@@ -284,7 +330,13 @@ open class SQLServerDialect : VendorDialect(dialectName, SQLServerDataTypeProvid
         return super.createIndex(index)
     }
 
-    override fun createIndexWithType(name: String, table: String, columns: String, type: String, filterCondition: String): String {
+    override fun createIndexWithType(
+        name: String,
+        table: String,
+        columns: String,
+        type: String,
+        filterCondition: String
+    ): String {
         return "CREATE $type INDEX $name ON $table $columns$filterCondition"
     }
 

exposed-java-time/src/test/kotlin/org/jetbrains/exposed/DefaultsTest.kt

@@ -11,6 +11,7 @@ import org.jetbrains.exposed.sql.statements.BatchDataInconsistentException
 import org.jetbrains.exposed.sql.statements.BatchInsertStatement
 import org.jetbrains.exposed.sql.tests.DatabaseTestsBase
 import org.jetbrains.exposed.sql.tests.TestDB
+import org.jetbrains.exposed.sql.tests.constraintNamePart
 import org.jetbrains.exposed.sql.tests.currentDialectTest
 import org.jetbrains.exposed.sql.tests.inProperCase
 import org.jetbrains.exposed.sql.tests.shared.assertEqualCollections
@@ -246,20 +247,20 @@ class DefaultsTest : DatabaseTestsBase() {
             val baseExpression = "CREATE TABLE " + addIfNotExistsIfSupported() +
                 "${"t".inProperCase()} (" +
                 "${"id".inProperCase()} ${currentDialectTest.dataTypeProvider.integerAutoincType()} PRIMARY KEY, " +
-                "${"s".inProperCase()} $varcharType DEFAULT 'test' NOT NULL, " +
-                "${"sn".inProperCase()} $varcharType DEFAULT 'testNullable' NULL, " +
-                "${"l".inProperCase()} ${currentDialectTest.dataTypeProvider.longType()} DEFAULT 42 NOT NULL, " +
-                "$q${"c".inProperCase()}$q CHAR DEFAULT 'X' NOT NULL, " +
-                "${"t1".inProperCase()} $dtType ${currentDT.itOrNull()}, " +
-                "${"t2".inProperCase()} $dtType ${nowExpression.itOrNull()}, " +
-                "${"t3".inProperCase()} $dtType ${dtLiteral.itOrNull()}, " +
-                "${"t4".inProperCase()} DATE ${dLiteral.itOrNull()}, " +
-                "${"t5".inProperCase()} $dtType ${tsLiteral.itOrNull()}, " +
-                "${"t6".inProperCase()} $dtType ${tsLiteral.itOrNull()}, " +
-                "${"t7".inProperCase()} $longType ${durLiteral.itOrNull()}, " +
-                "${"t8".inProperCase()} $longType ${durLiteral.itOrNull()}, " +
-                "${"t9".inProperCase()} $timeType ${tLiteral.itOrNull()}, " +
-                "${"t10".inProperCase()} $timeType ${tLiteral.itOrNull()}" +
+                "${"s".inProperCase()} $varcharType${testTable.s.constraintNamePart()} DEFAULT 'test' NOT NULL, " +
+                "${"sn".inProperCase()} $varcharType${testTable.sn.constraintNamePart()} DEFAULT 'testNullable' NULL, " +
+                "${"l".inProperCase()} ${currentDialectTest.dataTypeProvider.longType()}${testTable.l.constraintNamePart()} DEFAULT 42 NOT NULL, " +
+                "$q${"c".inProperCase()}$q CHAR${testTable.c.constraintNamePart()} DEFAULT 'X' NOT NULL, " +
+                "${"t1".inProperCase()} $dtType${testTable.t1.constraintNamePart()} ${currentDT.itOrNull()}, " +
+                "${"t2".inProperCase()} $dtType${testTable.t2.constraintNamePart()} ${nowExpression.itOrNull()}, " +
+                "${"t3".inProperCase()} $dtType${testTable.t3.constraintNamePart()} ${dtLiteral.itOrNull()}, " +
+                "${"t4".inProperCase()} DATE${testTable.t4.constraintNamePart()} ${dLiteral.itOrNull()}, " +
+                "${"t5".inProperCase()} $dtType${testTable.t5.constraintNamePart()} ${tsLiteral.itOrNull()}, " +
+                "${"t6".inProperCase()} $dtType${testTable.t6.constraintNamePart()} ${tsLiteral.itOrNull()}, " +
+                "${"t7".inProperCase()} $longType${testTable.t7.constraintNamePart()} ${durLiteral.itOrNull()}, " +
+                "${"t8".inProperCase()} $longType${testTable.t8.constraintNamePart()} ${durLiteral.itOrNull()}, " +
+                "${"t9".inProperCase()} $timeType${testTable.t9.constraintNamePart()} ${tLiteral.itOrNull()}, " +
+                "${"t10".inProperCase()} $timeType${testTable.t10.constraintNamePart()} ${tLiteral.itOrNull()}" +
                 ")"
 
             val expected = if (currentDialectTest is OracleDialect || currentDialectTest.h2Mode == H2Dialect.H2CompatibilityMode.Oracle) {
@@ -422,8 +423,8 @@ class DefaultsTest : DatabaseTestsBase() {
                 val baseExpression = "CREATE TABLE " + addIfNotExistsIfSupported() +
                     "${"t".inProperCase()} (" +
                     "${"id".inProperCase()} ${currentDialectTest.dataTypeProvider.integerAutoincType()} PRIMARY KEY, " +
-                    "${"t1".inProperCase()} $timestampWithTimeZoneType ${timestampWithTimeZoneLiteral.itOrNull()}, " +
-                    "${"t2".inProperCase()} $timestampWithTimeZoneType ${timestampWithTimeZoneLiteral.itOrNull()}" +
+                    "${"t1".inProperCase()} $timestampWithTimeZoneType${testTable.t1.constraintNamePart()} ${timestampWithTimeZoneLiteral.itOrNull()}, " +
+                    "${"t2".inProperCase()} $timestampWithTimeZoneType${testTable.t2.constraintNamePart()} ${timestampWithTimeZoneLiteral.itOrNull()}" +
                     ")"
 
                 val expected = if (currentDialectTest is OracleDialect ||

exposed-jodatime/src/test/kotlin/org/jetbrains/exposed/JodaTimeDefaultsTest.kt

@@ -10,6 +10,7 @@ import org.jetbrains.exposed.sql.jodatime.*
 import org.jetbrains.exposed.sql.statements.BatchDataInconsistentException
 import org.jetbrains.exposed.sql.statements.BatchInsertStatement
 import org.jetbrains.exposed.sql.tests.TestDB
+import org.jetbrains.exposed.sql.tests.constraintNamePart
 import org.jetbrains.exposed.sql.tests.currentDialectTest
 import org.jetbrains.exposed.sql.tests.inProperCase
 import org.jetbrains.exposed.sql.tests.shared.assertEqualCollections
@@ -179,14 +180,14 @@ class JodaTimeDefaultsTest : JodaTimeBaseTest() {
             val baseExpression = "CREATE TABLE " + addIfNotExistsIfSupported() +
                 "${"t".inProperCase()} (" +
                 "${"id".inProperCase()} ${currentDialectTest.dataTypeProvider.integerAutoincType()} PRIMARY KEY, " +
-                "${"s".inProperCase()} $varCharType DEFAULT 'test' NOT NULL, " +
-                "${"sn".inProperCase()} $varCharType DEFAULT 'testNullable' NULL, " +
-                "${"l".inProperCase()} ${currentDialectTest.dataTypeProvider.longType()} DEFAULT 42 NOT NULL, " +
-                "$q${"c".inProperCase()}$q CHAR DEFAULT 'X' NOT NULL, " +
-                "${"t1".inProperCase()} $dtType ${currentDT.itOrNull()}, " +
-                "${"t2".inProperCase()} $dtType ${nowExpression.itOrNull()}, " +
-                "${"t3".inProperCase()} $dtType ${dtLiteral.itOrNull()}, " +
-                "${"t4".inProperCase()} DATE ${dtLiteral.itOrNull()}" +
+                "${"s".inProperCase()} $varCharType${testTable.s.constraintNamePart()} DEFAULT 'test' NOT NULL, " +
+                "${"sn".inProperCase()} $varCharType${testTable.sn.constraintNamePart()} DEFAULT 'testNullable' NULL, " +
+                "${"l".inProperCase()} ${currentDialectTest.dataTypeProvider.longType()}${testTable.l.constraintNamePart()} DEFAULT 42 NOT NULL, " +
+                "$q${"c".inProperCase()}$q CHAR${testTable.c.constraintNamePart()} DEFAULT 'X' NOT NULL, " +
+                "${"t1".inProperCase()} $dtType${testTable.t1.constraintNamePart()} ${currentDT.itOrNull()}, " +
+                "${"t2".inProperCase()} $dtType${testTable.t2.constraintNamePart()} ${nowExpression.itOrNull()}, " +
+                "${"t3".inProperCase()} $dtType${testTable.t3.constraintNamePart()} ${dtLiteral.itOrNull()}, " +
+                "${"t4".inProperCase()} DATE${testTable.t4.constraintNamePart()} ${dtLiteral.itOrNull()}" +
                 ")"
 
             val expected = if (currentDialectTest is OracleDialect || currentDialectTest.h2Mode == H2Dialect.H2CompatibilityMode.Oracle) {
@@ -389,8 +390,8 @@ class JodaTimeDefaultsTest : JodaTimeBaseTest() {
                 val baseExpression = "CREATE TABLE " + addIfNotExistsIfSupported() +
                     "${"t".inProperCase()} (" +
                     "${"id".inProperCase()} ${currentDialectTest.dataTypeProvider.integerAutoincType()} PRIMARY KEY, " +
-                    "${"t1".inProperCase()} $timestampWithTimeZoneType ${timestampWithTimeZoneLiteral.itOrNull()}, " +
-                    "${"t2".inProperCase()} $timestampWithTimeZoneType ${timestampWithTimeZoneLiteral.itOrNull()}" +
+                    "${"t1".inProperCase()} $timestampWithTimeZoneType${testTable.t1.constraintNamePart()} ${timestampWithTimeZoneLiteral.itOrNull()}, " +
+                    "${"t2".inProperCase()} $timestampWithTimeZoneType${testTable.t2.constraintNamePart()} ${timestampWithTimeZoneLiteral.itOrNull()}" +
                     ")"
 
                 val expected = if (currentDialectTest is OracleDialect ||

exposed-kotlin-datetime/src/test/kotlin/org/jetbrains/exposed/sql/kotlin/datetime/DefaultsTest.kt

@@ -13,6 +13,7 @@ import org.jetbrains.exposed.sql.statements.BatchDataInconsistentException
 import org.jetbrains.exposed.sql.statements.BatchInsertStatement
 import org.jetbrains.exposed.sql.tests.DatabaseTestsBase
 import org.jetbrains.exposed.sql.tests.TestDB
+import org.jetbrains.exposed.sql.tests.constraintNamePart
 import org.jetbrains.exposed.sql.tests.currentDialectTest
 import org.jetbrains.exposed.sql.tests.inProperCase
 import org.jetbrains.exposed.sql.tests.shared.Category.defaultExpression
@@ -249,20 +250,20 @@ class DefaultsTest : DatabaseTestsBase() {
             val baseExpression = "CREATE TABLE " + addIfNotExistsIfSupported() +
                 "${"t".inProperCase()} (" +
                 "${"id".inProperCase()} ${currentDialectTest.dataTypeProvider.integerAutoincType()} PRIMARY KEY, " +
-                "${"s".inProperCase()} $varCharType DEFAULT 'test' NOT NULL, " +
-                "${"sn".inProperCase()} $varCharType DEFAULT 'testNullable' NULL, " +
-                "${"l".inProperCase()} ${currentDialectTest.dataTypeProvider.longType()} DEFAULT 42 NOT NULL, " +
-                "$q${"c".inProperCase()}$q CHAR DEFAULT 'X' NOT NULL, " +
-                "${"t1".inProperCase()} $dtType ${currentDT.itOrNull()}, " +
-                "${"t2".inProperCase()} $dtType ${nowExpression.itOrNull()}, " +
-                "${"t3".inProperCase()} $dtType ${dtLiteral.itOrNull()}, " +
-                "${"t4".inProperCase()} DATE ${dLiteral.itOrNull()}, " +
-                "${"t5".inProperCase()} $dtType ${tsLiteral.itOrNull()}, " +
-                "${"t6".inProperCase()} $dtType ${tsLiteral.itOrNull()}, " +
-                "${"t7".inProperCase()} $longType ${durLiteral.itOrNull()}, " +
-                "${"t8".inProperCase()} $longType ${durLiteral.itOrNull()}, " +
-                "${"t9".inProperCase()} $timeType ${tLiteral.itOrNull()}, " +
-                "${"t10".inProperCase()} $timeType ${tLiteral.itOrNull()}" +
+                "${"s".inProperCase()} $varCharType${testTable.s.constraintNamePart()} DEFAULT 'test' NOT NULL, " +
+                "${"sn".inProperCase()} $varCharType${testTable.sn.constraintNamePart()} DEFAULT 'testNullable' NULL, " +
+                "${"l".inProperCase()} ${currentDialectTest.dataTypeProvider.longType()}${testTable.l.constraintNamePart()} DEFAULT 42 NOT NULL, " +
+                "$q${"c".inProperCase()}$q CHAR${testTable.c.constraintNamePart()} DEFAULT 'X' NOT NULL, " +
+                "${"t1".inProperCase()} $dtType${testTable.t1.constraintNamePart()} ${currentDT.itOrNull()}, " +
+                "${"t2".inProperCase()} $dtType${testTable.t2.constraintNamePart()} ${nowExpression.itOrNull()}, " +
+                "${"t3".inProperCase()} $dtType${testTable.t3.constraintNamePart()} ${dtLiteral.itOrNull()}, " +
+                "${"t4".inProperCase()} DATE${testTable.t4.constraintNamePart()} ${dLiteral.itOrNull()}, " +
+                "${"t5".inProperCase()} $dtType${testTable.t5.constraintNamePart()} ${tsLiteral.itOrNull()}, " +
+                "${"t6".inProperCase()} $dtType${testTable.t6.constraintNamePart()} ${tsLiteral.itOrNull()}, " +
+                "${"t7".inProperCase()} $longType${testTable.t7.constraintNamePart()} ${durLiteral.itOrNull()}, " +
+                "${"t8".inProperCase()} $longType${testTable.t8.constraintNamePart()} ${durLiteral.itOrNull()}, " +
+                "${"t9".inProperCase()} $timeType${testTable.t9.constraintNamePart()} ${tLiteral.itOrNull()}, " +
+                "${"t10".inProperCase()} $timeType${testTable.t10.constraintNamePart()} ${tLiteral.itOrNull()}" +
                 ")"
 
             val expected = if (currentDialectTest is OracleDialect || currentDialectTest.h2Mode == H2Dialect.H2CompatibilityMode.Oracle) {
@@ -431,8 +432,8 @@ class DefaultsTest : DatabaseTestsBase() {
                 val baseExpression = "CREATE TABLE " + addIfNotExistsIfSupported() +
                     "${"t".inProperCase()} (" +
                     "${"id".inProperCase()} ${currentDialectTest.dataTypeProvider.integerAutoincType()} PRIMARY KEY, " +
-                    "${"t1".inProperCase()} $timestampWithTimeZoneType ${timestampWithTimeZoneLiteral.itOrNull()}, " +
-                    "${"t2".inProperCase()} $timestampWithTimeZoneType ${timestampWithTimeZoneLiteral.itOrNull()}" +
+                    "${"t1".inProperCase()} $timestampWithTimeZoneType${testTable.t1.constraintNamePart()} ${timestampWithTimeZoneLiteral.itOrNull()}, " +
+                    "${"t2".inProperCase()} $timestampWithTimeZoneType${testTable.t2.constraintNamePart()} ${timestampWithTimeZoneLiteral.itOrNull()}" +
                     ")"
 
                 val expected = if (currentDialectTest is OracleDialect ||

exposed-tests/src/main/kotlin/org/jetbrains/exposed/sql/tests/TestUtils.kt

@@ -1,7 +1,9 @@
 package org.jetbrains.exposed.sql.tests
 
+import org.jetbrains.exposed.sql.Column
 import org.jetbrains.exposed.sql.transactions.TransactionManager
 import org.jetbrains.exposed.sql.vendors.DatabaseDialect
+import org.jetbrains.exposed.sql.vendors.SQLServerDialect
 import java.util.EnumSet
 
 fun String.inProperCase(): String = TransactionManager.currentOrNull()?.db?.identifierManager?.inProperCase(this) ?: this
@@ -15,3 +17,7 @@ val currentDialectIfAvailableTest: DatabaseDialect? get() =
 
 inline fun <reified E : Enum<E>> enumSetOf(vararg elements: E): EnumSet<E> =
     elements.toCollection(EnumSet.noneOf(E::class.java))
+
+fun <T> Column<T>.constraintNamePart() = (currentDialectTest as? SQLServerDialect)?.let {
+    " CONSTRAINT DF_${table.tableName}_$name"
+} ?: ""

exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/CreateMissingTablesAndColumnsTests.kt

@@ -353,11 +353,11 @@ class CreateMissingTablesAndColumnsTests : DatabaseTestsBase() {
                     assertEquals(" ", whiteSpaceTable.select { whiteSpaceTable.id eq whiteSpaceId }.single()[whiteSpaceTable.column])
 
                     val actual = SchemaUtils.statementsRequiredToActualizeScheme(emptyTable)
-                    assertEquals(1, actual.size)
+                    val expected = if (testDb == TestDB.SQLSERVER) 2 else 1
+                    assertEquals(expected, actual.size)
 
-                    // SQL Server requires drop/create constraint to change defaults, unsupported for now
                     // Oracle treat '' as NULL column and can't alter from NULL to NULL
-                    if (testDb !in listOf(TestDB.SQLSERVER, TestDB.ORACLE)) {
+                    if (testDb != TestDB.ORACLE) {
                         // Apply changes
                         actual.forEach { exec(it) }
                     } else {
@@ -399,8 +399,8 @@ class CreateMissingTablesAndColumnsTests : DatabaseTestsBase() {
             override val primaryKey = PrimaryKey(id)
         }
 
-        val excludeSettings = listOf(TestDB.SQLITE, TestDB.SQLSERVER)
-        val complexAlterTable = listOf(TestDB.POSTGRESQL, TestDB.POSTGRESQLNG, TestDB.ORACLE, TestDB.H2_PSQL)
+        val excludeSettings = listOf(TestDB.SQLITE)
+        val complexAlterTable = listOf(TestDB.POSTGRESQL, TestDB.POSTGRESQLNG, TestDB.ORACLE, TestDB.H2_PSQL, TestDB.SQLSERVER)
         withDb(excludeSettings = excludeSettings) { testDb ->
             try {
                 SchemaUtils.createMissingTablesAndColumns(t1)