Skip to content

Commit

Permalink
fix: EXPOSED-696 [PostgreSQL] Drop of auto-increment sequence fails a…
Browse files Browse the repository at this point in the history
…fter column modified without dropping default (#2369)

DDL generated by MigrationUtils when a column is modified to lose its auto-increment
status was not dropping the column's default. This meant that attemtping to drop
the auto-increment status after would result in an exception because the sequence
was still being used by the altered column.

This ensures that the column is altered to drop its default if the existing database
column is auto-incrementing while the defined column isn't.
  • Loading branch information
bog-walk authored Jan 28, 2025
1 parent f6da63f commit e622c7c
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@ package org.jetbrains.exposed.sql
* Represents differences between a column definition and database metadata for the existing column.
*/
data class ColumnDiff(
/** Whether the nullability of the existing column is correct. */
/** Whether there is a mismatch between nullability of the existing column and the defined column. */
val nullability: Boolean,
/** Whether the existing column has a matching auto-increment sequence. */
/** Whether there is a mismatch between auto-increment status of the existing column and the defined column. */
val autoInc: Boolean,
/** Whether the default value of the existing column is correct. */
/** Whether the default value of the existing column matches that of the defined column. */
val defaults: Boolean,
/** Whether the existing column identifier matches and has the correct casing. */
/** Whether the existing column identifier matches that of the defined column and has the correct casing. */
val caseSensitiveName: Boolean,
/** Whether the size and scale of the existing column, if applicable, is correct. */
/** Whether the size and scale of the existing column, if applicable, match those of the defined column. */
val sizeAndScale: Boolean,
) {
/** Returns `true` if there is a difference between the column definition and the existing column in the database. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,11 @@ open class PostgreSQLDialect(override val name: String = dialectName) : VendorDi
val fallbackSequenceName = fallbackSequenceName(tableName = column.table.tableName, columnName = column.name)
append("ALTER COLUMN $colName SET DEFAULT nextval('$fallbackSequenceName')")
}
} else if (columnDiff.autoInc && column.autoIncColumnType == null) {
// based on logic in SchemaUtils.isIncorrectAutoInc this should only be possible if the existing
// column in database is auto-incrementing while defined table is not
append("ALTER COLUMN $colName TYPE ${column.columnType.sqlType()}")
append(", ALTER COLUMN $colName DROP DEFAULT")
} else {
append("ALTER COLUMN $colName TYPE ${column.columnType.sqlType()}")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import org.jetbrains.exposed.sql.tests.inProperCase
import org.jetbrains.exposed.sql.tests.shared.assertEqualCollections
import org.jetbrains.exposed.sql.tests.shared.assertEqualLists
import org.jetbrains.exposed.sql.tests.shared.assertEquals
import org.jetbrains.exposed.sql.tests.shared.assertFalse
import org.jetbrains.exposed.sql.tests.shared.assertTrue
import org.jetbrains.exposed.sql.tests.shared.expectException
import org.jetbrains.exposed.sql.vendors.MysqlDialect
Expand Down Expand Up @@ -374,7 +375,7 @@ class DatabaseMigrationTests : DatabaseTestsBase() {
when (testDb) {
TestDB.POSTGRESQL, TestDB.POSTGRESQLNG -> {
assertEquals(2, statements.size)
assertEquals("ALTER TABLE test_table ALTER COLUMN id TYPE BIGINT", statements[0])
assertEquals("ALTER TABLE test_table ALTER COLUMN id TYPE BIGINT, ALTER COLUMN id DROP DEFAULT", statements[0])
assertEquals(expectedDropSequenceStatement("test_table_id_seq"), statements[1])
}
TestDB.ORACLE, TestDB.H2_V2_ORACLE -> {
Expand Down Expand Up @@ -731,15 +732,13 @@ class DatabaseMigrationTests : DatabaseTestsBase() {

val statements = MigrationUtils.statementsRequiredForDatabaseMigration(tableWithoutAutoIncrement)
assertEquals(2, statements.size)
assertEquals("ALTER TABLE test_table_auto ALTER COLUMN id TYPE INT", statements[0])
assertEquals("ALTER TABLE test_table_auto ALTER COLUMN id TYPE INT, ALTER COLUMN id DROP DEFAULT", statements[0])
assertEquals(expectedDropSequenceStatement("test_table_auto_id_seq"), statements[1])

// fails due to EXPOSED-696
// https://youtrack.jetbrains.com/issue/EXPOSED-696/PostgreSQL-Drop-of-auto-increment-sequence-fails-after-column-modified-without-dropping-default)
// statements.forEach { exec(it) }
// assertFalse(autoSeq.exists())
// assertTrue(sequence.exists())
// assertTrue(implicitSeq.exists())
statements.forEach { exec(it) }
assertFalse(autoSeq.exists())
assertTrue(sequence.exists())
assertTrue(implicitSeq.exists())
} finally {
SchemaUtils.drop(tableWithAutoIncrement, tableWithExplSequence, tableWithImplSequence)
}
Expand Down

0 comments on commit e622c7c

Please sign in to comment.