Added support for BIGINT UNSIGNED type in MySQL and MariaDB DBTypes#1735
Added support for BIGINT UNSIGNED type in MySQL and MariaDB DBTypes#1735AndreiKingsley wants to merge 6 commits intomasterfrom
BIGINT UNSIGNED type in MySQL and MariaDB DBTypes#1735Conversation
|
could you add tests for this specific fix? :) (the build should not have failed, I'll rerun) |
|
@Jolanrensen added tests as an initiating of #1736. |
|
@AndreiKingsley that's good :) though, I would also recommend running a test with the actual database, like here https://github.com/Kotlin/dataframe/blob/c3742a95684d53a813d978a3c895090ba684c46e/dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/local/mysqlTest.kt (though, you'd need to set up a local database for that. Just ask Junie to do that for you ;P, I think @zaleslaw would appreciate it :) ) |
|
@Jolanrensen , added |
| class MariaDBTypes { | ||
|
|
||
| object BIGINT_UNSIGNED : ColumnType( | ||
| "BIGINT UNSIGNED", |
There was a problem hiding this comment.
I'd recommend using named arguments. Makes refactoring easier :)
| val javaClassName: String, | ||
| val expectedKotlinType: KType, | ||
| ) { | ||
| fun mockkColMetaData() = |
There was a problem hiding this comment.
since when does "mock" have 2 k's? ;P
There was a problem hiding this comment.
oh and "metadata" is one word, so it would become: "mockColMetadata()"
There was a problem hiding this comment.
Because I often use MockK variables usually called mockk.. 😆
| jdbcType, | ||
| 10, | ||
| javaClassName, | ||
| false, |
There was a problem hiding this comment.
hmm by setting this to false always, you might not catch the cases where someone forgets to take care about nullability (which is an easy mistake to make). Maybe we should test both isNullable and !isNullable. This could also be done in a next pr of course
|
|
||
| object BIGINT_UNSIGNED : ColumnType( | ||
| "BIGINT UNSIGNED", | ||
| 20, |
There was a problem hiding this comment.
I'd use Sql.Types for this
There was a problem hiding this comment.
oh, it doesn't exist?... interesting
There was a problem hiding this comment.
Yep, it's a MySQL/MariaDB specific type.
2 participants
Fixes #762.