<< Back to previous view

[JDBC-110] Requesting a nested transaction with a more restrictive isolation level appears to succeed, but does not have the requested effect Created: 22/Jul/15  Updated: 27/Jul/15  Resolved: 27/Jul/15

Status: Resolved
Project: java.jdbc
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Donald Ball Assignee: Sean Corfield
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File check-transaction-isolation-level.patch    
Patch: Code and Test

 Description   

The library helpfully allows nested transactions by maintaining an internal depth counter, but silently disregards any transaction options for nested transactions. This has actually bitten me recently in production; code that required a serializable transaction in order to maintain an invariant was inadvertently being executed within a repeatable-read transaction.

This patch changes the behavior to raise if a nested transaction requests a different isolation level in which the actual transaction is running.

A similar problem exists for nested transactions that request read-only mode. I will be happy to modify the patch to cover that case if you are interested in accepting this.



 Comments   
Comment by Sean Corfield [ 22/Jul/15 2:18 PM ]

Thanks Donald. I'll take a look at this.

You don't appear to be listed here: http://clojure.org/contributing – have you signed and submitted the Contributor's Agreement?

Comment by Donald Ball [ 23/Jul/15 1:34 PM ]

Not yet, but I will do so forthwith.

Comment by Donald Ball [ 23/Jul/15 1:37 PM ]

Okay, I'm legal.

Comment by Sean Corfield [ 23/Jul/15 1:38 PM ]

Great! I'll try to take a look at this over the weekend.

Comment by Sean Corfield [ 27/Jul/15 12:46 AM ]

Thanks for the patch Donald!

Comment by Donald Ball [ 27/Jul/15 8:38 AM ]

Would you be amenable to also raising if the inner transaction requests an incompatible read-only mode?

Comment by Sean Corfield [ 27/Jul/15 12:03 PM ]

Sure. Do you want to create a new ticket and attach a patch to it?





[JDBC-109] :port should default to 5432 for PostgreSQL Created: 11/Jun/15  Updated: 27/Jul/15  Resolved: 27/Jul/15

Status: Resolved
Project: java.jdbc
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Sean Corfield Assignee: Sean Corfield
Resolution: Completed Votes: 0
Labels: None


 Description   

We default the port for MySQL and SQL Server. We should do it for PostgreSQL too.

We should probably clean up the handling of Apache Derby, SQLite, H2, and HSQLDB with :dbtype so the host/port are omitted.



 Comments   
Comment by Sean Corfield [ 27/Jul/15 1:19 AM ]

Port defaulted and Derby, SQLite and HSQLDB updated to support dbtype / dbname.





[JDBC-111] db-do-prepared does not correctly check if transaction? was passed Created: 26/Jul/15  Updated: 27/Jul/15  Resolved: 27/Jul/15

Status: Resolved
Project: java.jdbc
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Stefan Kamphausen Assignee: Sean Corfield
Resolution: Completed Votes: 0
Labels: None
Environment:

[org.clojure/java.jdbc "0.3.7"]
Ubuntu Linux 12.04
Testing against a DB2 database


Attachments: Text File DBC-111-check-PS.patch     Text File DBC-111-with-docstring.patch    

 Description   

The function db-do-prepared allows an optional second parameter "transaction?". Whether or not this was passed is done by checking it for being a string:

(if (string? transaction?)
      (apply db-do-prepared db true transaction? opts)

The assumption behind this seems to be, that a string indicates an SQL query parameter.

But, as the code a little further down shows, users can also pass in a prepared statement:

(if (instance? PreparedStatement sql)

So, to find out if transaction? was passed as a parameter one could either check for string and prepared statement or turn the logic around an check for boolean:

(if (or (string? transaction?)
            (instance? PreparedStatement sql))
      (apply db-do-prepared db true transaction? opts)
      (if-let [^java.sql.Connection con (db-find-connection db)]

Or

(if (not (instance? Boolean transaction?))
      (apply db-do-prepared db true transaction? opts)

The latter does not deal with nil which would probably be a natural choice for passing falsehood while the former would have to be extended when another thing may be passed as SQL, which seems unlikely.



 Comments   
Comment by Sean Corfield [ 26/Jul/15 2:11 PM ]

The assumption was that if you wanted to pass a PreparedStatement, you had to provide transaction? first (i.e., it could only be omitted when passing a sql string).

I can see value in allowing it to be omitted when a PreparedStatement is passed by expanding the condition to: (if (or (string? transaction?) (instance? PreparedStatement transaction?)) ...

Comment by Stefan Kamphausen [ 26/Jul/15 3:56 PM ]

Assuming that transaction? be passed anyway, if a prepared statement is passed, is fair enough, too. In that case, I humbly suggest adjusting the docstring a bit.

BTW, just replacing

(if (string? transaction?)

with

(if (or (string? transaction?)
          (instance? PreparedStatement sql))

Breaks the tests. Seems like it is not as easy as it seemed at first glance.

Comment by Stefan Kamphausen [ 26/Jul/15 4:01 PM ]
(if (or (string? transaction?)
          (instance? PreparedStatement transaction?))

works of course. Sorry for the noise.

Comment by Stefan Kamphausen [ 26/Jul/15 4:11 PM ]

1. Adjust check for transaction? to test for PreparedStatement, too

2. First patch plus addition to docstring.

Comment by Sean Corfield [ 27/Jul/15 12:31 AM ]

Fixed. Thank you!





Generated at Sat Aug 01 13:17:42 CDT 2015 using JIRA 4.4#649-r158309.