From c3b998dd1c2dc0a953e4a31fe18e1f2bd9c78291 Mon Sep 17 00:00:00 2001 From: Alexey Soshin Date: Mon, 4 Sep 2023 11:01:09 +0100 Subject: [PATCH] Fix how changes are calculated for non-default schema table (#1678) Allow schema-prefixed table names to be used with SchemaUtils validating functions. Ensure tables with index and foreign key names are also covered when changes are assessed. Co-authored-by: Chantal Loncle <82039410+bog-walk@users.noreply.github.com> --- exposed-core/api/exposed-core.api | 1 + .../org/jetbrains/exposed/sql/Constraints.kt | 10 +-- .../kotlin/org/jetbrains/exposed/sql/Table.kt | 13 ++-- .../jetbrains/exposed/sql/vendors/Default.kt | 6 +- .../jetbrains/exposed/sql/vendors/Mysql.kt | 10 +-- .../jdbc/JdbcDatabaseMetadataImpl.kt | 76 +++++++++++++------ .../ddl/CreateMissingTablesAndColumnsTests.kt | 48 ++++++++++-- 7 files changed, 118 insertions(+), 46 deletions(-) diff --git a/exposed-core/api/exposed-core.api b/exposed-core/api/exposed-core.api index ded68426f0..6567dd054b 100644 --- a/exposed-core/api/exposed-core.api +++ b/exposed-core/api/exposed-core.api @@ -2240,6 +2240,7 @@ public class org/jetbrains/exposed/sql/Table : org/jetbrains/exposed/sql/ColumnS public final fun getForeignKeys ()Ljava/util/List; public final fun getIndices ()Ljava/util/List; public fun getPrimaryKey ()Lorg/jetbrains/exposed/sql/Table$PrimaryKey; + public final fun getSchemaName ()Ljava/lang/String; public fun getTableName ()Ljava/lang/String; public fun hashCode ()I public final fun index (Ljava/lang/String;Z[Lorg/jetbrains/exposed/sql/Column;Ljava/util/List;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)V diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Constraints.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Constraints.kt index c8f6d54a4d..9db3220733 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Constraints.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Constraints.kt @@ -257,17 +257,17 @@ data class Index( /** Name of the index. */ val indexName: String get() = customName ?: buildString { - append(table.nameInDatabaseCase()) + append(table.nameInDatabaseCaseUnquoted()) append('_') - append(columns.joinToString("_") { it.name }.inProperCase()) + append(columns.joinToString("_") { it.name }) functions?.let { f -> if (columns.isNotEmpty()) append('_') - append(f.joinToString("_") { it.toString().substringBefore("(").lowercase() }.inProperCase()) + append(f.joinToString("_") { it.toString().substringBefore("(").lowercase() }) } if (unique) { - append("_unique".inProperCase()) + append("_unique") } - } + }.inProperCase() init { require(columns.isNotEmpty() || functions?.isNotEmpty() == true) { "At least one column or function is required to create an index" } diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Table.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Table.kt index 7fd2c33a99..115ce24216 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Table.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Table.kt @@ -335,7 +335,10 @@ open class Table(name: String = "") : ColumnSet(), DdlAware { else -> javaClass.name.removePrefix("${javaClass.`package`.name}.").substringAfter('$').removeSuffix("Table") } - internal val tableNameWithoutScheme: String get() = tableName.substringAfter(".") + /** Returns the schema name, or null if one does not exist for this table. */ + val schemaName: String? = if (name.contains(".")) name.substringBeforeLast(".") else null + + internal val tableNameWithoutScheme: String get() = tableName.substringAfterLast(".") // Table name may contain quotes, remove those before appending internal val tableNameWithoutSchemeSanitized: String get() = tableNameWithoutScheme.replace("\"", "").replace("'", "") @@ -369,15 +372,15 @@ open class Table(name: String = "") : ColumnSet(), DdlAware { fun nameInDatabaseCase(): String = tableName.inProperCase() /** - * Returns the table name, in proper case, with wrapping single- and double-quotation characters removed. + * Returns the table name, without schema and in proper case, with wrapping single- and double-quotation characters removed. * - * **Note** If used with MySQL or MariaDB, the column name is returned unchanged, since these databases use a + * **Note** If used with MySQL or MariaDB, the table name is returned unchanged, since these databases use a * backtick character as the identifier quotation. */ fun nameInDatabaseCaseUnquoted(): String = if (currentDialect is MysqlDialect) { - nameInDatabaseCase() + tableNameWithoutScheme.inProperCase() } else { - nameInDatabaseCase().trim('\"', '\'') + tableNameWithoutScheme.inProperCase().trim('\"', '\'') } override fun describe(s: Transaction, queryBuilder: QueryBuilder): Unit = queryBuilder { append(s.identity(this@Table)) } diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Default.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Default.kt index 0b08d6e386..d67bb8ed53 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Default.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Default.kt @@ -1139,7 +1139,7 @@ abstract class VendorDialect( } override fun tableExists(table: Table): Boolean { - val tableScheme = table.tableName.substringBefore('.', "").takeIf { it.isNotEmpty() } + val tableScheme = table.schemaName val scheme = tableScheme?.inProperCase() ?: TransactionManager.current().connection.metadata { currentScheme } val allTables = getAllTableNamesCache().getValue(scheme) return allTables.any { @@ -1171,11 +1171,11 @@ abstract class VendorDialect( ): Map>>, List> { val constraints = HashMap>>, MutableList>() - val tablesToLoad = tables.filter { !columnConstraintsCache.containsKey(it.nameInDatabaseCase()) } + val tablesToLoad = tables.filter { !columnConstraintsCache.containsKey(it.nameInDatabaseCaseUnquoted()) } fillConstraintCacheForTables(tablesToLoad) tables.forEach { table -> - columnConstraintsCache[table.nameInDatabaseCase()].orEmpty().forEach { + columnConstraintsCache[table.nameInDatabaseCaseUnquoted()].orEmpty().forEach { constraints.getOrPut(table to it.from) { arrayListOf() }.add(it) } } diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Mysql.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Mysql.kt index 0077e545bb..8d9cd8af62 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Mysql.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Mysql.kt @@ -294,11 +294,11 @@ open class MysqlDialect : VendorDialect(dialectName, MysqlDataTypeProvider, Mysq } override fun fillConstraintCacheForTables(tables: List) { - val allTables = SchemaUtils.sortTablesByReferences(tables).associateBy { it.nameInDatabaseCase() } + val allTables = SchemaUtils.sortTablesByReferences(tables).associateBy { it.nameInDatabaseCaseUnquoted() } val allTableNames = allTables.keys val inTableList = allTableNames.joinToString("','", prefix = " ku.TABLE_NAME IN ('", postfix = "')") val tr = TransactionManager.current() - val schemaName = "'${getDatabase()}'" + val tableSchema = "'${tables.mapNotNull { it.schemaName }.toSet().singleOrNull() ?: getDatabase()}'" val constraintsToLoad = HashMap>() tr.exec( """SELECT @@ -312,9 +312,9 @@ open class MysqlDialect : VendorDialect(dialectName, MysqlDataTypeProvider, Mysq FROM INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS rc INNER JOIN INFORMATION_SCHEMA.KEY_COLUMN_USAGE ku ON ku.TABLE_SCHEMA = rc.CONSTRAINT_SCHEMA AND rc.CONSTRAINT_NAME = ku.CONSTRAINT_NAME - WHERE ku.TABLE_SCHEMA = $schemaName - AND ku.CONSTRAINT_SCHEMA = $schemaName - AND rc.CONSTRAINT_SCHEMA = $schemaName + WHERE ku.TABLE_SCHEMA = $tableSchema + AND ku.CONSTRAINT_SCHEMA = $tableSchema + AND rc.CONSTRAINT_SCHEMA = $tableSchema AND $inTableList ORDER BY ku.ORDINAL_POSITION """.trimIndent() 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 6cead460ee..13b69821c9 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 @@ -146,25 +146,32 @@ class JdbcDatabaseMetadataImpl(database: String, val metadata: DatabaseMetaData) } override fun columns(vararg tables: Table): Map> { - val rs = metadata.getColumns(databaseName, currentScheme, "%", "%") - val result = rs.extractColumns(tables) { - // @see java.sql.DatabaseMetaData.getColumns - // That read should go first as Oracle driver closes connection after that - val defaultDbValue = it.getString("COLUMN_DEF")?.let { sanitizedDefault(it) } - val autoIncrement = it.getString("IS_AUTOINCREMENT") == "YES" - val type = it.getInt("DATA_TYPE") - val columnMetadata = ColumnMetadata( - it.getString("COLUMN_NAME"), - type, - it.getBoolean("NULLABLE"), - it.getInt("COLUMN_SIZE").takeIf { it != 0 }, - autoIncrement, - // Not sure this filters enough but I dont think we ever want to have sequences here - defaultDbValue?.takeIf { !autoIncrement }, - ) - it.getString("TABLE_NAME") to columnMetadata + val result = mutableMapOf>() + val useSchemaInsteadOfDatabase = currentDialect is MysqlDialect + + val tablesBySchema = tables.groupBy { identifierManager.inProperCase(it.schemaName ?: currentScheme) } + tablesBySchema.forEach { (schema, schemaTables) -> + val catalog = if (!useSchemaInsteadOfDatabase || schema == currentScheme) databaseName else schema + val rs = metadata.getColumns(catalog, schema, "%", "%") + result += rs.extractColumns(schemaTables.toTypedArray()) { + // @see java.sql.DatabaseMetaData.getColumns + // That read should go first as Oracle driver closes connection after that + val defaultDbValue = it.getString("COLUMN_DEF")?.let { sanitizedDefault(it) } + val autoIncrement = it.getString("IS_AUTOINCREMENT") == "YES" + val type = it.getInt("DATA_TYPE") + val columnMetadata = ColumnMetadata( + it.getString("COLUMN_NAME"), + type, + it.getBoolean("NULLABLE"), + it.getInt("COLUMN_SIZE").takeIf { it != 0 }, + autoIncrement, + // Not sure this filters enough but I dont think we ever want to have sequences here + defaultDbValue?.takeIf { !autoIncrement }, + ) + it.getString("TABLE_NAME") to columnMetadata + } + rs.close() } - rs.close() return result } @@ -188,12 +195,14 @@ class JdbcDatabaseMetadataImpl(database: String, val metadata: DatabaseMetaData) private val existingIndicesCache = HashMap>() + @Suppress("CyclomaticComplexMethod") override fun existingIndices(vararg tables: Table): Map> { for (table in tables) { val transaction = TransactionManager.current() + val (catalog, tableSchema) = tableCatalogAndSchema(table) existingIndicesCache.getOrPut(table) { - val pkNames = metadata.getPrimaryKeys(databaseName, currentScheme, table.nameInDatabaseCaseUnquoted()).let { rs -> + val pkNames = metadata.getPrimaryKeys(catalog, tableSchema, table.nameInDatabaseCaseUnquoted()).let { rs -> val names = arrayListOf() while (rs.next()) { rs.getString("PK_NAME")?.let { names += it } @@ -201,7 +210,8 @@ class JdbcDatabaseMetadataImpl(database: String, val metadata: DatabaseMetaData) rs.close() names } - val rs = metadata.getIndexInfo(databaseName, currentScheme, table.nameInDatabaseCase(), false, false) + val storedIndexTable = if (tableSchema == currentScheme) table.nameInDatabaseCase() else table.nameInDatabaseCaseUnquoted() + val rs = metadata.getIndexInfo(catalog, tableSchema, storedIndexTable, false, false) val tmpIndices = hashMapOf, MutableList>() @@ -243,7 +253,8 @@ class JdbcDatabaseMetadataImpl(database: String, val metadata: DatabaseMetaData) override fun existingPrimaryKeys(vararg tables: Table): Map { return tables.associateWith { table -> - metadata.getPrimaryKeys(databaseName, currentScheme, table.nameInDatabaseCaseUnquoted()).let { rs -> + val (catalog, tableSchema) = tableCatalogAndSchema(table) + metadata.getPrimaryKeys(catalog, tableSchema, table.nameInDatabaseCaseUnquoted()).let { rs -> val columnNames = mutableListOf() var pkName = "" while (rs.next()) { @@ -258,9 +269,10 @@ class JdbcDatabaseMetadataImpl(database: String, val metadata: DatabaseMetaData) @Synchronized override fun tableConstraints(tables: List
): Map> { - val allTables = SchemaUtils.sortTablesByReferences(tables).associateBy { it.nameInDatabaseCase() } + val allTables = SchemaUtils.sortTablesByReferences(tables).associateBy { it.nameInDatabaseCaseUnquoted() } return allTables.keys.associateWith { table -> - metadata.getImportedKeys(databaseName, currentScheme, table).iterate { + val (catalog, tableSchema) = tableCatalogAndSchema(allTables[table]!!) + metadata.getImportedKeys(catalog, identifierManager.inProperCase(tableSchema), table).iterate { val fromTableName = getString("FKTABLE_NAME")!! val fromColumnName = identifierManager.quoteIdentifierWhenWrongCaseOrNecessary(getString("FKCOLUMN_NAME")!!) val fromColumn = allTables[fromTableName]?.columns?.firstOrNull { @@ -291,6 +303,24 @@ class JdbcDatabaseMetadataImpl(database: String, val metadata: DatabaseMetaData) } } + /** + * Returns the name of the database in which a [table] is found, as well as it's schema name. + * + * If the table name does not include a schema prefix, the metadata value `currentScheme` is used instead. + * + * MySQL/MariaDB are special cases in that a schema definition is treated like a separate database. This means that + * a connection to 'testDb' with a table defined as 'my_schema.my_table' will only successfully find the table's + * metadata if 'my_schema' is used as the database name. + */ + private fun tableCatalogAndSchema(table: Table): Pair { + val tableSchema = identifierManager.inProperCase(table.schemaName ?: currentScheme) + return if (currentDialect is MysqlDialect && tableSchema != currentScheme) { + tableSchema to tableSchema + } else { + databaseName to tableSchema + } + } + @Synchronized override fun cleanCache() { existingIndicesCache.clear() diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/CreateMissingTablesAndColumnsTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/CreateMissingTablesAndColumnsTests.kt index 7d3f7be669..65db329aa9 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/CreateMissingTablesAndColumnsTests.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/CreateMissingTablesAndColumnsTests.kt @@ -3,6 +3,7 @@ package org.jetbrains.exposed.sql.tests.shared.ddl import org.jetbrains.exposed.dao.id.EntityID import org.jetbrains.exposed.dao.id.IdTable import org.jetbrains.exposed.dao.id.IntIdTable +import org.jetbrains.exposed.dao.id.LongIdTable import org.jetbrains.exposed.sql.* import org.jetbrains.exposed.sql.SqlExpressionBuilder.isNull import org.jetbrains.exposed.sql.tests.DatabaseTestsBase @@ -294,7 +295,7 @@ class CreateMissingTablesAndColumnsTests : DatabaseTestsBase() { withDb { testDb -> try { // MySQL doesn't support default values on text columns, hence excluded - table = if(testDb != TestDB.MYSQL) { + table = if (testDb != TestDB.MYSQL) { object : Table("varchar_test") { val varchar = varchar("varchar_column", 255).default(" ") val text = text("text_column").default(" ") @@ -328,7 +329,7 @@ class CreateMissingTablesAndColumnsTests : DatabaseTestsBase() { @Test fun `columns with default values that are whitespaces shouldn't be treated as empty strings`() { - val tableWhitespaceDefaultVarchar = StringFieldTable("varchar_whitespace_test", false," ") + val tableWhitespaceDefaultVarchar = StringFieldTable("varchar_whitespace_test", false, " ") val tableWhitespaceDefaultText = StringFieldTable("text_whitespace_test", true, " ") @@ -539,7 +540,8 @@ class CreateMissingTablesAndColumnsTests : DatabaseTestsBase() { } } - @Test fun testCreateTableWithReferenceMultipleTimes() { + @Test + fun testCreateTableWithReferenceMultipleTimes() { withTables(PlayerTable, SessionsTable) { SchemaUtils.createMissingTablesAndColumns(PlayerTable, SessionsTable) SchemaUtils.createMissingTablesAndColumns(PlayerTable, SessionsTable) @@ -554,7 +556,8 @@ class CreateMissingTablesAndColumnsTests : DatabaseTestsBase() { val playerId = integer("player_id").references(PlayerTable.id) } - @Test fun createTableWithReservedIdentifierInColumnName() { + @Test + fun createTableWithReservedIdentifierInColumnName() { withDb(TestDB.MYSQL) { SchemaUtils.createMissingTablesAndColumns(T1, T2) SchemaUtils.createMissingTablesAndColumns(T1, T2) @@ -567,11 +570,13 @@ class CreateMissingTablesAndColumnsTests : DatabaseTestsBase() { object ExplicitTable : IntIdTable() { val playerId = integer("player_id").references(PlayerTable.id, fkName = "Explicit_FK_NAME") } + object NonExplicitTable : IntIdTable() { val playerId = integer("player_id").references(PlayerTable.id) } - @Test fun explicitFkNameIsExplicit() { + @Test + fun explicitFkNameIsExplicit() { withTables(ExplicitTable, NonExplicitTable) { assertEquals("Explicit_FK_NAME", ExplicitTable.playerId.foreignKey!!.customFkName) assertEquals(null, NonExplicitTable.playerId.foreignKey!!.customFkName) @@ -582,6 +587,7 @@ class CreateMissingTablesAndColumnsTests : DatabaseTestsBase() { val name = integer("name").uniqueIndex() val tmp = varchar("temp", 255) } + object T2 : Table("CHAIN") { val ref = integer("ref").references(T1.name) } @@ -645,4 +651,36 @@ class CreateMissingTablesAndColumnsTests : DatabaseTestsBase() { } } } + + @Test + fun testCreateTableWithSchemaPrefix() { + val schemaName = "my_schema" + val schema = Schema(schemaName) + // index and foreign key both use table name to auto-generate their own names & to compare metadata + val parentTable = object : IntIdTable("$schemaName.parent_table") { + val secondId = integer("second_id").uniqueIndex() + } + val childTable = object : LongIdTable("$schemaName.child_table") { + val parent = reference("my_parent", parentTable) + } + + // SQLite does not recognize creation of schema other than the attached database + withDb(excludeSettings = listOf(TestDB.SQLITE)) { testDb -> + SchemaUtils.createSchema(schema) + SchemaUtils.create(parentTable, childTable) + + try { + SchemaUtils.createMissingTablesAndColumns(parentTable, childTable) + assertTrue(parentTable.exists()) + assertTrue(childTable.exists()) + } finally { + if (testDb == TestDB.SQLSERVER) { + SchemaUtils.drop(childTable, parentTable) + SchemaUtils.dropSchema(schema) + } else { + SchemaUtils.dropSchema(schema, cascade = true) + } + } + } + } }