Skip to content

Commit

Permalink
fix!: EXPOSED-150 Auto-quoted column names change case across databases
Browse files Browse the repository at this point in the history
Add global flag, preserveKeywordCasing, to DatabaseConfig to allow temporary
opt-out from new behavior.

Log warnings if table is created using identifiers that are in keywords list.
  • Loading branch information
bog-walk committed Sep 15, 2023
1 parent 6c9649d commit d10524e
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ class DatabaseConfig private constructor(
val keepLoadedReferencesOutOfTransaction: Boolean,
val explicitDialect: DatabaseDialect?,
val defaultSchema: Schema?,
val logTooMuchResultSetsThreshold: Int
val logTooMuchResultSetsThreshold: Int,
val preserveKeywordCasing: Boolean,
) {

class Builder(
Expand Down Expand Up @@ -85,26 +86,28 @@ class DatabaseConfig private constructor(
* Useful when [eager loading](https://github.com/JetBrains/Exposed/wiki/DAO#eager-loading) is used.
*/
var keepLoadedReferencesOutOfTransaction: Boolean = false,

/**
* Set the explicit dialect for a database.
* This can be useful when working with unsupported dialects which have the same behavior as the one that
* Exposed supports.
*/
var explicitDialect: DatabaseDialect? = null,

/**
* Set the default schema for a database.
*/
var defaultSchema: Schema? = null,

/**
* Log too much result sets opened in parallel.
* The error log will contain the stacktrace of the place in the code where a new result set occurs, and it
* exceeds the threshold.
* 0 value means no log needed.
*/
var logTooMuchResultSetsThreshold: Int = 0,
/**
* Toggle whether table and column identifiers that are also keywords should retain their case sensitivity.
* Keeping user-defined case sensitivity (value set to `true`) will become the default in future releases.
*/
var preserveKeywordCasing: Boolean = true,
)

companion object {
Expand All @@ -124,7 +127,8 @@ class DatabaseConfig private constructor(
keepLoadedReferencesOutOfTransaction = builder.keepLoadedReferencesOutOfTransaction,
explicitDialect = builder.explicitDialect,
defaultSchema = builder.defaultSchema,
logTooMuchResultSetsThreshold = builder.logTooMuchResultSetsThreshold
logTooMuchResultSetsThreshold = builder.logTooMuchResultSetsThreshold,
preserveKeywordCasing = builder.preserveKeywordCasing,
)
}
}
Expand Down
21 changes: 19 additions & 2 deletions exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Table.kt
Original file line number Diff line number Diff line change
Expand Up @@ -1209,6 +1209,21 @@ open class Table(name: String = "") : ColumnSet(), DdlAware {
}
}

@Deprecated(
message = "This will be removed in future releases when the check becomes default in IdentifierManagerApi",
level = DeprecationLevel.WARNING
)
private fun String.warnIfAKeyword() {
val isAKeyword = TransactionManager.currentOrNull()?.db?.identifierManager?.isAKeyword(this) == true
if (isAKeyword) {
exposedLogger.warn(
"Keyword identifier used: '$this'. Case sensitivity will be kept when quoted by default. " +
"To temporarily opt-out, set 'preserveKeywordCasing' to false in DatabaseConfig block."
)
}
}

@Suppress("CyclomaticComplexMethod")
override fun createStatement(): List<String> {
val createSequence = autoIncColumn?.autoIncColumnType?.autoincSeq?.let {
Sequence(
Expand All @@ -1228,9 +1243,11 @@ open class Table(name: String = "") : ColumnSet(), DdlAware {
if (currentDialect.supportsIfNotExists) {
append("IF NOT EXISTS ")
}
append(TransactionManager.current().identity(this@Table))
append(TransactionManager.current().identity(this@Table).also { tableName.warnIfAKeyword() })
if (columns.isNotEmpty()) {
columns.joinTo(this, prefix = " (") { it.descriptionDdl(false) }
columns.joinTo(this, prefix = " (") { column ->
column.descriptionDdl(false).also { column.name.warnIfAKeyword() }
}

if (columns.any { it.isPrimaryConstraintWillBeDefined }) {
primaryKeyConstraint()?.let { append(", $it") }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jetbrains.exposed.sql.statements.api

import org.jetbrains.exposed.sql.transactions.TransactionManager
import org.jetbrains.exposed.sql.vendors.ANSI_SQL_2003_KEYWORDS
import org.jetbrains.exposed.sql.vendors.VENDORS_KEYWORDS
import org.jetbrains.exposed.sql.vendors.currentDialect
Expand Down Expand Up @@ -38,13 +39,25 @@ abstract class IdentifierManagerApi {
private fun String.isIdentifier() = !isEmpty() && first().isIdentifierStart() && all { it.isIdentifierStart() || it in '0'..'9' }
private fun Char.isIdentifierStart(): Boolean = this in 'a'..'z' || this in 'A'..'Z' || this == '_' || this in extraNameCharacters

private fun String.isAKeyword(): Boolean = checkedKeywordsCache.getOrPut(lowercase()) {
keywords.any { this.equals(it, true) }
@Deprecated(
message = "This will become private in future releases when the check becomes default in IdentifierManagerApi",
level = DeprecationLevel.WARNING
)
internal fun isAKeyword(identity: String): Boolean = checkedKeywordsCache.getOrPut(identity.lowercase()) {
keywords.any { identity.equals(it, true) }
}

@Deprecated(
message = "This will be removed in future releases when the behavior becomes default in IdentifierManagerApi",
level = DeprecationLevel.WARNING
)
private val shouldPreserveKeywordCasing by lazy {
TransactionManager.currentOrNull()?.db?.config?.preserveKeywordCasing == true
}

fun needQuotes(identity: String): Boolean {
return checkedIdentitiesCache.getOrPut(identity.lowercase()) {
!identity.isAlreadyQuoted() && (identity.isAKeyword() || !identity.isIdentifier())
!identity.isAlreadyQuoted() && (isAKeyword(identity) || !identity.isIdentifier())
}
}

Expand All @@ -56,7 +69,7 @@ abstract class IdentifierManagerApi {
val alreadyUpper = identity == identity.uppercase()
when {
alreadyQuoted -> false
identity.isAKeyword() -> true
isAKeyword(identity) && shouldPreserveKeywordCasing -> true
supportsMixedIdentifiers -> false
alreadyLower && isLowerCaseIdentifiers -> false
alreadyUpper && isUpperCaseIdentifiers -> false
Expand All @@ -72,7 +85,8 @@ abstract class IdentifierManagerApi {
alreadyQuoted && supportsMixedQuotedIdentifiers -> identity
alreadyQuoted && isUpperCaseQuotedIdentifiers -> identity.uppercase()
alreadyQuoted && isLowerCaseQuotedIdentifiers -> identity.lowercase()
supportsMixedIdentifiers || identity.isAKeyword() -> identity
supportsMixedIdentifiers -> identity
isAKeyword(identity) && shouldPreserveKeywordCasing -> identity
oracleVersion != OracleVersion.NonOracle -> identity.uppercase()
isUpperCaseIdentifiers -> identity.uppercase()
isLowerCaseIdentifiers -> identity.lowercase()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jetbrains.exposed.sql.tests.shared

import nl.altindag.log.LogCaptor
import org.jetbrains.exposed.dao.IntEntity
import org.jetbrains.exposed.dao.IntEntityClass
import org.jetbrains.exposed.dao.id.EntityID
Expand Down Expand Up @@ -57,7 +58,7 @@ class DDLTests : DatabaseTestsBase() {

@Test
fun testCreateTableWithKeywordIdentifiers() {
val keywords = listOf("key", "public", "data", "constraint")
val keywords = listOf("data", "public", "key", "constraint")
val keywordTable = object : Table(keywords[0]) {
val public = bool(keywords[1])
val data = integer(keywords[2])
Expand Down Expand Up @@ -98,6 +99,27 @@ class DDLTests : DatabaseTestsBase() {
}
}

// TODO: Remove test when preserveKeywordCasing flag is deprecated
@Test
fun testKeywordIdentifiersLogWarningOnCreate() {
val logCaptor = LogCaptor.forName(exposedLogger.name)
val tester = object : Table("BooLean") {
val name = varchar("name", 32)
}

withDb {
SchemaUtils.create(tester)
assertEquals(2, logCaptor.warnLogs.size)
logCaptor.clearLogs()

tester.insert { it[name] = "A" }
tester.selectAll().toList()
SchemaUtils.drop(tester)
assertEquals(0, logCaptor.warnLogs.size)
logCaptor.clearLogs()
}
}

// Placed outside test function to shorten generated name
val UnnamedTable = object : Table() {
val id = integer("id")
Expand Down

0 comments on commit d10524e

Please sign in to comment.