Simply removing JDBC generated key support is NOT a fix

[barrycaceres]

Version 3.43.0.0 and later breaks all support for obtaining generated keys because it was not "safe" in some cases.
This is NOT a fix. This is crippling SQLite access via JDBC.

This is supposedly a resolution to Issue #329 , but removing a feature does not resolve the problem.

We still need a reliable way to obtain the generated ID after a single INSERT statement.

There is discussion of using a "RETURNING" clause but even this says its not a complete solution.

The previous functionality worked JUST FINE if you were smart enough to obtain the generated ID after each individual insert and not let any other thread obtain access to your Connection object until you had atomically inserted and obtained the ID.

Just because this did not work with batch inserts or poorly crafted multi-threaded applications did not mean that the functionality should be removed.

Please restore support for obtaining generated keys via JDBC.

ADDITIONALLY: Removing this support in a minor release (i.e.: not 4.x but going from 3.42.x to 3.43.x) breaks backwards compatibility and therefore is not adhering to proper semantic versioning.

[sjlombardo]

+1

I fully understand that:

1. the previous behavior was not ideal, and that it broke down in more complicated scenarios featuring multi-row inserts, cross thread use, etc.
2. in a perfect world every application would check driver capabilities and gracefully handle that getGeneratedKeys is not available according to the JDBC spec

On the other hand, the sqlite-jdbc project supported (if not perfectly) getGeneratedKeys for at least 16 years. It was a long term, well established, and generally understood feature of the software with a clear precedent of support.

The abrupt removal of this capability has broken integrations with everything from Spring to Hibernate and has clearly caused problems for dozens of applications, plus more that we don't know about. I know first hand that it has caused projects to completely stop including upstream versions of sqlite-jdbc because it will irreparably break applications.

It would really be infinitely better to restore the old getGeneratedKeys behavior and document the limitations. The project could then pursue a long-term investigation for a way to improve the behavior of getGeneratedKeys without breaking backwards compatibility.

[barrycaceres]

In my searches, I thought I read that Microsoft/Mojang's Java-based Minecraft was also broken by this update.

Clearly not something that should have happened in version 3.43.0.0 -- definitely should have been saved for version 4.x.

In fact the comments in Issue #329 say that the functionality should be resolved in "the next major version"

See: #329 (comment)

It seems to me that somebody jumped the gun because 3.43.0.0 is NOT a "major version" but rather, a "minor version"

[gotson]

You seem to wrongly assume that this project uses SemVer when saying it was released in a minor release. The project does not follow SemVer, as explained here. There won't be a 4.x version, unless there is a sqlite4.

As for the change of behaviour, this is simply because there is no way to be compliant with the minimum JDBC requirements for getGeneratedKeys with the existing capabilities of sqlite3, notably the fact that it should return multiple keys in case of multiple inserts.

The function last_insert_rowid() is still there though, and you can still call it if you want from your code. You can also use a RETURNING clause, which is more accurate as it will return all the generated keys, not just the last one.

[sjlombardo]

@gotson - as a compromise, why not have DatabaseMetaData#supportsGetGeneratedKeys() return false so that spec-adhering integrations will opt not use the function, but restore the functionality to still allow getGeneratedKeys to return a value? Also, in #329, the approach of calling last_insert_rowid immediately after statement execution (instead of lazily) would address the entire subset of issues around multi-threading.

[gotson]

@gotson - as a compromise, why not have DatabaseMetaData#supportsGetGeneratedKeys() return false so that spec-adhering integrations will opt not use the function, but restore the functionality to still allow getGeneratedKeys to return a value? Also, in #329, the approach of calling last_insert_rowid immediately after statement execution (instead of lazily) would address the entire subset of issues around multi-threading.

it turns out that almost no one checks DatabaseMetaData supportsXXX and everyone has their own assumption on what a driver can/should do 🤷🏻

[sjlombardo]

it turns out that almost no one checks DatabaseMetaData supportsXXX and everyone has their own assumption on what a driver can/should do 🤷🏻

@gotson - that is actually my point exactly! Properly behaving consumers that abide by the JDBC spec and guidance will check DatabaseMetaData, and will be informed that getGeneratedKeys is not officially supported. Those that do not could continue to use the existing functionality that has been in place for almost 2 decades without breaking. It would be the best of both worlds.

I am confident that a relatively simple change could address the multi threading concerns around getGeneratedKeys. That would fix the issues in #329. The only limitation would be around multi-insert statements which are rarely used and that limitation could be documented.

I would be happy to do the work on this and submit a pull request, if you would be open to adding back in a threadsafe version of getGeneratedKeys . This would be a huge improvement and would solve a lot of problems for people using the driver today.

[gotson]

I would be happy to do the work on this and submit a pull request, if you would be open to adding back in a threadsafe version of getGeneratedKeys . This would be a huge improvement and would solve a lot of problems for people using the driver today.

Sure, if that helps people we can have a look at a PR

[sjlombardo]

@prrvchr this issue was already resolved by PR #1035, 2 releases ago. I don't think spam-posting direct links to a jar file in your fork on dozens of closed issues is having the effect you want. It's just causing a lot of noise in this project. Furthermore, it seems like there are some shortcomings and risks in the version you are linking too that could adversely impact the applications that are using this library, since it changes the core behavior that many applications have come to depend on.

[prrvchr]

@sjlombardo If you want to comply with JDBC 4.1 (java 7 / 2011) you must support getGeneratedKeys() otherwise it's JDBC 4.0 (java 6 / 2006).
There is no point in trying to reinvent JDBC, you have to implement getGeneratedKeys() and nothing will be resolved without that.
And sorry for the noise, but there are a lot of people looking for JDBC 4.1 compatibility.

[sjlombardo]

@prrvchr #1035 reintroduced getGeneratedKeys() support for single insert statements in a manner that was compatible with the previous historic behavior of this JDBC driver. That PR fixed the underlying issue that was reported in almost every ticket you commented on today with your jar link (e.g. #329, #963, #965, #980, #1018, #1028).

I will concede that the current implementation is not JDBC 4.1 compliant because it doesn't handle insertion of multiple rows in the same statement (#468). On that point, however, none of the other aforementioned tickets you commented on today actually need that because they don't do multi-row inserts. They only needed to get single insert keys from getGeneratedKeys, consistent with this drivers historic behavior. #1035 added this feature back in a thread-safe manner. This is clear if you carefully read the details of the issues you posted on and take the history of the project functionality into account.

As @gotson has very patiently explained in several other issues, it is difficult to handle multi-inserts due to SQLite limitations. Your approach may work, but it has potentially dangerous side effects in terms of modifying the user specified queries. The change in behavior would also probably break at least some existing applications. The maintainer of this project (and perhaps others) have expressed legitimate concerns about that approach, and explained why they don't want to merge it.

I say all this mainly to clarify that you are probably not going to drum up support for a hard fork by spam posting copy-pasted jar links onto issues that are only tangentially related.

That said I'm not looking to get pulled into an argument, so I'm not planning to reply back to any further conversation on this thread.

[gotson]

@prrvchr please stop spamming comments on closed issues. Posting a jar link is mostly useless anyway, since most people use dependency management anyway to pull their deps.

You are most welcome to maintain your github fork, and even publish to Maven Central if you want. However, i have already stated the position of this project that we are not interested in merging your changes. Your concerns about JDBC compatibility are well understood, and have been explained.