JetBrains / Exposed

Kotlin SQL Framework
http://jetbrains.github.io/Exposed/
Apache License 2.0
8.05k stars 676 forks source link

fix!: EXPOSED-269 Incompatible with sqlite-jdbc 3.45.0.0 #2030

Closed joc-a closed 3 months ago

joc-a commented 3 months ago

sqlite-jdbc 3.45.0.0 reintroduced improved support for Statement#getGeneratedKeys(), but the implementation is different from the previous implementation and does not work with Exposed. This is a temporary workaround to fix the crash caused by that until we figure out how we're supposed to use sqlite-jdbc 3.45.0.0+ with Exposed.

To be investigated (YouTrack issue):

The problem is that a newly-introduced function Statement#updateGeneratedKeys() in sqlite-jdbc, which sets the value of the ResultSet, is not invoked when executeBatch() is used, but only with executeUpdate(). This means that something probably has to be changed in the following function in InsertStatement.kt in Exposed to accommodate this new behaviour:

protected open fun PreparedStatementApi.execInsertFunction(): Pair<Int, ResultSet?> {
    val inserted = if (arguments().count() > 1 || isAlwaysBatch) executeBatch().sum() else executeUpdate()
    val rs = if (autoIncColumns.isNotEmpty()) {
        resultSet
    } else {
        null
    }
    return inserted to rs
}
joc-a commented 3 months ago

Just confirming that these changes won't work with the older SQLite version, right? So bumping to the upcoming Exposed version would mean needing to bump SQLite too?

@bog-walk That's correct. Should we add a note about this under Breaking Changes or just mention it in the GitHub release description?

bog-walk commented 3 months ago

I'd include a note wherever gets the most visibility to users, just to be safe, so I'd probably go for the breaking changes section in the changelog.

JordanPlayz158 commented 3 months ago

Glad to see this is documented, just got burned by this so can't wait for this to get this fix pushed and included in next release. The docs gave me a clue because it had specific versions pinned luckily so I went, to heck with it and tried, thank goodness it worked, would've been pulling my hair out for hours