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.

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

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

Merged
merged 2 commits into from
Aug 11, 2023

Conversation

joc-a
Copy link
Collaborator

@joc-a joc-a commented Jul 31, 2023

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 (if it exists), and then add a new one.

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.

Example:

val t1 = object : Table("foo") {
    val id = integer("idcol")
    val col = integer("col")
    override val primaryKey = PrimaryKey(id)
}
val t2 = object : Table("foo") {
    val id = integer("idcol")
    val col = integer("COL").nullable().default(1)
    override val primaryKey = PrimaryKey(id)
}

withDb(excludeSettings = listOf(TestDB.SQLITE)) {
    try {
        SchemaUtils.createMissingTablesAndColumns(t1)
        val missingStatements = SchemaUtils.addMissingColumnsStatements(t2)
        println(missingStatements)
    } finally {
        SchemaUtils.drop(t1)
    }
}

Expected output: [ALTER TABLE foo ALTER COLUMN COL INT NULL, ALTER TABLE foo DROP CONSTRAINT IF EXISTS DF_foo_COL; ALTER TABLE foo ADD CONSTRAINT DF_foo_COL DEFAULT 1 for COL]

It's possible to put everything in a single String item without any commas or semicolons. This works:
exec("ALTER TABLE foo ALTER COLUMN COL INT NULL ALTER TABLE foo DROP CONSTRAINT IF EXISTS DF_foo_COL ALTER TABLE foo ADD CONSTRAINT DF_foo_COL DEFAULT 1 for COL")
But I thought it would be neater to separate them.

@joc-a joc-a force-pushed the joc/add-support-for-changing-default-value-in-sqlserver branch from affc799 to 6c1e4c1 Compare July 31, 2023 14:47
@joc-a joc-a changed the title feat: Add support for changing default value in SQL Server feat: EXPOSED-85 Add support for changing default value in SQL Server Aug 1, 2023
@joc-a joc-a force-pushed the joc/add-support-for-changing-default-value-in-sqlserver branch 3 times, most recently from de0c2d7 to 308528a Compare August 8, 2023 14:27
@joc-a joc-a requested review from bog-walk and e5l August 8, 2023 16:38
@joc-a joc-a marked this pull request as ready for review August 8, 2023 16:38
Copy link
Member

@bog-walk bog-walk left a comment

Choose a reason for hiding this comment

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

@joc-a The test file UnsignedColumnTypeTests uses Column.modifyStatement() a few times (which in turn uses modifyColumn(this, ColumnDiff.AllChanged)).

Could you confirm if those tests are now failing on SQL Server? The tests that use it may need to replace it with a raw exec("""ALTER TABLE ... ALTER COLUMN""").

Copy link
Member

@e5l e5l left a comment

Choose a reason for hiding this comment

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

lgtm

@joc-a joc-a force-pushed the joc/add-support-for-changing-default-value-in-sqlserver branch 2 times, most recently from ecf3c6a to 618c13d Compare August 9, 2023 22:02
@joc-a joc-a marked this pull request as draft August 10, 2023 08:42
@joc-a joc-a force-pushed the joc/add-support-for-changing-default-value-in-sqlserver branch from 618c13d to 5f5225a Compare August 10, 2023 09:08
Comment on lines +268 to +275
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")
}
Copy link
Collaborator Author

@joc-a joc-a Aug 10, 2023

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

@joc-a joc-a force-pushed the joc/add-support-for-changing-default-value-in-sqlserver branch 3 times, most recently from cad293d to 1533901 Compare August 10, 2023 12:11
@joc-a joc-a marked this pull request as ready for review August 10, 2023 12:54
@joc-a joc-a requested a review from bog-walk August 10, 2023 12:54
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.
…e 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.
@joc-a joc-a force-pushed the joc/add-support-for-changing-default-value-in-sqlserver branch from 1533901 to ed02776 Compare August 11, 2023 12:42
@joc-a joc-a merged commit 73dd153 into main Aug 11, 2023
3 checks passed
@joc-a joc-a deleted the joc/add-support-for-changing-default-value-in-sqlserver branch August 11, 2023 15:46
saral pushed a commit to saral/Exposed that referenced this pull request Oct 3, 2023
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants