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-269 Incompatible with sqlite-jdbc 3.45.0.0 #2030

Merged
merged 1 commit into from
Mar 25, 2024

Conversation

joc-a
Copy link
Collaborator

@joc-a joc-a commented Mar 20, 2024

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 joc-a force-pushed the joc/fix-error-with-sqlite-jdbc-3.45.0.0 branch from 1832af4 to 55db9ee Compare March 20, 2024 15:44
@joc-a joc-a marked this pull request as ready for review March 20, 2024 16:20
@joc-a joc-a requested review from bog-walk and e5l March 20, 2024 17:54
Copy link
Member

@bog-walk bog-walk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@joc-a
Copy link
Collaborator Author

joc-a commented Mar 21, 2024

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
Copy link
Member

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
Copy link

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

@joc-a joc-a changed the title fix: EXPOSED-269 Incompatible with sqlite-jdbc 3.45.0.0 fix!: EXPOSED-269 Incompatible with sqlite-jdbc 3.45.0.0 Mar 25, 2024
sqlite-jdbc 3.45.0.0 [reintroduced improved support for Statement#getGeneratedKeys()](xerial/sqlite-jdbc@f7d49f6), 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.
@joc-a joc-a force-pushed the joc/fix-error-with-sqlite-jdbc-3.45.0.0 branch from 55db9ee to 38ee832 Compare March 25, 2024 09:56
@joc-a joc-a merged commit 6db261d into main Mar 25, 2024
3 of 5 checks passed
@joc-a joc-a deleted the joc/fix-error-with-sqlite-jdbc-3.45.0.0 branch March 25, 2024 09:59
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.

4 participants