From e2bf0c361cdcf66731251e338b91528599ddfb28 Mon Sep 17 00:00:00 2001 From: Jocelyne Date: Mon, 23 Sep 2024 14:08:29 +0200 Subject: [PATCH] feat: EXPOSED-552 Include DROP statements for unmapped columns for migration --- exposed-core/api/exposed-core.api | 2 + .../org/jetbrains/exposed/sql/Database.kt | 5 ++ .../statements/api/ExposedDatabaseMetadata.kt | 3 + exposed-jdbc/api/exposed-jdbc.api | 1 + .../jdbc/JdbcDatabaseMetadataImpl.kt | 1 + exposed-migration/api/exposed-migration.api | 2 + .../src/main/kotlin/MigrationUtils.kt | 36 ++++++++++- .../shared/ddl/DatabaseMigrationTests.kt | 60 +++++++++++++++++++ 8 files changed, 109 insertions(+), 1 deletion(-) diff --git a/exposed-core/api/exposed-core.api b/exposed-core/api/exposed-core.api index 3a2d6f3c2a..f8d9e9b781 100644 --- a/exposed-core/api/exposed-core.api +++ b/exposed-core/api/exposed-core.api @@ -664,6 +664,7 @@ public final class org/jetbrains/exposed/sql/Database { public final fun getDialect ()Lorg/jetbrains/exposed/sql/vendors/DatabaseDialect; public final fun getIdentifierManager ()Lorg/jetbrains/exposed/sql/statements/api/IdentifierManagerApi; public final fun getSupportsAlterTableWithAddColumn ()Z + public final fun getSupportsAlterTableWithDropColumn ()Z public final fun getSupportsMultipleResultSets ()Z public final fun getUrl ()Ljava/lang/String; public final fun getUseNestedTransactions ()Z @@ -3508,6 +3509,7 @@ public abstract class org/jetbrains/exposed/sql/statements/api/ExposedDatabaseMe public abstract fun getIdentifierManager ()Lorg/jetbrains/exposed/sql/statements/api/IdentifierManagerApi; public abstract fun getSchemaNames ()Ljava/util/List; public abstract fun getSupportsAlterTableWithAddColumn ()Z + public abstract fun getSupportsAlterTableWithDropColumn ()Z public abstract fun getSupportsMultipleResultSets ()Z public abstract fun getSupportsSelectForUpdate ()Z public abstract fun getTableNames ()Ljava/util/Map; diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Database.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Database.kt index ce5496e6ad..d55c7dae28 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Database.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Database.kt @@ -69,6 +69,11 @@ class Database private constructor( LazyThreadSafetyMode.NONE ) { metadata { supportsAlterTableWithAddColumn } } + /** Whether the database supports ALTER TABLE with a drop column clause. */ + val supportsAlterTableWithDropColumn by lazy( + LazyThreadSafetyMode.NONE + ) { metadata { supportsAlterTableWithDropColumn } } + /** Whether the database supports getting multiple result sets from a single execute. */ val supportsMultipleResultSets by lazy(LazyThreadSafetyMode.NONE) { metadata { supportsMultipleResultSets } } diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/api/ExposedDatabaseMetadata.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/api/ExposedDatabaseMetadata.kt index 117d3d64d4..78271a0e70 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/api/ExposedDatabaseMetadata.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/api/ExposedDatabaseMetadata.kt @@ -30,6 +30,9 @@ abstract class ExposedDatabaseMetadata(val database: String) { /** Whether the database supports `ALTER TABLE` with an add column clause. */ abstract val supportsAlterTableWithAddColumn: Boolean + /** Whether the database supports `ALTER TABLE` with a drop column clause. */ + abstract val supportsAlterTableWithDropColumn: Boolean + /** Whether the database supports getting multiple result sets from a single execute. */ abstract val supportsMultipleResultSets: Boolean diff --git a/exposed-jdbc/api/exposed-jdbc.api b/exposed-jdbc/api/exposed-jdbc.api index 9e62e8b9fa..5ee212d776 100644 --- a/exposed-jdbc/api/exposed-jdbc.api +++ b/exposed-jdbc/api/exposed-jdbc.api @@ -45,6 +45,7 @@ public final class org/jetbrains/exposed/sql/statements/jdbc/JdbcDatabaseMetadat public final fun getMetadata ()Ljava/sql/DatabaseMetaData; public fun getSchemaNames ()Ljava/util/List; public fun getSupportsAlterTableWithAddColumn ()Z + public fun getSupportsAlterTableWithDropColumn ()Z public fun getSupportsMultipleResultSets ()Z public fun getSupportsSelectForUpdate ()Z public fun getTableNames ()Ljava/util/Map; diff --git a/exposed-jdbc/src/main/kotlin/org/jetbrains/exposed/sql/statements/jdbc/JdbcDatabaseMetadataImpl.kt b/exposed-jdbc/src/main/kotlin/org/jetbrains/exposed/sql/statements/jdbc/JdbcDatabaseMetadataImpl.kt index adbc74826e..e8d8a7d34b 100644 --- a/exposed-jdbc/src/main/kotlin/org/jetbrains/exposed/sql/statements/jdbc/JdbcDatabaseMetadataImpl.kt +++ b/exposed-jdbc/src/main/kotlin/org/jetbrains/exposed/sql/statements/jdbc/JdbcDatabaseMetadataImpl.kt @@ -50,6 +50,7 @@ class JdbcDatabaseMetadataImpl(database: String, val metadata: DatabaseMetaData) override val defaultIsolationLevel: Int by lazyMetadata { defaultTransactionIsolation } override val supportsAlterTableWithAddColumn by lazyMetadata { supportsAlterTableWithAddColumn() } + override val supportsAlterTableWithDropColumn by lazyMetadata { supportsAlterTableWithDropColumn() } override val supportsMultipleResultSets by lazyMetadata { supportsMultipleResultSets() } override val supportsSelectForUpdate: Boolean by lazyMetadata { supportsSelectForUpdate() } diff --git a/exposed-migration/api/exposed-migration.api b/exposed-migration/api/exposed-migration.api index d351ae4656..7b8e5deceb 100644 --- a/exposed-migration/api/exposed-migration.api +++ b/exposed-migration/api/exposed-migration.api @@ -1,5 +1,7 @@ public final class MigrationUtils { public static final field INSTANCE LMigrationUtils; + public final fun dropUnmappedColumnsStatements ([Lorg/jetbrains/exposed/sql/Table;Z)Ljava/util/List; + public static synthetic fun dropUnmappedColumnsStatements$default (LMigrationUtils;[Lorg/jetbrains/exposed/sql/Table;ZILjava/lang/Object;)Ljava/util/List; public final fun generateMigrationScript ([Lorg/jetbrains/exposed/sql/Table;Ljava/lang/String;Ljava/lang/String;Z)Ljava/io/File; public static synthetic fun generateMigrationScript$default (LMigrationUtils;[Lorg/jetbrains/exposed/sql/Table;Ljava/lang/String;Ljava/lang/String;ZILjava/lang/Object;)Ljava/io/File; public final fun statementsRequiredForDatabaseMigration ([Lorg/jetbrains/exposed/sql/Table;Z)Ljava/util/List; diff --git a/exposed-migration/src/main/kotlin/MigrationUtils.kt b/exposed-migration/src/main/kotlin/MigrationUtils.kt index 82b8783fa4..4ad4110a6b 100644 --- a/exposed-migration/src/main/kotlin/MigrationUtils.kt +++ b/exposed-migration/src/main/kotlin/MigrationUtils.kt @@ -76,7 +76,8 @@ object MigrationUtils { checkMissingSequences(tables = tables, withLogs).flatMap { it.createStatement() } } val alterStatements = logTimeSpent("Preparing alter table statements", withLogs) { - addMissingColumnsStatements(tables = tablesToAlter.toTypedArray(), withLogs) + addMissingColumnsStatements(tables = tablesToAlter.toTypedArray(), withLogs) + + dropUnmappedColumnsStatements(tables = tablesToAlter.toTypedArray(), withLogs) } val modifyTablesStatements = logTimeSpent("Checking mapping consistence", withLogs) { @@ -90,6 +91,39 @@ object MigrationUtils { return allStatements } + fun dropUnmappedColumnsStatements(vararg tables: Table, withLogs: Boolean = true): List { + if (tables.isEmpty()) return emptyList() + + val statements = mutableListOf() + + val dbSupportsAlterTableWithDropColumn = TransactionManager.current().db.supportsAlterTableWithDropColumn + + if (dbSupportsAlterTableWithDropColumn) { + val existingTablesColumns = logTimeSpent("Extracting table columns", withLogs) { + currentDialect.tableColumns(*tables) + } + + val tr = TransactionManager.current() + + tables.forEach { table -> + val existingColumns = existingTablesColumns[table].orEmpty().toSet() + val tableColumns = table.columns.toSet() + val mappedColumns = existingColumns.mapNotNull { columnMetadata -> + val mappedCol = tableColumns.find { column -> columnMetadata.name.equals(column.nameUnquoted(), ignoreCase = true) } + if (mappedCol != null) columnMetadata else null + }.toSet() + val unmappedColumns = existingColumns.subtract(mappedColumns) + unmappedColumns.forEach { + statements.add( + "ALTER TABLE ${tr.identity(table)} DROP COLUMN ${tr.db.identifierManager.quoteIdentifierWhenWrongCaseOrNecessary(it.name)}" + ) + } + } + } + + return statements + } + /** * Log Exposed table mappings <-> real database mapping problems and returns DDL Statements to fix them, including * DROP/DELETE statements (unlike [checkMappingConsistence]) diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/DatabaseMigrationTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/DatabaseMigrationTests.kt index 4037eea8ee..1889536ad3 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/DatabaseMigrationTests.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/DatabaseMigrationTests.kt @@ -12,6 +12,7 @@ import org.jetbrains.exposed.sql.tests.DatabaseTestsBase import org.jetbrains.exposed.sql.tests.TestDB import org.jetbrains.exposed.sql.tests.currentDialectTest 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.assertTrue @@ -132,6 +133,65 @@ class DatabaseMigrationTests : DatabaseTestsBase() { } } + @Test + fun testDropUnmappedColumnsStatementsIdentical() { + val t1 = object : Table("foo") { + val id = integer("idcol") + + override val primaryKey = PrimaryKey(id) + } + + val t2 = object : Table("foo") { + val id = integer("idcol") + + override val primaryKey = PrimaryKey(id) + } + + withTables(t1) { + val statements = MigrationUtils.dropUnmappedColumnsStatements(t2, withLogs = false) + assertEqualCollections(statements, emptyList()) + } + } + + @Test + fun testDropUnmappedColumnsStatementsIdentical2() { + val t1 = object : Table("foo") { + val id = integer("idCol") + + override val primaryKey = PrimaryKey(id) + } + + val t2 = object : Table("foo") { + val id = integer("idCol") + + override val primaryKey = PrimaryKey(id) + } + + withTables(t1) { + val statements = MigrationUtils.dropUnmappedColumnsStatements(t2, withLogs = false) + assertEqualCollections(statements, emptyList()) + } + } + + @Test + fun testDropUnmappedColumns() { + val t1 = object : Table("foo") { + val id = integer("id") + val name = text("name") + } + + val t2 = object : Table("foo") { + val id = integer("id") + } + + withTables(excludeSettings = listOf(TestDB.SQLITE, TestDB.ORACLE), t1) { + assertEqualCollections(MigrationUtils.statementsRequiredForDatabaseMigration(t1, withLogs = false), emptyList()) + + val statements = MigrationUtils.statementsRequiredForDatabaseMigration(t2, withLogs = false) + assertEquals(1, statements.size) + } + } + @Test fun testAddNewPrimaryKeyOnExistingColumn() { val tableName = "tester"