Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: EXPOSED-85 Add support for changing default value in SQL Server #1812

Merged
merged 2 commits into from
Aug 11, 2023
Merged

feat: EXPOSED-85 Add support for changing default value in SQL Server #1812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66f7d65
feat: Add support for changing default value in SQL Server
joc-a Jul 31, 2023
ed02776
fix: Change the way modifyColumn constructs its statements because th…
joc-a Aug 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Column.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
}
}
}

Expand Down
60 changes: 56 additions & 4 deletions exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/SQLServerDialect.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Comment on lines +266 to +275
Copy link
Collaborator Author

@joc-a joc-a Aug 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic was copied from Column.descriptionDdl function, because this is what modifyColumn in SQLServerDialect was using before, when it was invoking super.modifyColumn:

override fun modifyColumn(column: Column<*>, columnDiff: ColumnDiff): List<String> =
        listOf("ALTER TABLE ${TransactionManager.current().identity(column.table)} MODIFY COLUMN ${column.descriptionDdl(true)}")

}
}
)

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()}"

Expand Down Expand Up @@ -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"
}

Expand Down
33 changes: 17 additions & 16 deletions exposed-java-time/src/test/kotlin/org/jetbrains/exposed/DefaultsTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 ||
Expand Down
21 changes: 11 additions & 10 deletions exposed-jodatime/src/test/kotlin/org/jetbrains/exposed/JodaTimeDefaultsTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 ||
Expand Down
33 changes: 17 additions & 16 deletions ...kotlin-datetime/src/test/kotlin/org/jetbrains/exposed/sql/kotlin/datetime/DefaultsTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 ||
Expand Down
6 changes: 6 additions & 0 deletions exposed-tests/src/main/kotlin/org/jetbrains/exposed/sql/tests/TestUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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"
} ?: ""
10 changes: 5 additions & 5 deletions ...t/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/CreateMissingTablesAndColumnsTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
Loading