feat: EXPOSED-552 Include DROP statements for unmapped columns for migration #2249
Conversation
joc-a
force-pushed
the
joc/exposed-552
branch
from
e2bf0c3
to
b518b76
Compare
| statements.add( | ||
| "ALTER TABLE ${tr.identity(table)} DROP COLUMN ${tr.db.identifierManager.quoteIdentifierWhenWrongCaseOrNecessary(it.name)}" | ||
| ) |
There was a problem hiding this comment.
From what I'm seeing, this is what using Column.dropStatement().single() directly returns, no?
...d-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/DatabaseMigrationTests.kt
Outdated
Show resolved
Hide resolved
| val existingTablesColumns = logTimeSpent("Extracting table columns", withLogs) { | ||
| currentDialect.tableColumns(*tables) | ||
| } |
There was a problem hiding this comment.
This is also always called in addMissingColumnsStatements() directly before with the same arguments, right? It may be inefficient to do this twice given the potential queries involved. I get that it must be possible in isolation because you've made this method public, so the metadata needs to be retrieved if this is invoked alone.
But maybe you could log an issue for us to later restructure the calls inside statementsRequiredForDatabaseMigration()? As in maybe we could have a single invokation to retrieve and store existingTablesColumns there. Then we could either have private overloads for addMissingColumnsStatements and dropUnmappedColumnsStatements or just default parameters that allow us to pass in the column data results & avoid duplicate queries.
joc-a
force-pushed
the
joc/exposed-552
branch
from
b518b76
to
a2c2bc2
Compare
Description
MigrationUtils.statementsRequiredForDatabaseMigrationnow includes DROP statements for unmapped columns (those that exist in the database but not in the Exposed Kotlin code).Detailed description:
Necessary for migration.
A new function
dropUnmappedColumnsStatementswas added toMigrationUtils. This function returns the SQL statements that drop any columns that exist in the database but are not defined in the Exposed tables. This function is then invoked instatementsRequiredForDatabaseMigration.Type of Change
Please mark the relevant options with an "X":
Updates/remove existing public API methods:
Affected databases:
Checklist
Related Issues