Skip to content

Commit

Permalink
fix: EXPOSED-415 SchemaUtils incorrectly generates ALTER statements f… (
Browse files Browse the repository at this point in the history 
#2136)

* fix: EXPOSED-415 SchemaUtils incorrectly generates ALTER statements for existing nullable columns [MariaDB]
  • Loading branch information
@obabichevjb
obabichevjb authored Jul 10, 2024
1 parent 2028dd1 commit 94cfb88
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 17 deletions.
33 changes: 29 additions & 4 deletions exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/SchemaUtils.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ object SchemaUtils {
}

is MariaDBDialect -> processed.trim('\'')
is MysqlDialect -> "_utf8mb4\\'${processed.trim('(', ')', '\'')}\\"
is MysqlDialect -> "_utf8mb4\\'${processed.trim('(', ')', '\'')}\\'"
else -> processed.trim('\'')
}
}
Expand Down Expand Up @@ -316,10 +316,11 @@ object SchemaUtils {
val incorrectNullability = existingCol.nullable != columnType.nullable
// Exposed doesn't support changing sequences on columns
val incorrectAutoInc = existingCol.autoIncrement != columnType.isAutoInc && col.autoIncColumnType?.autoincSeq == null
val incorrectDefaults = existingCol.defaultDbValue != col.dbDefaultValue?.let {
dataTypeProvider.dbDefaultToString(col, it)
}

val incorrectDefaults = isIncorrectDefault(dataTypeProvider, existingCol, col)

val incorrectCaseSensitiveName = existingCol.name.inProperCase() != col.nameUnquoted().inProperCase()

ColumnDiff(incorrectNullability, incorrectAutoInc, incorrectDefaults, incorrectCaseSensitiveName)
}.filterValues { it.hasDifferences() }

Expand All @@ -343,6 +344,30 @@ object SchemaUtils {
return statements
}

/**
* For DDL purposes we do not segregate the cases when the default value was not specified, and when it
* was explicitly set to `null`.
*/
private fun isIncorrectDefault(dataTypeProvider: DataTypeProvider, columnMeta: ColumnMetadata, column: Column<*>): Boolean {
val isExistingColumnDefaultNull = columnMeta.defaultDbValue == null
val isDefinedColumnDefaultNull = column.dbDefaultValue == null ||
(column.dbDefaultValue is LiteralOp<*> && (column.dbDefaultValue as? LiteralOp<*>)?.value == null)

return when {
// Both values are null-like, no DDL update is needed
isExistingColumnDefaultNull && isDefinedColumnDefaultNull -> false
// Only one of the values is null-like, DDL update is needed
isExistingColumnDefaultNull != isDefinedColumnDefaultNull -> true

else -> {
val columnDefaultValue = column.dbDefaultValue?.let {
dataTypeProvider.dbDefaultToString(column, it)
}
columnMeta.defaultDbValue != columnDefaultValue
}
}
}

private fun addMissingColumnConstraints(vararg tables: Table, withLogs: Boolean): List<String> {
val existingColumnConstraint = logTimeSpent("Extracting column constraints", withLogs) {
currentDialect.columnConstraints(*tables)
Expand Down
65 changes: 52 additions & 13 deletions ...dbc/src/main/kotlin/org/jetbrains/exposed/sql/statements/jdbc/JdbcDatabaseMetadataImpl.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,27 +179,66 @@ class JdbcDatabaseMetadataImpl(database: String, val metadata: DatabaseMetaData)
)
}

private fun sanitizedDefault(defaultValue: String): String {
/**
* Here is the table of default values which are returned from the column `"COLUMN_DEF"` depending on how it was configured:
*
* - Not set: `varchar("any", 128).nullable()`
* - Set null: `varchar("any", 128).nullable().default(null)`
* - Set "NULL": `varchar("any", 128).nullable().default("NULL")`
* ```
* DB Not set Set null Set "NULL"
* SqlServer null "(NULL)" "('NULL')"
* SQLite null "NULL" "'NULL'"
* Postgres null "NULL::character varying" "'NULL'::character varying"
* PostgresNG null "NULL::character varying" "'NULL'::character varying"
* Oracle null "NULL " "'NULL' "
* MySql5 null null "NULL"
* MySql8 null null "NULL"
* MariaDB3 "NULL" "NULL" "'NULL'"
* MariaDB2 "NULL" "NULL" "'NULL'"
* H2V1 null "NULL" "'NULL'"
* H2V1 (MySql) null "NULL" "'NULL'"
* H2V2 null "NULL" "'NULL'"
* H2V2 (MySql) null "NULL" "'NULL'"
* H2V2 (MariaDB) null "NULL" "'NULL'"
* H2V2 (PSQL) null "NULL" "'NULL'"
* H2V2 (Oracle) null "NULL" "'NULL'"
* H2V2 (SqlServer) null "NULL" "'NULL'"
* ```
* According to this table there is no simple rule of what is the default value. It should be checked
* for each DB (or groups of DBs) specifically.
* In the case of MySql and MariaDB it's also not possible to say whether was default value skipped or
* explicitly set to `null`.
*
* @return `null` - if the value was set to `null` or not configured. `defaultValue` in other case.
*/
private fun sanitizedDefault(defaultValue: String): String? {
val dialect = currentDialect
val h2Mode = dialect.h2Mode
return when {
dialect is SQLServerDialect -> defaultValue.trim('(', ')', '\'')
dialect is OracleDialect || h2Mode == H2CompatibilityMode.Oracle -> defaultValue.trim().let {
if (it.startsWith('\'') && it.endsWith('\'')) it.trim('\'') else it
// Check for MariaDB must be before MySql because MariaDBDialect as a class inherits MysqlDialect
dialect is MariaDBDialect || h2Mode == H2CompatibilityMode.MariaDB -> when {
defaultValue.startsWith("b'") -> defaultValue.substringAfter("b'").trim('\'')
else -> defaultValue.extractNullAndStringFromDefaultValue()
}
dialect is MysqlDialect || h2Mode == H2CompatibilityMode.MySQL || h2Mode == H2CompatibilityMode.MariaDB -> defaultValue.substringAfter(
"b'"
).trim('\'')

dialect is PostgreSQLDialect || h2Mode == H2CompatibilityMode.PostgreSQL -> when {
defaultValue.startsWith('\'') && defaultValue.endsWith('\'') -> defaultValue.trim('\'')
else -> defaultValue
// It's the special case, because MySql returns default string "NULL" as string "NULL", but other DBs return it as "'NULL'"
dialect is MysqlDialect && defaultValue == "NULL" -> defaultValue
dialect is MysqlDialect || h2Mode == H2CompatibilityMode.MySQL -> when {
defaultValue.startsWith("b'") -> defaultValue.substringAfter("b'").trim('\'')
else -> defaultValue.extractNullAndStringFromDefaultValue()
}

else -> defaultValue.trim('\'')
dialect is SQLServerDialect -> defaultValue.trim('(', ')').extractNullAndStringFromDefaultValue()
dialect is OracleDialect -> defaultValue.trim().extractNullAndStringFromDefaultValue()
else -> defaultValue.extractNullAndStringFromDefaultValue()
}
}

private fun String.extractNullAndStringFromDefaultValue() = when {
this.startsWith("NULL") -> null
this.startsWith('\'') && this.endsWith('\'') -> this.trim('\'')
else -> this
}

private val existingIndicesCache = HashMap<Table, List<Index>>()

@Suppress("CyclomaticComplexMethod")
Expand Down
71 changes: 71 additions & 0 deletions ...t/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/CreateMissingTablesAndColumnsTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -726,4 +726,75 @@ class CreateMissingTablesAndColumnsTests : DatabaseTestsBase() {
}
}
}

@Test
fun testNoChangesOnCreateMissingNullableColumns() {
val testerWithDefaults = object : Table("tester") {
val defaultNullNumber = integer("default_null_number").nullable().default(null)
val defaultNullWord = varchar("default_null_word", 8).nullable().default(null)
val nullNumber = integer("null_number").nullable()
val nullWord = varchar("null_word", 8).nullable()
val defaultNumber = integer("default_number").default(999).nullable()
val defaultWord = varchar("default_word", 8).default("Hello").nullable()
}

val testerWithoutDefaults = object : Table("tester") {
val defaultNullNumber = integer("default_null_number").nullable()
val defaultNullWord = varchar("default_null_word", 8).nullable()
val nullNumber = integer("null_number").nullable()
val nullWord = varchar("null_word", 8).nullable()
val defaultNumber = integer("default_number").default(999).nullable()
val defaultWord = varchar("default_word", 8).default("Hello").nullable()
}

listOf(
testerWithDefaults to testerWithDefaults,
testerWithDefaults to testerWithoutDefaults,
testerWithoutDefaults to testerWithDefaults,
testerWithoutDefaults to testerWithoutDefaults
).forEach { (existingTable, definedTable) ->
withTables(existingTable) {
SchemaUtils.statementsRequiredToActualizeScheme(definedTable).also {
assertTrue(it.isEmpty())
}
}
}
}

@Test
fun testChangesOnCreateMissingNullableColumns() {
val testerWithDefaults = object : Table("tester") {
val defaultNullString = varchar("default_null_string", 8).nullable().default("NULL")
val defaultNumber = integer("default_number").default(999).nullable()
val defaultWord = varchar("default_word", 8).default("Hello").nullable()
}

val testerWithoutDefaults = object : Table("tester") {
val defaultNullString = varchar("default_null_string", 8).nullable()
val defaultNumber = integer("default_number").nullable()
val defaultWord = varchar("default_word", 8).nullable()
}

listOf(
testerWithDefaults to testerWithoutDefaults,
testerWithoutDefaults to testerWithDefaults,
).forEach { (existingTable, definedTable) ->
withTables(excludeSettings = listOf(TestDB.SQLITE), existingTable) {
SchemaUtils.statementsRequiredToActualizeScheme(definedTable).also {
assertTrue(it.isNotEmpty())
}
}
}

listOf(
testerWithDefaults to testerWithDefaults,
testerWithoutDefaults to testerWithoutDefaults
).forEach { (existingTable, definedTable) ->
withTables(excludeSettings = listOf(TestDB.SQLITE), existingTable) {
SchemaUtils.statementsRequiredToActualizeScheme(definedTable).also {
assertTrue(it.isEmpty())
}
}
}
}
}

0 comments on commit 94cfb88

Please sign in to comment.