Skip to content

Commit

Permalink
fix!: EXPOSED-536 Short column allows out-of-range values
Browse files Browse the repository at this point in the history
For Oracle, Short data type was changed from SMALLINT to NUMBER(5) because SMALLINT is stored as NUMBER(38) in the database and takes up unnecessary storage. Another reason is that this is a precursor to the column type change detection feature for migration, where it will be useful to have the column type be more specific than NUMBER(38).

A check constraint was added to ensure that no out-of-range values are stored.
  • Loading branch information
joc-a committed Sep 10, 2024
1 parent ac8fee1 commit ef3c88c
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 8 deletions.
3 changes: 3 additions & 0 deletions documentation-website/Writerside/topics/Breaking-Changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
The original `FunctionProvider.queryLimit()` is also being deprecated in favor of `queryLimitAndOffset()`, which takes a
nullable `size` parameter to allow exclusion of the LIMIT clause. This latter deprecation only affects extensions of the
`FunctionProvider` class when creating a custom `VendorDialect` class.
* In Oracle and H2 Oracle, the `short` column now maps to data type `NUMBER(5)` instead of `SMALLINT` because `SMALLINT` is stored as `NUMBER(38)` in the database and
takes up unnecessary storage.
In Oracle, H2 Oracle, and SQLite, using the `short` column in a table now also creates a check constraint to ensure that no out-of-range values are inserted.

## 0.54.0

Expand Down
32 changes: 24 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 @@ -509,7 +509,8 @@ open class Table(name: String = "") : ColumnSet(), DdlAware {

private val checkConstraints = mutableListOf<Pair<String, Op<Boolean>>>()

private val generatedCheckPrefix = "chk_${tableName}_unsigned_"
private val generatedUnsignedCheckPrefix = "chk_${tableName}_unsigned_"
private val generatedSignedCheckPrefix = "chk_${tableName}_signed_"

/**
* Returns the table name in proper case.
Expand Down Expand Up @@ -696,19 +697,21 @@ open class Table(name: String = "") : ColumnSet(), DdlAware {
* between 0 and [UByte.MAX_VALUE] inclusive.
*/
fun ubyte(name: String): Column<UByte> = registerColumn(name, UByteColumnType()).apply {
check("${generatedCheckPrefix}byte_$name") { it.between(0u, UByte.MAX_VALUE) }
check("${generatedUnsignedCheckPrefix}byte_$name") { it.between(0u, UByte.MAX_VALUE) }
}

/** Creates a numeric column, with the specified [name], for storing 2-byte integers. */
fun short(name: String): Column<Short> = registerColumn(name, ShortColumnType())
fun short(name: String): Column<Short> = registerColumn(name, ShortColumnType()).apply {
check("${generatedSignedCheckPrefix}short_$name") { it.between(Short.MIN_VALUE, Short.MAX_VALUE) }
}

/** Creates a numeric column, with the specified [name], for storing 2-byte unsigned integers.
*
* **Note:** If the database being used is not MySQL or MariaDB, this column will use the database's 4-byte
* integer type with a check constraint that ensures storage of only values between 0 and [UShort.MAX_VALUE] inclusive.
*/
fun ushort(name: String): Column<UShort> = registerColumn(name, UShortColumnType()).apply {
check("$generatedCheckPrefix$name") { it.between(0u, UShort.MAX_VALUE) }
check("$generatedUnsignedCheckPrefix$name") { it.between(0u, UShort.MAX_VALUE) }
}

/** Creates a numeric column, with the specified [name], for storing 4-byte integers. */
Expand All @@ -721,7 +724,7 @@ open class Table(name: String = "") : ColumnSet(), DdlAware {
* between 0 and [UInt.MAX_VALUE] inclusive.
*/
fun uinteger(name: String): Column<UInt> = registerColumn(name, UIntegerColumnType()).apply {
check("$generatedCheckPrefix$name") { it.between(0u, UInt.MAX_VALUE) }
check("$generatedUnsignedCheckPrefix$name") { it.between(0u, UInt.MAX_VALUE) }
}

/** Creates a numeric column, with the specified [name], for storing 8-byte integers. */
Expand Down Expand Up @@ -1641,12 +1644,25 @@ open class Table(name: String = "") : ColumnSet(), DdlAware {
}

if (checkConstraints.isNotEmpty()) {
val filteredChecks = when (currentDialect) {
val filteredChecks = when (val dialect = currentDialect) {
is MysqlDialect -> checkConstraints.filterNot { (name, _) ->
name.startsWith(generatedCheckPrefix)
name.startsWith(generatedUnsignedCheckPrefix) ||
name.startsWith(generatedSignedCheckPrefix)
}
is SQLServerDialect -> checkConstraints.filterNot { (name, _) ->
name.startsWith("${generatedCheckPrefix}byte_")
name.startsWith("${generatedUnsignedCheckPrefix}byte_") ||
name.startsWith(generatedSignedCheckPrefix)
}
is PostgreSQLDialect -> checkConstraints.filterNot { (name, _) ->
name.startsWith(generatedSignedCheckPrefix)
}
is H2Dialect -> {
when (dialect.h2Mode) {
H2Dialect.H2CompatibilityMode.Oracle -> checkConstraints
else -> checkConstraints.filterNot { (name, _) ->
name.startsWith(generatedSignedCheckPrefix)
}
}
}
else -> checkConstraints
}.ifEmpty { null }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import java.util.*
internal object OracleDataTypeProvider : DataTypeProvider() {
override fun byteType(): String = "SMALLINT"
override fun ubyteType(): String = "NUMBER(4)"
override fun shortType(): String = "NUMBER(5)"
override fun ushortType(): String = "NUMBER(6)"
override fun integerType(): String = "NUMBER(12)"
override fun integerAutoincType(): String = "NUMBER(12)"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package org.jetbrains.exposed.sql.tests.shared.types

import org.jetbrains.exposed.sql.Table
import org.jetbrains.exposed.sql.insert
import org.jetbrains.exposed.sql.tests.DatabaseTestsBase
import org.jetbrains.exposed.sql.tests.TestDB
import org.jetbrains.exposed.sql.tests.shared.assertEquals
import org.jetbrains.exposed.sql.tests.shared.assertFailAndRollback
import org.jetbrains.exposed.sql.tests.shared.assertTrue
import org.junit.Test

class NumericColumnTypesTests : DatabaseTestsBase() {
@Test
fun testShortAcceptsOnlyAllowedRange() {
val testTable = object : Table("test_table") {
val short = short("short")
}

withTables(testTable) { testDb ->
val columnName = testTable.short.nameInDatabaseCase()
val ddlEnding = when (testDb) {
TestDB.SQLITE, in TestDB.ALL_ORACLE_LIKE -> "CHECK ($columnName BETWEEN ${Short.MIN_VALUE} and ${Short.MAX_VALUE}))"
else -> "($columnName ${testTable.short.columnType} NOT NULL)"
}
assertTrue(testTable.ddl.single().endsWith(ddlEnding, ignoreCase = true))

testTable.insert { it[short] = Short.MIN_VALUE }
testTable.insert { it[short] = Short.MAX_VALUE }
assertEquals(2, testTable.select(testTable.short).count())

val tableName = testTable.nameInDatabaseCase()
assertFailAndRollback(message = "Out-of-range error (or CHECK constraint violation for SQLite & Oracle)") {
val outOfRangeValue = Short.MIN_VALUE - 1
exec("INSERT INTO $tableName ($columnName) VALUES ($outOfRangeValue)")
}
assertFailAndRollback(message = "Out-of-range error (or CHECK constraint violation for SQLite & Oracle)") {
val outOfRangeValue = Short.MAX_VALUE + 1
exec("INSERT INTO $tableName ($columnName) VALUES ($outOfRangeValue)")
}
}
}
}

0 comments on commit ef3c88c

Please sign in to comment.