java.jdbc

Add sqlite3 support

Details

  • Type: Enhancement Enhancement
  • Status: Resolved Resolved
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test

Description

sqlite3 has some problems:

1) PreparedStatements.addBatch does not do anything without some parameters, so executeBatch doesn't do anything.
2) Transaction support appears to require closing the ResultSet object from generatedKeys.

I've only tested the included patch with the default test dbs and sqlite3.

Attachments

  1. Text File
    mssql-error.log
    17/Feb/12 10:06 PM
    42 kB
    Nelson Morris
  2. Text File
    sqlite_transaction_test.patch
    01/Mar/12 1:15 AM
    1 kB
    Nelson Morris
  3. Text File
    sqlite3-support.patch
    13/Feb/12 11:46 PM
    6 kB
    Nelson Morris

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Sean Corfield added a comment - - edited

This will break things - see JDBC-16 which required that .addBatch be called when there are no param-groups. I'm not going to reject this out of hand but patches for new database support need testing against MySQL, PostgreSQL and MS SQL Server.

Show
Sean Corfield added a comment - - edited This will break things - see JDBC-16 which required that .addBatch be called when there are no param-groups. I'm not going to reject this out of hand but patches for new database support need testing against MySQL, PostgreSQL and MS SQL Server.
Hide
Permalink
Nelson Morris added a comment -

Test suite runs correctly for TEST_DBS=mysql,postgres,derby,hsqldb,sqlite3 and TEST_DBS=mysql-str,postgres-str. This just leaves MS SQL server, which I do not have the ability to run.

Regarding JDBC-16, when I revert to just using .executeBatch for the no param-groups case I can see the errors produced in the test-suite. Using .executeUpdate for the no params-group case continues to fix these errors (note MS SQL server untested). I do not see any other information in the ticket that would explain why using .executeUpdate instead of .addBatch/.executeBatch for the no params-group case would break JDBC-16. Is there a reason I am missing?

Show
Nelson Morris added a comment - Test suite runs correctly for TEST_DBS=mysql,postgres,derby,hsqldb,sqlite3 and TEST_DBS=mysql-str,postgres-str. This just leaves MS SQL server, which I do not have the ability to run. Regarding JDBC-16, when I revert to just using .executeBatch for the no param-groups case I can see the errors produced in the test-suite. Using .executeUpdate for the no params-group case continues to fix these errors (note MS SQL server untested). I do not see any other information in the ticket that would explain why using .executeUpdate instead of .addBatch/.executeBatch for the no params-group case would break JDBC-16. Is there a reason I am missing?
Hide
Permalink
Nelson Morris added a comment -

I ran the test suite against an ec2 instance with MS SQL server 2008 R2. Current test suite on master fails with attached mssql-error.log. Using patch produces the same output.

Please let me know if there is anything that you would like changed.

Show
Nelson Morris added a comment - I ran the test suite against an ec2 instance with MS SQL server 2008 R2. Current test suite on master fails with attached mssql-error.log. Using patch produces the same output. Please let me know if there is anything that you would like changed.
Nelson Morris made changes -
Field Original Value New Value
Attachment mssql-error.log [ 10923 ]
Hide
Permalink
Sean Corfield added a comment -

Thanx Nelson. I've been a bit busy with work this week but I'll try to look at this at the weekend. I appreciate your thoroughness on testing this!

Show
Sean Corfield added a comment - Thanx Nelson. I've been a bit busy with work this week but I'll try to look at this at the weekend. I appreciate your thoroughness on testing this!
Hide
Permalink
Sean Corfield added a comment -

Integrated parts of the patch. SQLite 3 is now supported. Thanx!

Show
Sean Corfield added a comment - Integrated parts of the patch. SQLite 3 is now supported. Thanx!
Sean Corfield made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Resolved [ 5 ]
Hide
Permalink
Nelson Morris added a comment -

I still get the transaction issue (part 2 from original description) when clojure.java.jdbc tries to return the generated keys, in a transaction, using sqlite. https://gist.github.com/1947746

I believe this is because the ResultSet from .getGeneratedKeys never gets .close called, so sqlite keeps the db locked and can't rollback. Would you like me to make a new issue?

Show
Nelson Morris added a comment - I still get the transaction issue (part 2 from original description) when clojure.java.jdbc tries to return the generated keys, in a transaction, using sqlite. https://gist.github.com/1947746 I believe this is because the ResultSet from .getGeneratedKeys never gets .close called, so sqlite keeps the db locked and can't rollback. Would you like me to make a new issue?
Hide
Permalink
Sean Corfield added a comment - - edited

The tests pass locally (Mac OS X) and on the Clojure build server (Linux) but you're right that I didn't include the close call on the record set.

I didn't include your additional test which I'm guessing is the key to highlighting the errant behavior for SQLite?

(I couldn't actually apply your patch because I'd spent some time getting all the tests passing for MS SQL Server with both the Microsoft and jTDS drivers)

Show
Sean Corfield added a comment - - edited The tests pass locally (Mac OS X) and on the Clojure build server (Linux) but you're right that I didn't include the close call on the record set. I didn't include your additional test which I'm guessing is the key to highlighting the errant behavior for SQLite? (I couldn't actually apply your patch because I'd spent some time getting all the tests passing for MS SQL Server with both the Microsoft and jTDS drivers)
Sean Corfield made changes -
Status Resolved [ 5 ] Reopened [ 4 ]
Resolution Completed [ 1 ]
Nelson Morris made changes -
Attachment sqlite_transaction_test.patch [ 10975 ]
Hide
Permalink
Nelson Morris added a comment -

Attached patch that fails with `mvn test`.

Show
Nelson Morris added a comment - Attached patch that fails with `mvn test`.
Hide
Permalink
Sean Corfield added a comment - - edited

I believe this was fixed in 0.1.3 (at the end of February). As far as I can tell, all of the patch components are applied. Specifically, your transaction test is included in the test suite and passes.

Show
Sean Corfield added a comment - - edited I believe this was fixed in 0.1.3 (at the end of February). As far as I can tell, all of the patch components are applied. Specifically, your transaction test is included in the test suite and passes.
Sean Corfield made changes -
Resolution Completed [ 1 ]
Status Reopened [ 4 ] Resolved [ 5 ]

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.