Skip to content

Commit

Permalink
fix: Incorrect SQL statements when creating a table with a dot in its…
Browse files Browse the repository at this point in the history
… name (#1871)

* fix: Incorrect SQL statements when creating a table with a dot in its name

* test: fix Failing test createTableWithExplicitForeignKeyName4

The test failed because fallbackSeqName = null_id_seq. The reason tableName was null is that its declaration in the test was missing get().

* chore: add comment on Oracle exclusion in test

* fix: Test failing when checking if table exists

tester.exists() was returning false because of issues with processing the quoted table name that led to a mismatch

* fix: testCreateTableWithQuotedIdentifiers fails

In `String.metadataMatchesTable`, we should not use `table.tableNameWithoutSchemeSanitized` because "sanitization" in this case should be dialect-aware (aka we should be removing only what is considered a valid quote string in MySQL, which is the backtick). This means that if a table has double quotes (") in MySQL, they should not be removed when we compare it with the table name returned from the metadata, because the metadata (DatabaseMetaData.getTables method) returns it WITH the double quotes. If a name is quoted with backticks (`) instead, the metadata returns it WITHOUT backticks.

For example:
For table name "\"IdentifierTable\"", metadata returns -> "testdb."IdentifierTable""
For table name "`SomeNamespace.SomeTable`", metadata returns -> "testdb.SomeNamespace.SomeTable"

* chore: add KDoc for tableNameWithoutSchemeSanitized

* chore: add KDoc for tableNameWithoutScheme

* chore: update KDoc for schemaName
  • Loading branch information
joc-a authored Oct 11, 2023
1 parent 0eb0ac1 commit 3656269
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 11 deletions.
48 changes: 40 additions & 8 deletions exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Table.kt
Original file line number Diff line number Diff line change
Expand Up @@ -345,15 +345,40 @@ open class Table(name: String = "") : ColumnSet(), DdlAware {
else -> javaClass.name.removePrefix("${javaClass.`package`.name}.").substringAfter('$').removeSuffix("Table")
}

/** Returns the schema name, or null if one does not exist for this table. */
val schemaName: String? = if (name.contains(".")) name.substringBeforeLast(".") else null
/** Returns the schema name, or null if one does not exist for this table.
*
* If the table is quoted, a dot in the name is considered part of the table name and the whole string is taken to
* be the table name as is, so there would be no schema. If it is not quoted, whatever is after the dot is
* considered to be the table name, and whatever is before the dot is considered to be the schema.
*/
val schemaName: String? = if (name.contains(".") && !name.isAlreadyQuoted()) {
name.substringBeforeLast(".")
} else {
null
}

internal val tableNameWithoutScheme: String get() = tableName.substringAfterLast(".")
/**
* Returns the table name without schema.
*
* If the table is quoted, a dot in the name is considered part of the table name and the whole string is taken to
* be the table name as is. If it is not quoted, whatever is after the dot is considered to be the table name.
*/
internal val tableNameWithoutScheme: String
get() = if (!tableName.isAlreadyQuoted()) tableName.substringAfterLast(".") else tableName

// Table name may contain quotes, remove those before appending
internal val tableNameWithoutSchemeSanitized: String get() = tableNameWithoutScheme
.replace("\"", "")
.replace("'", "")
/**
* Returns the table name without schema, with all quotes removed.
*
* Used for two purposes:
* 1. Forming primary and foreign key names
* 2. Comparing table names from database metadata (except MySQL and MariaDB)
* @see org.jetbrains.exposed.sql.vendors.VendorDialect.metadataMatchesTable
*/
internal val tableNameWithoutSchemeSanitized: String
get() = tableNameWithoutScheme
.replace("\"", "")
.replace("'", "")
.replace("`", "")

private val _columns = mutableListOf<Column<*>>()

Expand Down Expand Up @@ -1192,8 +1217,10 @@ open class Table(name: String = "") : ColumnSet(), DdlAware {
private fun <T> Column<T>.cloneWithAutoInc(idSeqName: String?): Column<T> = when (columnType) {
is AutoIncColumnType -> this
is ColumnType -> {
val q = if (tableName.contains('.')) "\"" else ""
val fallbackSeqName = "$q${tableName.replace("\"", "")}_${name}_seq$q"
this.withColumnType(
AutoIncColumnType(columnType, idSeqName, "${tableName?.replace("\"", "")}_${name}_seq")
AutoIncColumnType(columnType, idSeqName, fallbackSeqName)
)
}

Expand Down Expand Up @@ -1335,3 +1362,8 @@ fun ColumnSet.targetTables(): List<Table> = when (this) {
is Join -> this.table.targetTables() + this.joinParts.flatMap { it.joinPart.targetTables() }
else -> error("No target provided for update")
}

private fun String.isAlreadyQuoted(): Boolean =
listOf("\"", "'", "`").any { quoteString ->
startsWith(quoteString) && endsWith(quoteString)
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ abstract class IdentifierManagerApi {
}

fun quoteIfNecessary(identity: String): String {
return if (identity.contains('.')) {
return if (identity.contains('.') && !identity.isAlreadyQuoted()) {
identity.split('.').joinToString(".") { quoteTokenIfNecessary(it) }
} else {
quoteTokenIfNecessary(identity)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ open class MysqlDialect : VendorDialect(dialectName, MysqlDataTypeProvider, Mysq
return when {
schema.isEmpty() -> this == table.nameInDatabaseCaseUnquoted()
else -> {
val sanitizedTableName = table.tableNameWithoutScheme
val sanitizedTableName = table.tableNameWithoutScheme.replace("`", "")
val nameInDb = "$schema.$sanitizedTableName".inProperCase()
this == nameInDb
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ 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.eq
import org.jetbrains.exposed.sql.tests.DatabaseTestsBase
import org.jetbrains.exposed.sql.tests.TestDB
import org.jetbrains.exposed.sql.tests.currentDialectTest
Expand Down Expand Up @@ -444,7 +445,7 @@ class CreateTableTests : DatabaseTestsBase() {
fun createTableWithExplicitForeignKeyName4() {
val fkName = "MyForeignKey4"
val parent = object : LongIdTable() {
override val tableName = "parent4"
override val tableName get() = "parent4"
val uniqueId = uuid("uniqueId").clientDefault { UUID.randomUUID() }.uniqueIndex()
}
val child = object : LongIdTable("child4") {
Expand Down Expand Up @@ -629,4 +630,36 @@ class CreateTableTests : DatabaseTestsBase() {
}
}
}

/**
* Note on Oracle exclusion in this test:
* Oracle names are not case-sensitive. They can be made case-sensitive by using quotes around them. The Oracle JDBC
* driver converts the entire SQL INSERT statement to upper case before extracting the table name from it. This
* happens regardless of whether there is a dot in the name. Even when a name is quoted, the driver converts
* it to upper case. Therefore, the INSERT statement fails when it contains a quoted table name because it attempts
* to insert into a table that does not exist (“SOMENAMESPACE.SOMETABLE” is not found) . It does not fail when the
* table name is not quoted because the case would not matter in that scenario.
*/
@Test
fun `create table with dot in name without creating schema beforehand`() {
withDb(excludeSettings = listOf(TestDB.ORACLE)) {
val q = db.identifierManager.quoteString
val tableName = "${q}SomeNamespace.SomeTable$q"

val tester = object : IntIdTable(tableName) {
val text_col = text("text_col")
}

try {
SchemaUtils.create(tester)
assertTrue(tester.exists())

val id = tester.insertAndGetId { it[text_col] = "Inserted text" }
tester.update({ tester.id eq id }) { it[text_col] = "Updated text" }
tester.deleteWhere { tester.id eq id }
} finally {
SchemaUtils.drop(tester)
}
}
}
}

0 comments on commit 3656269

Please sign in to comment.