From 88509166b53423437f83d371ec87a0ec4da022de Mon Sep 17 00:00:00 2001 From: Chantal Loncle <82039410+bog-walk@users.noreply.github.com> Date: Tue, 28 Jan 2025 22:03:30 -0500 Subject: [PATCH] refactor: EXPOSED-710 Move plain SQL for retrieving foreign keys from MysqlDial> Plain SQL in MysqlDialect.fillConstraintCacheForTables() is now extracted out o> core module and into jdbc module, in preparation for R2DBC implementation. Logic has been placed in tableConstraints() and end result adjusted to fit previous behavior. --- exposed-core/api/exposed-core.api | 1 - .../exposed/sql/vendors/MysqlDialect.kt | 59 --------- .../jdbc/JdbcDatabaseMetadataImpl.kt | 119 +++++++++++++----- .../sql/tests/shared/ConnectionTests.kt | 18 ++- .../tests/sqlite/ForeignKeyConstraintTests.kt | 32 +++-- 5 files changed, 127 insertions(+), 102 deletions(-) diff --git a/exposed-core/api/exposed-core.api b/exposed-core/api/exposed-core.api index fdcc8026d7..91bb2c96ca 100644 --- a/exposed-core/api/exposed-core.api +++ b/exposed-core/api/exposed-core.api @@ -4191,7 +4191,6 @@ public class org/jetbrains/exposed/sql/vendors/MysqlDialect : org/jetbrains/expo public fun createSchema (Lorg/jetbrains/exposed/sql/Schema;)Ljava/lang/String; public fun dropIndex (Ljava/lang/String;Ljava/lang/String;ZZ)Ljava/lang/String; public fun dropSchema (Lorg/jetbrains/exposed/sql/Schema;Z)Ljava/lang/String; - protected fun fillConstraintCacheForTables (Ljava/util/List;)V public fun getSupportsCreateSequence ()Z public fun getSupportsOrderByNullsFirstLast ()Z public fun getSupportsSetDefaultReferenceOption ()Z diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/MysqlDialect.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/MysqlDialect.kt index a5b445ac69..c9bf7c63e2 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/MysqlDialect.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/MysqlDialect.kt @@ -376,65 +376,6 @@ open class MysqlDialect : VendorDialect(dialectName, MysqlDataTypeProvider, Mysq return e.toString().trim() !in notAcceptableDefaults } - override fun fillConstraintCacheForTables(tables: List) { - 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 tableSchema = "'${tables.mapNotNull { it.schemaName }.toSet().singleOrNull() ?: getDatabase()}'" - val constraintsToLoad = HashMap>() - tr.exec( - """SELECT - rc.CONSTRAINT_NAME, - ku.TABLE_NAME, - ku.COLUMN_NAME, - ku.REFERENCED_TABLE_NAME, - ku.REFERENCED_COLUMN_NAME, - rc.UPDATE_RULE, - rc.DELETE_RULE - 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 = $tableSchema - AND ku.CONSTRAINT_SCHEMA = $tableSchema - AND rc.CONSTRAINT_SCHEMA = $tableSchema - AND $inTableList - ORDER BY ku.ORDINAL_POSITION - """.trimIndent() - ) { rs -> - while (rs.next()) { - val fromTableName = rs.getString("TABLE_NAME")!! - if (fromTableName !in allTableNames) continue - val fromColumnName = rs.getString("COLUMN_NAME")!!.quoteIdentifierWhenWrongCaseOrNecessary(tr) - allTables.getValue(fromTableName).columns.firstOrNull { - it.nameInDatabaseCase().quoteIdentifierWhenWrongCaseOrNecessary(tr) == fromColumnName - }?.let { fromColumn -> - val constraintName = rs.getString("CONSTRAINT_NAME")!! - val targetTableName = rs.getString("REFERENCED_TABLE_NAME")!! - val targetColumnName = rs.getString("REFERENCED_COLUMN_NAME")!!.quoteIdentifierWhenWrongCaseOrNecessary(tr) - val targetColumn = allTables.getValue(targetTableName).columns.first { - it.nameInDatabaseCase().quoteIdentifierWhenWrongCaseOrNecessary(tr) == targetColumnName - } - val constraintUpdateRule = ReferenceOption.valueOf(rs.getString("UPDATE_RULE")!!.replace(" ", "_")) - val constraintDeleteRule = ReferenceOption.valueOf(rs.getString("DELETE_RULE")!!.replace(" ", "_")) - constraintsToLoad.getOrPut(fromTableName) { mutableMapOf() }.merge( - constraintName, - ForeignKeyConstraint( - target = targetColumn, - from = fromColumn, - onUpdate = constraintUpdateRule, - onDelete = constraintDeleteRule, - name = constraintName - ), - ForeignKeyConstraint::plus - ) - } - } - - columnConstraintsCache.putAll(constraintsToLoad.mapValues { (_, v) -> v.values }) - } - } - override fun createIndex(index: Index): String { if (index.functions != null && !isMysql8) { exposedLogger.warn( 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 d1ae431ec5..4f47d809be 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 @@ -399,37 +399,98 @@ class JdbcDatabaseMetadataImpl(database: String, val metadata: DatabaseMetaData) @Synchronized override fun tableConstraints(tables: List
): Map> { val allTables = SchemaUtils.sortTablesByReferences(tables).associateBy { it.nameInDatabaseCaseUnquoted() } - return allTables.keys.associateWith { table -> - 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 { - identifierManager.quoteIdentifierWhenWrongCaseOrNecessary(it.name) == fromColumnName - } ?: return@iterate null // Do not crash if there are missing fields in Exposed's tables - val constraintName = getString("FK_NAME")!! - val targetTableName = getString("PKTABLE_NAME")!! - val targetColumnName = identifierManager.quoteIdentifierWhenWrongCaseOrNecessary( - identifierManager.inProperCase(getString("PKCOLUMN_NAME")!!) - ) - val targetColumn = allTables[targetTableName]?.columns?.firstOrNull { - identifierManager.quoteIdentifierWhenWrongCaseOrNecessary(it.nameInDatabaseCase()) == targetColumnName - } ?: return@iterate null // Do not crash if there are missing fields in Exposed's tables - val constraintUpdateRule = getObject("UPDATE_RULE")?.toString()?.toIntOrNull()?.let { - currentDialect.resolveRefOptionFromJdbc(it) + val dialect = currentDialect + + return if (dialect is MysqlDialect) { + val transaction = TransactionManager.current() + val inTableList = allTables.keys.joinToString("','", prefix = " ku.TABLE_NAME IN ('", postfix = "')") + val tableSchema = "'${tables.mapNotNull { it.schemaName }.toSet().singleOrNull() ?: currentSchema}'" + val constraintsToLoad = HashMap>() + transaction.exec( + """ + SELECT + rc.CONSTRAINT_NAME AS FK_NAME, + ku.TABLE_NAME AS FKTABLE_NAME, + ku.COLUMN_NAME AS FKCOLUMN_NAME, + ku.REFERENCED_TABLE_NAME AS PKTABLE_NAME, + ku.REFERENCED_COLUMN_NAME AS PKCOLUMN_NAME, + rc.UPDATE_RULE, + rc.DELETE_RULE + 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 = $tableSchema + AND ku.CONSTRAINT_SCHEMA = $tableSchema + AND rc.CONSTRAINT_SCHEMA = $tableSchema + AND $inTableList + ORDER BY ku.ORDINAL_POSITION + """.trimIndent() + ) { rs -> + while (rs.next()) { + rs.extractForeignKeys(allTables, true)?.let { (fromTableName, fk) -> + constraintsToLoad.getOrPut(fromTableName) { mutableMapOf() } + .merge(fk.fkName, fk, ForeignKeyConstraint::plus) + } } - val constraintDeleteRule = currentDialect.resolveRefOptionFromJdbc(getInt("DELETE_RULE")) - ForeignKeyConstraint( - target = targetColumn, - from = fromColumn, - onUpdate = constraintUpdateRule, - onDelete = constraintDeleteRule, - name = constraintName - ) - }.filterNotNull().groupBy { it.fkName }.values.map { it.reduce(ForeignKeyConstraint::plus) } + } + // This ensures MySQL/MariaDB have same behavior as before: a map entry for every table even if no FKs + allTables.keys.forEach { constraintsToLoad.putIfAbsent(it, mutableMapOf()) } + constraintsToLoad.mapValues { (_, v) -> v.values.toList() } + } else { + allTables.keys.associateWith { table -> + val (catalog, tableSchema) = tableCatalogAndSchema(allTables[table]!!) + metadata.getImportedKeys(catalog, identifierManager.inProperCase(tableSchema), table) + .iterate { extractForeignKeys(allTables, false) } + .filterNotNull() + .unzip().second + .groupBy { it.fkName }.values + .map { it.reduce(ForeignKeyConstraint::plus) } + } + } + } + + private fun ResultSet.extractForeignKeys( + allTables: Map, + isMysqlDialect: Boolean + ): Pair? { + val fromTableName = getString("FKTABLE_NAME")!! + if (isMysqlDialect && fromTableName !in allTables.keys) return null + val fromColumnName = identifierManager.quoteIdentifierWhenWrongCaseOrNecessary( + getString("FKCOLUMN_NAME")!! + ) + val fromColumn = allTables[fromTableName]?.columns?.firstOrNull { + val identifier = if (isMysqlDialect) it.nameInDatabaseCase() else it.name + identifierManager.quoteIdentifierWhenWrongCaseOrNecessary(identifier) == fromColumnName + } ?: return null // Do not crash if there are missing fields in Exposed's tables + val constraintName = getString("FK_NAME")!! + val targetTableName = getString("PKTABLE_NAME")!! + val targetColumnName = identifierManager.quoteIdentifierWhenWrongCaseOrNecessary( + if (isMysqlDialect) { + getString("PKCOLUMN_NAME")!! + } else { + identifierManager.inProperCase(getString("PKCOLUMN_NAME")!!) + } + ) + val targetColumn = allTables[targetTableName]?.columns?.firstOrNull { + identifierManager.quoteIdentifierWhenWrongCaseOrNecessary(it.nameInDatabaseCase()) == targetColumnName + } ?: return null // Do not crash if there are missing fields in Exposed's tables + val constraintUpdateRule = if (isMysqlDialect) { + ReferenceOption.valueOf(getString("UPDATE_RULE")!!.replace(" ", "_")) + } else { + getObject("UPDATE_RULE")?.toString()?.toIntOrNull()?.let { currentDialect.resolveRefOptionFromJdbc(it) } + } + val constraintDeleteRule = if (isMysqlDialect) { + ReferenceOption.valueOf(getString("DELETE_RULE")!!.replace(" ", "_")) + } else { + currentDialect.resolveRefOptionFromJdbc(getInt("DELETE_RULE")) } + return fromTableName to ForeignKeyConstraint( + target = targetColumn, + from = fromColumn, + onUpdate = constraintUpdateRule, + onDelete = constraintDeleteRule, + name = constraintName + ) } /** diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ConnectionTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ConnectionTests.kt index 5af72164fe..fe5baa2168 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ConnectionTests.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ConnectionTests.kt @@ -40,20 +40,32 @@ class ConnectionTests : DatabaseTestsBase() { } } - // GitHub issue #838 @Test - fun testTableConstraints() { + fun testTableConstraintsWithFKColumnsThatNeedQuoting() { val parent = object : LongIdTable("parent") { val scale = integer("scale").uniqueIndex() } val child = object : LongIdTable("child") { val scale = reference("scale", parent.scale) } - withTables(listOf(TestDB.MYSQL_V5), child, parent) { + + withTables(child, parent) { testDb -> val constraints = connection.metadata { tableConstraints(listOf(child)) } + // tableConstraints() returns entries for all tables involved in the FK (parent + child) assertEquals(2, constraints.keys.size) + + // EXPOSED-711 https://youtrack.jetbrains.com/issue/EXPOSED-711/Oracle-tableConstraints-columnContraints-dont-return-foreign-keys + // but only child entry has a non-empty list of FKs + if (testDb != TestDB.ORACLE) { + assertEquals( + 1, + constraints.values.count { fks -> + fks.any { it.fkName == child.scale.foreignKey?.fkName } + } + ) + } } } } diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/sqlite/ForeignKeyConstraintTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/sqlite/ForeignKeyConstraintTests.kt index cf3a65490e..0f411986b5 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/sqlite/ForeignKeyConstraintTests.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/sqlite/ForeignKeyConstraintTests.kt @@ -1,6 +1,7 @@ package org.jetbrains.exposed.sql.tests.sqlite import org.jetbrains.exposed.dao.id.IntIdTable +import org.jetbrains.exposed.dao.id.LongIdTable import org.jetbrains.exposed.exceptions.ExposedSQLException import org.jetbrains.exposed.sql.* import org.jetbrains.exposed.sql.SqlExpressionBuilder.eq @@ -186,22 +187,15 @@ class ForeignKeyConstraintTests : DatabaseTestsBase() { override val primaryKey = PrimaryKey(id) } - withTables(category, item) { testDb -> + withTables(category, item) { if (currentDialectTest.supportsOnUpdate) { val constraints = connection.metadata { tableConstraints(listOf(item)) } constraints.values.forEach { list -> list.forEach { - // According to the documentation: "NO ACTION: A keyword from standard SQL. For InnoDB, this is equivalent to RESTRICT;" - // https://dev.mysql.com/doc/refman/8.0/en/create-table-foreign-keys.html - if (testDb == TestDB.MYSQL_V5) { - assertEquals(ReferenceOption.NO_ACTION, it.updateRule) - assertEquals(ReferenceOption.NO_ACTION, it.deleteRule) - } else { - assertEquals(currentDialectTest.defaultReferenceOption, it.updateRule) - assertEquals(currentDialectTest.defaultReferenceOption, it.deleteRule) - } + assertEquals(currentDialectTest.defaultReferenceOption, it.updateRule) + assertEquals(currentDialectTest.defaultReferenceOption, it.deleteRule) } } } @@ -282,4 +276,22 @@ class ForeignKeyConstraintTests : DatabaseTestsBase() { } } } + + @Test + fun testColumnConstraintsWithFKColumnsThatNeedQuoting() { + val parent = object : LongIdTable("parent") { + val scale = integer("scale").uniqueIndex() + } + val child = object : LongIdTable("child") { + val scale = reference("scale", parent.scale) + } + + // EXPOSED-711 https://youtrack.jetbrains.com/issue/EXPOSED-711/Oracle-tableConstraints-columnContraints-dont-return-foreign-keys + withTables(excludeSettings = listOf(TestDB.ORACLE), child, parent) { + val constraints = currentDialectTest.columnConstraints(child) + // columnConstraints() only return entry for table that has column with FK + assertEquals(1, constraints.keys.size) + assertEquals(child.scale.foreignKey?.fkName, constraints.entries.single().value.single().fkName) + } + } }