<< Back to previous view

[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 Wed Jul 29 19:19:47 CDT 2015 using JIRA 4.4#649-r158309.