Sqlite types fix - #2001
Open
AndreiKingsley wants to merge 5 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves SQLite type handling in the dataframe-jdbc module by introducing built-in conversions for SQLite’s “declared type vs storage class” mismatch (notably booleans and temporal types) and adding a DSL to register per-type / per-column custom converters, addressing issues like #1013/#1935.
Changes:
- Added
Sqlite.withCustomConverters { ... }DSL for mapping declared SQLite types / columns to KotlinKTypes and optional value conversion. - Implemented SQLite-specific preprocessing to convert storage classes into idiomatic Kotlin types for
BOOLEAN,DATE,TIME,DATETIME, andTIMESTAMP. - Expanded SQLite test coverage to include boolean and temporal column conversions, and updated docs/samples to the new custom-converter approach.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| samples/src/test/kotlin/org/jetbrains/kotlinx/dataframe/samples/schemas/DataSchemasTroubleshooting.kt | Updates troubleshooting sample to use the new SQLite custom converter DSL. |
| docs/StardustDocs/topics/schemas/Data-Schemas-And-Extension-Properties-Troubleshooting.md | Updates documentation section explaining SQLite type affinity and custom converter DSL usage. |
| dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/sqliteTest.kt | Adds regression tests for SQLite boolean + date/time/timestamp conversions. |
| dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/sqliteCustomTypesTest.kt | Updates and expands tests demonstrating custom converter DSL behavior and precedence rules. |
| dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/db/jdbcTypesTest.kt | Updates SQLite type tests to assert nullability behavior of identity forType<T>(...) mappings. |
| dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/db/Sqlite.kt | Implements new converter DSL, custom mapping precedence, and storage-class→Kotlin conversions for SQLite. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+26
to
+29
| /** | ||
| * A user-provided converter from an SQLite declared column type name to | ||
| * the [DataFrame] column type and a lambda thatн converts each stored value. | ||
| * |
Comment on lines
+246
to
+247
| // Column-name override wins over type-name override. Column nullability from the | ||
| // schema is always applied on top of the KType the user declared. |
Comment on lines
+508
to
+513
| private fun conversionError(problem: String, meta: TableColumnMetadata): Nothing = | ||
| error( | ||
| "SQLite: $problem from column '${meta.name}' (declared '${meta.sqlTypeName}'). " + | ||
| "Register a custom converter for this type or column via " + | ||
| "`Sqlite.withCustomConverters { } to override the built-in mapping.", | ||
| ) |
Comment on lines
+494
to
+497
| private fun julianDayToInstant(julianDay: Double): Instant { | ||
| val epochSeconds = ((julianDay - JULIAN_DAY_UNIX_EPOCH) * SECONDS_PER_DAY).toLong() | ||
| return Instant.fromEpochSeconds(epochSeconds) | ||
| } |
Comment on lines
542
to
+545
| public val default: Sqlite = Sqlite() | ||
|
|
||
| public fun withCustomTypes(customTypesMap: Map<String, KType>): Sqlite = Sqlite(customTypesMap) | ||
| /** | ||
| * Builds a [Sqlite] with custom type converters registered via a [SqliteCustomConvertersBuilder] DSL block. |
Comment on lines
+4
to
6
| import kotlinx.datetime.LocalDateTime | ||
| import kotlinx.datetime.format.DateTimeFormat | ||
| import org.jetbrains.kotlinx.dataframe.DataFrame |
Comment on lines
+72
to
+76
| val customFormat = LocalDateTime.Format { year() } | ||
|
|
||
| // SampleStart | ||
| val sqliteCustom = Sqlite.withCustomTypes( | ||
| mapOf( | ||
| "LONGVARCHAR" to typeOf<String>(), | ||
| "LONGINT" to typeOf<Long>(), | ||
| ), | ||
| ) | ||
| val sqliteCustom = Sqlite.withCustomConverters { | ||
| // SQLite assigns `NUMERIC` affinity to the custom `LONGVARCHAR` type, |
Comment on lines
+126
to
+131
| // Convert values from the "time_stamp" column regardless of its SQL type. | ||
| // The raw values are stored as strings and parsed into LocalDateTime values; | ||
| // the resulting column has LocalDateTime type as well. | ||
| forColumn("time_stamp") { raw: String -> | ||
| LocalDateTime.parse(raw, customFormat) | ||
| } |
Jolanrensen
reviewed
Jul 28, 2026
| } | ||
| val target = expectedPreprocessedValueType.classifier | ||
| @Suppress("UNCHECKED_CAST") | ||
| return when (target) { |
Collaborator
There was a problem hiding this comment.
best to stick to KTypes, they're cheaper than KClasses AFAIK
Jolanrensen
reviewed
Jul 28, 2026
| * @param R the target Kotlin type for the resulting DataFrame column. | ||
| */ | ||
| public inline fun <T, reified R> forColumn(columnName: String, crossinline convert: (T) -> R) { | ||
| val mapping: SqliteCustomTypeConverter<T> = typeOf<R>() to { raw -> convert(raw) } |
Collaborator
There was a problem hiding this comment.
You only store type R, making getExpectedJdbcType return the wrong type. getExpectedJdbcType should return the type getObject returns. Then, if preprocessing is done, getPreprocessedValueType should return the preprocessed type, aka R here.
Collaborator
There was a problem hiding this comment.
Same for forType(""") {}
Jolanrensen
reviewed
Jul 28, 2026
| // For DECIMAL/NUMERIC we already resolved the DataFrame type from the storage class in | ||
| // getExpectedJdbcType, so we keep that as-is. For other types we let the base decide | ||
| // (base maps TIMESTAMP → Instant, BINARY(UUID) → Uuid, etc.). | ||
| override fun getPreprocessedValueType(tableColumnMetadata: TableColumnMetadata, expectedJdbcType: KType): KType = |
Collaborator
There was a problem hiding this comment.
This function should return the type the values have after they are preprocessed. However, it doesn't seem to do anything with the type mappings
Jolanrensen
reviewed
Jul 28, 2026
| * chars(" UTC") | ||
| * } | ||
| * | ||
| * val sqliteCustom = Sqlite.withCustomConverters { |
Collaborator
There was a problem hiding this comment.
that's a nice looking dsl btw :)
Collaborator
|
@AndreiKingsley please fix Copilot and Jolan review comments |
Collaborator
|
Seems to have a lot of duplication with AdvancedDbType. Maybe the necessary changes (like converters based on column name) could be added there instead. No need to reinvent the wheel :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1013.
Fixes #1935.
Helps #1797.
Significantly improved different types support for SQLite
DbType.Add out-of-box support for
Booleanand date-time types. They don't have a their own storage classes inside SQLite, but they have an official specification, so we can detect them from column metadata name and try to extract from possible SQLite storage classes (primitives likeInt,Long,String, etc., see full list in code).Add a special DSL for specifying any other custom SQLite type , by providing converter from storage classes to expected Kotlin types. Column
KTypes are infered automatically!