Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: EXPOSED-108 Incorrect mapping for UInt data type #1809

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

bog-walk
Copy link
Member

Related to PR #1799

Current State of UIntColumnType (excluding MySQL/MariaDB):

When attempting to insert a UInt value outside of the range 0..Int.MAX_VALUE, Exposed first truncates the value by calling value.toInt() before sending it to the DB:

object UIntTable : Table("uint_table") {
    val unsignedInt = uinteger("uint")
}

transaction {
    UIntTable.insert { it[unsignedInt] = 3_221_225_471u }  // compiles OK because it is a UInt
    // but generates SQL:
    // INSERT INTO uint_table (uint) VALUES (-1073741825)
}

The value is successfully stored as a negative number because most databases don't support unsigned types natively, which means Exposed is actually mapping UInt to 4-byte INT, which accepts the range -2^31..(2^31)-1. The value returned from the DB is the negative overflow and an IllegalStateException is thrown by valueFromDB().

Note: The compiler will prevent a negative number from being assigned to it[unsignedInt], but there is nothing stopping the DB from storing a negative number that is inserted directly using raw SQL:

exec("""INSERT INTO uint_table ("uint") VALUES (-1073741825);""")  // success with no SQLException
// only throws IllegalStateException if attempting to retrieve data

Solution:
Change the column type in the DB to use the next higher type, BIGINT in this case, and add a check constraint when uinteger() is registered. This would ensure that only UInt values are accepted, even if raw SQL is used.

Exception:
[Oracle] The type has not changed because it was already set to NUMBER(13) (versus the INT alias for NUMBER(38)), but a check constraint has been implemented.

Saving Storage Space:
While using a larger numeric type with a check constraint is a common solution for supporting unsigned integers, some users may not want to use 8 bytes of storage for every number. If the end-goal of the user is a column that only uses 4-bytes but does not allow negative values and they know for a fact that inserted data won't exceed Int.MAX_VALUE, then the recommendation should be to use a integer() column with a manually created check constraint:

integer("number").check { it.between(0, Int.MAX_VALUE) }
// OR
integer("number").check { (it greaterEq 0) and (it lessEq Int.MAX_VALUE) }

Currently, when attempting to insert a UInt value outside of the range [0, 2,147,483,647],
Exposed truncates the value by calling value.toInt() before sending it to the
DB, causing overflow. The value is stored successfully as a negative number because
all databases (except MySQL and MariaDB) don't support unsigned types natively,
which means Exposed is actually mapping to 4-byte INT, which accepts
the range [-2,147,483,648, 2,147,483,647].

Change the default mapping to the next higher-up integer data type BIGINT (technically
a 8-byte storage type) and remove the truncation conversions so that an accurate
value is sent/received to/from the database. To ensure that the intended behavior
cannot be overriden using exec() directly, a check constraint is auto-applied to
the column when registered if the database is not MySQL/MariaDB.
@bog-walk bog-walk requested review from e5l and joc-a July 30, 2023 20:39
@bog-walk bog-walk merged commit 7a61852 into main Jul 31, 2023
3 checks passed
@bog-walk bog-walk deleted the bog-walk/fix-uint-type branch July 31, 2023 13:34
saral pushed a commit to saral/Exposed that referenced this pull request Oct 3, 2023
Currently, when attempting to insert a UInt value outside of the range [0, 2,147,483,647],
Exposed truncates the value by calling value.toInt() before sending it to the
DB, causing overflow. The value is stored successfully as a negative number because
all databases (except MySQL and MariaDB) don't support unsigned types natively,
which means Exposed is actually mapping to 4-byte INT, which accepts
the range [-2,147,483,648, 2,147,483,647].

Change the default mapping to the next higher-up integer data type BIGINT (technically
a 8-byte storage type) and remove the truncation conversions so that an accurate
value is sent/received to/from the database. To ensure that the intended behavior
cannot be overriden using exec() directly, a check constraint is auto-applied to
the column when registered if the database is not MySQL/MariaDB.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants