-   Notifications  
You must be signed in to change notification settings  - Fork 76
 
Add support for read-only mode in DuckDB and SQLite #1383
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
Conversation
This commit introduces `createConnection` methods for `DuckDb` and `Sqlite` to handle read-only mode during connection creation. It also updates test configurations and ensures proper handling of in-memory databases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for read-only database connections in DuckDB and SQLite by implementing database-specific connection creation logic. The main changes address the limitation where some databases require read-only mode to be configured during connection establishment rather than after.
- Implements 
createConnectionmethods forDuckDbandSqliteto handle read-only mode appropriately - Updates test configurations to use temporary files instead of in-memory databases for proper read-only testing
 - Refactors connection handling logic to use database-specific connection creation
 
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description | 
|---|---|
| dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/db/DbType.kt | Adds base createConnection method with default read-only handling |  
| dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/db/DuckDb.kt | Implements DuckDB-specific connection creation with read-only property support and in-memory database validation | 
| dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/db/Sqlite.kt | Implements SQLite-specific connection creation using reflection for SQLiteConfig | 
| dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/readJdbc.kt | Updates connection handling to use database-specific connection creation | 
| dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/sqliteTest.kt | Changes from in-memory to temporary file database for read-only testing | 
| dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/local/duckDbTest.kt | Updates read-only tests to handle in-memory database limitations | 
| dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/h2/h2Test.kt | Minor H2 connection string update | 
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|   |  ||
| return connection.use { conn -> | ||
| try { | ||
| if (dbConfig.readOnly) { | 
     Copilot AI  Aug 14, 2025  
 There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The connection properties (autoCommit) are being modified but not restored after use. Consider saving the original state and restoring it in the finally block to ensure proper cleanup in pooled connection environments.
| if (dbConfig.readOnly) { | |
| return connection.use { conn -> | |
| var originalAutoCommit: Boolean? = null | |
| try { | |
| if (dbConfig.readOnly) { | |
| originalAutoCommit = conn.autoCommit | 
   dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/db/Sqlite.kt  Outdated   Show resolved Hide resolved  
 There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small things only, good fix :)
 (requires an apiDump call)
   dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/local/duckDbTest.kt  Outdated   Show resolved Hide resolved  
    dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/db/DbType.kt  Outdated   Show resolved Hide resolved  
    dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/db/Sqlite.kt  Outdated   Show resolved Hide resolved  
    dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/db/DuckDb.kt  Show resolved Hide resolved  
    dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/db/DuckDb.kt  Outdated   Show resolved Hide resolved  
 …dling Streamlined `createConnection` for SQLite using `SQLiteConfig` for read-only mode. Improved `isInMemoryDuckDb` logic for better URL handling. Updated dependencies and tests for consistency.
This commit introduces
createConnectionmethods forDuckDbandSqliteto handle read-only mode during connection creation. It also updates test configurations and ensures proper handling of in-memory databases.Fixes #1365