Add check if JDBC driver supports generated keys

[andeee]

JDBC API has DatabaseMetaData to check for features of the database. Since JDBC3 supportsGetGeneratedKeys is available.

See http://download.oracle.com/javase/6/docs/api/java/sql/DatabaseMetaData.html#supportsGetGeneratedKeys().

Attention: sqlite-jdbc reports false, but supports it - I'll file a bug on this for sqlite-jdbc

[andeee]

sorry the amount of commits, just trying to get started with git

[andeee]

Filed bug for sqlite-jdbc: http://code.google.com/p/sqlite-jdbc/issues/detail?id=10

[bendlas]

Thanks for the pull request. I have to ask you to clarify and change a few things, before commiting this.

• Splitting things up into multiple commits is a good thing. Especially good: first commiting the test cases, then the solution. Makes verifying that much easier. OTOH multiple commits are less than helpful, if you are introducing changes and reverting (some of) them a few commits later.

  Could you please clean up the commits? If nothing else, you can do that by creating a new branch with its HEAD at the latest master commit. Then add (possibly failing) test cases, commit. Then commit the fix. If you have an erroneous commit, roll it back with git reset. Don't git revert, unless the change is already released.

• You can leave in the test cases against an embedded database, like SA. More databases to run the test cases against, w/o config overhead == good.
• Can you clarify, which advantage your approach has, above the currently implemented heuristic? Is performance better? Does it handle some corner cases better? If so why and which, respectively?

[andeee]

Thank you for your comments and advice.

I'll try to clarify my intent:

• Not every JDBC driver does support getGeneratedKeys, in my example Sybase SQL Anywhere doesn't support it and throws an SQLException when e. g. trying to conj! onto a table
• We can make clojureql work with these drivers when calling supportsGetGeneratedKeys onto DatabaseMetaData before we create the statement and append the generated keys to the result
• I also check for the corner case when supportsGetGeneratedKeys isn't even implemented, catching the AbstractMethodError.
  Support for generated keys was introduced with JDBC Version 3, so here we could start to support even older drivers - tested with a SQL Anywhere JDBC Driver Version 9 which only supports JDBC Version 2

I'll now cleanup the commits and make another pull request. If you have further questions, please ask.

[bendlas]

Great! As far as I have understood, SQL Anywhere is embedded and thus can be used in the test suite w/o any external config. If so, please add it and add test cases, highlighting the wrong behavior. Please also double check, that there are test cases in place for everything that might be affected by your patch (to a reasonable extent, it will be audited anyway).

The latest problems we had in that area (of auto-generated keys), was with newer MySQL drivers. They would throw, if you didn't fetch the generated keys after a query.

Whereas the Postgres Driver would throw, when trying to get generated keys in batch mode.

I implemented a workaround, by only getting generated keys when not in batch mode, as per 257a5e02

You might want to check how your work interacts with that behavior.

Thanks for your help!

[andeee]

Thanks for your further info! SQL Anywhere is not as embedded as for example derby or hsqldb. Therefore one must download the Developer Editon from Sybase, create the database and deploy the jdbc driver to a local maven repo since it's not available on maven central.

Should I add it to the test cases anyway?

[bendlas]

In this case, please not. If you can easily reproduce it with another db, go ahead, otherwise just make sure it doesn't break anything.

Thanks!

[andeee]

cleaned up commits in #108